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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 22 Sep 2016 19:08:30 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Vadim Pasternak <vadimp@...lanox.com>
cc:     mingo@...hat.com, hpa@...or.com, davem@...emloft.net,
        geert@...ux-m68k.org, akpm@...ux-foundation.org,
        gregkh@...uxfoundation.org, kvalo@...eaurora.org,
        mchehab@...nel.org, linux@...ck-us.net, x86@...nel.org,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        jiri@...nulli.us
Subject: Re: [patch v5] x86/platform/mellanox: introduce support for Mellanox
 systems platform

On Tue, 20 Sep 2016, vadimp@...lanox.com 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