Re: [patch v5] x86/platform/mellanox: introduce support for Mellanox systems platform

From: Thomas Gleixner
Date: Thu Sep 22 2016 - 13:11:56 EST


On Tue, 20 Sep 2016, vadimp@xxxxxxxxxxxx wrote:

Just a few nitpicks.

> +/* Regions for LPC I2C controller and LPC base register space */
> +static struct resource mlxplat_lpc_resources[] = {

Shouldnt this be const?

> +static int mlxplat_default_channels[][8] = {
> + {

Ditto. And some more of these?

> +static struct dmi_system_id mlxplat_dmi_table[] __initdata = {

This one as well.

> +static int __init mlxplat_init(void)
> +{
> + struct mlxplat_priv *priv;
> + int i;
> + int err;

Same types can go into one line.

> +
> + return err;

err is unitialized if we get here. Just return 0 and be done with it.
> +
> +fail_platform_mux_register:

> +static void __exit mlxplat_exit(void)
> +{
> + struct mlxplat_priv *priv = platform_get_drvdata(mlxplat_dev);
> + int i;
> +
> + for (i = ARRAY_SIZE(mlxplat_mux_data) - 1; i >= 0 ; i--)
> + platform_device_unregister(priv->pdev_mux[i]);
> +
> + platform_device_unregister(priv->pdev_i2c);
> + platform_device_unregister(mlxplat_dev);
> +}
> +
> +module_init(mlxplat_init);
> +module_exit(mlxplat_exit);

Please move the module_xxx() right under the closing brace of the function
to which it belongs.

Otherwise this looks good.

Thanks,

tglx