Re: [PATCH] i2c: Convert to subsys_platform_driver()

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

Hi,

> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -896,19 +896,7 @@ static struct platform_driver davinci_i2c_driver = {
>  		.of_match_table = davinci_i2c_of_match,
>  	},
>  };
> -
> -/* I2C may be needed to bring up other drivers */
> -static int __init davinci_i2c_init_driver(void)

I'd vote to keep this comment.

> -{
> -	return platform_driver_register(&davinci_i2c_driver);
> -}
> -subsys_initcall(davinci_i2c_init_driver);
> -
> -static void __exit davinci_i2c_exit_driver(void)
> -{
> -	platform_driver_unregister(&davinci_i2c_driver);
> -}
> -module_exit(davinci_i2c_exit_driver);
> +subsys_platform_driver(davinci_i2c_driver);

...

> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index bd659dfa702f..cd4d6d6b32c6 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -1611,20 +1611,7 @@ static struct platform_driver omap_i2c_driver = {
>  		.of_match_table = of_match_ptr(omap_i2c_of_match),
>  	},
>  };
> -
> -/* I2C may be needed to bring up other drivers */

Ditto.

> -static int __init
> -omap_i2c_init_driver(void)
> -{
> -	return platform_driver_register(&omap_i2c_driver);
> -}
> -subsys_initcall(omap_i2c_init_driver);
> -
> -static void __exit omap_i2c_exit_driver(void)
> -{
> -	platform_driver_unregister(&omap_i2c_driver);
> -}
> -module_exit(omap_i2c_exit_driver);
> +subsys_platform_driver(omap_i2c_driver);
>  
>  MODULE_AUTHOR("MontaVista Software, Inc. (and others)");
>  MODULE_DESCRIPTION("TI OMAP I2C bus adapter");
> diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
> index e1cc2b2bd628..9a9a12b7e981 100644
> --- a/drivers/i2c/busses/i2c-pnx.c
> +++ b/drivers/i2c/busses/i2c-pnx.c
> @@ -735,16 +735,6 @@ static struct platform_driver i2c_pnx_driver = {
>  	.remove = i2c_pnx_remove,
>  };
>  
> -static int __init i2c_adap_pnx_init(void)
> -{
> -	return platform_driver_register(&i2c_pnx_driver);
> -}
> -
> -static void __exit i2c_adap_pnx_exit(void)
> -{
> -	platform_driver_unregister(&i2c_pnx_driver);
> -}
> -
>  MODULE_AUTHOR("Vitaly Wool");
>  MODULE_AUTHOR("Dennis Kovalev <source@mvista.com>");
>  MODULE_DESCRIPTION("I2C driver for Philips IP3204-based I2C busses");
> @@ -752,5 +742,4 @@ MODULE_LICENSE("GPL");
>  MODULE_ALIAS("platform:pnx-i2c");
>  
>  /* We need to make sure I2C is initialized before USB */
> -subsys_initcall(i2c_adap_pnx_init);
> -module_exit(i2c_adap_pnx_exit);
> +subsys_platform_driver(i2c_pnx_driver);

Use this occasion to move the comment and your code upwards right below
the driver struct?

> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
> index 3a55f803e4f1..1ecf14890209 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -986,18 +986,7 @@ static struct platform_driver sh_mobile_i2c_driver = {
>  	.probe		= sh_mobile_i2c_probe,
>  	.remove		= sh_mobile_i2c_remove,
>  };
> -
> -static int __init sh_mobile_i2c_adap_init(void)
> -{
> -	return platform_driver_register(&sh_mobile_i2c_driver);
> -}
> -subsys_initcall(sh_mobile_i2c_adap_init);
> -
> -static void __exit sh_mobile_i2c_adap_exit(void)
> -{
> -	platform_driver_unregister(&sh_mobile_i2c_driver);
> -}
> -module_exit(sh_mobile_i2c_adap_exit);
> +subsys_platform_driver(sh_mobile_i2c_driver);

Here you have no empty line after the driver struct...

>  
>  MODULE_DESCRIPTION("SuperH Mobile I2C Bus Controller driver");
>  MODULE_AUTHOR("Magnus Damm");
> diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
> index a1ab6ef6f071..152ab19f272a 100644
> --- a/drivers/i2c/busses/i2c-versatile.c
> +++ b/drivers/i2c/busses/i2c-versatile.c
> @@ -116,18 +116,7 @@ static struct platform_driver i2c_versatile_driver = {
>  	},
>  };
>  
> -static int __init i2c_versatile_init(void)
> -{
> -	return platform_driver_register(&i2c_versatile_driver);
> -}
> -
> -static void __exit i2c_versatile_exit(void)
> -{
> -	platform_driver_unregister(&i2c_versatile_driver);
> -}
> -
> -subsys_initcall(i2c_versatile_init);
> -module_exit(i2c_versatile_exit);
> +subsys_platform_driver(i2c_versatile_driver);

... while you have one here. Please make it consistent to no empty line.

Other than that, looks good.

Happy hacking,

   Wolfram