mfd: make da90xx drivers explicitly non-modular

To: [email protected]
Cc: Paul Gortmaker; Support Opensource; Lee Jones; Eric Miao; Mike Rapoport
Subject: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config PMIC_DA903X
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9030/DA9034 PMIC
Support"

...meaning that it currently is not being built as a module by anyone.

Remove the modular code that is essentially orphaned, so that when reading the driver there is no doubt it is builtin-only.

Also explicitly disallow a driver unbind, since that doesn't have a sensible use case anyway, and it allows us to drop the ".remove" code for non-modular drivers.

Since module_init was not in use by this code, the init ordering remains unchanged with this commit.

We replace module.h with init.h and export.h ; the latter since this file does export some syms.

Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.

We also delete the MODULE_LICENSE tag etc. since all that information is already contained at the top of the file in the comments.

Cc Support Opensource <[email protected]>

Cc Lee Jones <[email protected]>

Cc Eric Miao <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Paul Gortmaker <[email protected]>

---
 drivers/mfd/da903x.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

drivers/mfd/da903x.c +3 -24

index 09f367571c58..78edb0558a0f 100644
@@ -13,7 +13,7 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>

@@ -446,7 +446,6 @@ static const struct i2c_device_id da903x_id_table[] = {
 	{ "da9034", 1 },
 	{ },
 };
-MODULE_DEVICE_TABLE(i2c, da903x_id_table);
 
 static int __remove_subdev(struct device *dev, void *unused)
 {

@@ -535,20 +534,12 @@ static int da903x_probe(struct i2c_client *client,
 	return da903x_add_subdevs(chip, pdata);
 }
 
-static int da903x_remove(struct i2c_client *client)
-{
-	struct da903x_chip *chip = i2c_get_clientdata(client);
-
-	da903x_remove_subdevs(chip);
-	return 0;
-}
-
 static struct i2c_driver da903x_driver = {
 	.driver	= {
-		.name	= "da903x",
+		.name			= "da903x",
+		.suppress_bind_attrs	= true,
 	},
 	.probe		= da903x_probe,
-	.remove		= da903x_remove,
 	.id_table	= da903x_id_table,
 };
 

@@ -557,14 +548,3 @@ static int __init da903x_init(void)
 	return i2c_add_driver(&da903x_driver);
 }
 subsys_initcall(da903x_init);
-
-static void __exit da903x_exit(void)
-{
-	i2c_del_driver(&da903x_driver);
-}
-module_exit(da903x_exit);
-
-MODULE_DESCRIPTION("PMIC Driver for Dialog Semiconductor DA9034");
-MODULE_AUTHOR("Eric Miao <[email protected]>");
-MODULE_AUTHOR("Mike Rapoport <[email protected]>");
-MODULE_LICENSE("GPL v2");
--

2.11.0

Hi Paul,

Quoted 11 lines · On 03 June 2017 14:04 Paul Gortmaker wrote:

To: [email protected]
Cc: Paul Gortmaker; Support Opensource; Lee Jones; Eric Miao; Mike Rapoport
Subject: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config PMIC_DA903X
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9030/DA9034 PMIC
Support"

...meaning that it currently is not being built as a module by anyone.

With regards to your four patches,

[4/4] mfd: da9055-i2c: Make it explicitly non-modular - - - 2017-06-03
[3/4] mfd: da9055-core: make it explicitly non-modular - - - 2017-06-03
[2/4] mfd: da9052-*: Make it explicitly non-modular - - - 2017-06-03
[1/4] mfd: da903x: Make it explicitly non-modular - - - 2017-06-03

Is there an reason not to make the Kconfig tristate? Would that not be simpler?

Previously, something similar was suggested by Geert Uytterhoeven for DA9063:
https://patchwork.kernel.org/patch/7788001/

Regards,
Steve

Quoted 88 lines

Remove the modular code that is essentially orphaned, so that when reading the driver there is no doubt it is builtin-only.

Also explicitly disallow a driver unbind, since that doesn't have a sensible use case anyway, and it allows us to drop the ".remove" code for non-modular drivers.

Since module_init was not in use by this code, the init ordering remains unchanged with this commit.

We replace module.h with init.h and export.h ; the latter since this file does export some syms.

Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.

We also delete the MODULE_LICENSE tag etc. since all that information is already contained at the top of the file in the comments.

Cc Support Opensource <[email protected]>

Cc Lee Jones <[email protected]>

Cc Eric Miao <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Paul Gortmaker <[email protected]>

---
 drivers/mfd/da903x.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

drivers/mfd/da903x.c +3 -24

index 09f367571c58..78edb0558a0f 100644
@@ -13,7 +13,7 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>

@@ -446,7 +446,6 @@ static const struct i2c_device_id da903x_id_table[] = {
 	{ "da9034", 1 },
 	{ },
 };
-MODULE_DEVICE_TABLE(i2c, da903x_id_table);
 
 static int __remove_subdev(struct device *dev, void *unused)
 {

@@ -535,20 +534,12 @@ static int da903x_probe(struct i2c_client *client,
 	return da903x_add_subdevs(chip, pdata);
 }
 
-static int da903x_remove(struct i2c_client *client)
-{
-	struct da903x_chip *chip = i2c_get_clientdata(client);
-
-	da903x_remove_subdevs(chip);
-	return 0;
-}
-
 static struct i2c_driver da903x_driver = {
 	.driver	= {
-		.name	= "da903x",
+		.name			= "da903x",
+		.suppress_bind_attrs	= true,
 	},
 	.probe		= da903x_probe,
-	.remove		= da903x_remove,
 	.id_table	= da903x_id_table,
 };
 

@@ -557,14 +548,3 @@ static int __init da903x_init(void)
 	return i2c_add_driver(&da903x_driver);
 }
 subsys_initcall(da903x_init);
-
-static void __exit da903x_exit(void)
-{
-	i2c_del_driver(&da903x_driver);
-}
-module_exit(da903x_exit);
-
-MODULE_DESCRIPTION("PMIC Driver for Dialog Semiconductor DA9034");
-MODULE_AUTHOR("Eric Miao <[email protected]>");
-MODULE_AUTHOR("Mike Rapoport <[email protected]>");
-MODULE_LICENSE("GPL v2");
--

2.11.0

Subject: Re: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular

[RE: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular]

Quoted 16 lines · On 05/06/2017 (Mon 10:30) Steve Twiss wrote:

Quoted 7 lines · On 03 June 2017 14:04 Paul Gortmaker wrote:

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config PMIC_DA903X
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9030/DA9034 PMIC
Support"

...meaning that it currently is not being built as a module by anyone.

With regards to your four patches,

[4/4] mfd: da9055-i2c: Make it explicitly non-modular - - - 2017-06-03
[3/4] mfd: da9055-core: make it explicitly non-modular - - - 2017-06-03
[2/4] mfd: da9052-*: Make it explicitly non-modular - - - 2017-06-03
[1/4] mfd: da903x: Make it explicitly non-modular - - - 2017-06-03

Quoted 2 lines

Is there an reason not to make the Kconfig tristate? Would that not be simpler?

I explicitly covered that in the 0/4 e-mail:

As always, the option exists for someone with the hardware and the desire to extend the functionality to make any given driver tristate. But given the number of these tree wide and the fact that I can't test that new extended functionality in all cases, I just make the code consistent with existing Kconfig/Makefile settings that currently restrict them to "bool".

Hi Paul,

Quoted 21 lines · Paul Gortmaker

Subject: Re: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular

[RE: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular]

Quoted 16 lines · On 05/06/2017 (Mon 10:30) Steve Twiss wrote:

Quoted 7 lines · On 03 June 2017 14:04 Paul Gortmaker wrote:

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config PMIC_DA903X
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9030/DA9034 PMIC
Support"

...meaning that it currently is not being built as a module by anyone.

With regards to your four patches,

[4/4] mfd: da9055-i2c: Make it explicitly non-modular - - - 2017-06-03
[3/4] mfd: da9055-core: make it explicitly non-modular - - - 2017-06-03
[2/4] mfd: da9052-*: Make it explicitly non-modular - - - 2017-06-03
[1/4] mfd: da903x: Make it explicitly non-modular - - - 2017-06-03

Quoted 4 lines

Quoted 2 lines

Is there an reason not to make the Kconfig tristate? Would that not be simpler?

I explicitly covered that in the 0/4 e-mail:

Apologies, I don't seem to have been CC'ed on the 0/4 e-mail, although I got the others. Dialog's developer e-mail addresses come as part of the Support Opensource e-mail distribution list.

Quoted 6 lines

As always, the option exists for someone with the hardware and the desire to extend the functionality to make any given driver tristate. But given the number of these tree wide and the fact that I can't test that new extended functionality in all cases, I just make the code consistent with existing Kconfig/Makefile settings that currently restrict them to "bool".

I see your point, however we have many customers and it is unclear whether they are using modules for any of these drivers.
Even if that feature in Kconfig is not enabled, it is possible a tristate change has been made and is being used, but has not been pushed back into the Linux mainline.

So I would recommend against removing this feature.

The driver code is being supported by Dialog Semiconductor, where possible.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/MAINTAINERS?h=v4.12-rc4#n3984
If there is a question about supporting modules in these drivers, we have the ability to test on target for da9055/52.

Regards,
Steve

Subject: Re: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular

Quoted 11 lines

Quoted 5 lines

As always, the option exists for someone with the hardware and the desire to extend the functionality to make any given driver tristate. But given the number of these tree wide and the fact that I can't test that new extended functionality in all cases, I just make the code consistent with existing Kconfig/Makefile settings that currently restrict them to "bool".

I see your point, however we have many customers and it is unclear whether they are using modules for any of these drivers.
Even if that feature in Kconfig is not enabled, it is possible a tristate change has been made and is being used, but has not been pushed back into the Linux mainline.

I'd have to suspect that is pretty unlikely, but in the end if you think these chips have a use case for being modular that isn't just academic, then there is no reason why they can't be tristate.

Quoted 6 lines

So I would recommend against removing this feature.

The driver code is being supported by Dialog Semiconductor, where possible.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/MAINTAINERS?h=v4.12-rc4#n3984
If there is a question about supporting modules in these drivers, we have the ability to test on target for da9055/52.

So, the "conversion" patches would be the trivial one line change from "bool" to "tristate" and the real effort is the validation. Do you want to submit those trivial changes after you've had a chance to validate them? There is no point in me sending you one line patches to test.

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config PMIC_DA903X
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9030/DA9034 PMIC Support"

...meaning that it currently is not being built as a module by anyone.

Remove the modular code that is essentially orphaned, so that when reading the driver there is no doubt it is builtin-only.

Also explicitly disallow a driver unbind, since that doesn't have a sensible use case anyway, and it allows us to drop the ".remove" code for non-modular drivers.

Since module_init was not in use by this code, the init ordering remains unchanged with this commit.

We replace module.h with init.h and export.h ; the latter since this file does export some syms.

Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.

We also delete the MODULE_LICENSE tag etc. since all that information is already contained at the top of the file in the comments.

Cc Support Opensource <[email protected]>

Cc Lee Jones <[email protected]>

Cc Eric Miao <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Paul Gortmaker <[email protected]>

---
 drivers/mfd/da903x.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

drivers/mfd/da903x.c +3 -23

index 09f367571c58..78edb0558a0f 100644
@@ -13,7 +13,7 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>

@@ -446,7 +446,6 @@ static const struct i2c_device_id da903x_id_table[] = {
 	{ "da9034", 1 },
 	{ },
 };
-MODULE_DEVICE_TABLE(i2c, da903x_id_table);
 
 static int __remove_subdev(struct device *dev, void *unused)
 {

@@ -535,20 +534,12 @@ static int da903x_probe(struct i2c_client *client,
 	return da903x_add_subdevs(chip, pdata);
 }
 
-static int da903x_remove(struct i2c_client *client)
-{
-	struct da903x_chip *chip = i2c_get_clientdata(client);
-
-	da903x_remove_subdevs(chip);
-	return 0;
-}
-
 static struct i2c_driver da903x_driver = {
 	.driver	= {
-		.name	= "da903x",
+		.name			= "da903x",
+		.suppress_bind_attrs	= true,
 	},
 	.probe		= da903x_probe,
-	.remove		= da903x_remove,
 	.id_table	= da903x_id_table,
 };
 

@@ -557,14 +548,3 @@ static int __init da903x_init(void)
 	return i2c_add_driver(&da903x_driver);
 }
 subsys_initcall(da903x_init);
-
-static void __exit da903x_exit(void)
-{
-	i2c_del_driver(&da903x_driver);
-}
-module_exit(da903x_exit);
-
-MODULE_DESCRIPTION("PMIC Driver for Dialog Semiconductor DA9034");
-MODULE_AUTHOR("Eric Miao <[email protected]>");
-MODULE_AUTHOR("Mike Rapoport <[email protected]>");
-MODULE_LICENSE("GPL v2");

Quoted 10 lines · Paul Gortmaker

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig:config PMIC_DA903X
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9030/DA9034 PMIC Support"

...meaning that it currently is not being built as a module by anyone.

Remove the modular code that is essentially orphaned, so that when reading the driver there is no doubt it is builtin-only.

Quoted 3 lines

Also explicitly disallow a driver unbind, since that doesn't have a sensible use case anyway, and it allows us to drop the ".remove" code for non-modular drivers.

Do this device resides only on platforms which cannot be shut down, right?

Quoted 80 lines

Since module_init was not in use by this code, the init ordering remains unchanged with this commit.

We replace module.h with init.h and export.h ; the latter since this file does export some syms.

Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.

We also delete the MODULE_LICENSE tag etc. since all that information is already contained at the top of the file in the comments.

Cc Support Opensource <[email protected]>

Cc Lee Jones <[email protected]>

Cc Eric Miao <[email protected]>

Cc Mike Rapoport <[email protected]>

Signed-off-by Paul Gortmaker <[email protected]>

---
 drivers/mfd/da903x.c | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

drivers/mfd/da903x.c +3 -23

index 09f367571c58..78edb0558a0f 100644
@@ -13,7 +13,7 @@
  */
 
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>

@@ -446,7 +446,6 @@ static const struct i2c_device_id da903x_id_table[] = {
 	{ "da9034", 1 },
 	{ },
 };
-MODULE_DEVICE_TABLE(i2c, da903x_id_table);
 
 static int __remove_subdev(struct device *dev, void *unused)
 {

@@ -535,20 +534,12 @@ static int da903x_probe(struct i2c_client *client,
 	return da903x_add_subdevs(chip, pdata);
 }
 
-static int da903x_remove(struct i2c_client *client)
-{
-	struct da903x_chip *chip = i2c_get_clientdata(client);
-
-	da903x_remove_subdevs(chip);
-	return 0;
-}
-
 static struct i2c_driver da903x_driver = {
 	.driver	= {
-		.name	= "da903x",
+		.name			= "da903x",
+		.suppress_bind_attrs	= true,
 	},
 	.probe		= da903x_probe,
-	.remove		= da903x_remove,
 	.id_table	= da903x_id_table,
 };
 

@@ -557,14 +548,3 @@ static int __init da903x_init(void)
 	return i2c_add_driver(&da903x_driver);
 }
 subsys_initcall(da903x_init);
-
-static void __exit da903x_exit(void)
-{
-	i2c_del_driver(&da903x_driver);
-}
-module_exit(da903x_exit);
-
-MODULE_DESCRIPTION("PMIC Driver for Dialog Semiconductor DA9034");
-MODULE_AUTHOR("Eric Miao <[email protected]>");
-MODULE_AUTHOR("Mike Rapoport <[email protected]>");
-MODULE_LICENSE("GPL v2");

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Hi Paul,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.12-rc3 next-20170602]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Gortmaker/mfd-make-da90xx-drivers-explicitly-non-modular/20170603-211054
base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: s390-allmodconfig (attached as .config)
compiler: s390x-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705

reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390

Note: the linux-review/Paul-Gortmaker/mfd-make-da90xx-drivers-explicitly-non-modular/20170603-211054 HEAD c8ab14d7a1d96ff8fb5121c974030c48824d0914 builds fine.
It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `da9055_i2c_remove':

Quoted 1 line

Quoted 1 line

drivers/mfd/.tmp_gl_da9055-i2c.o:(.text+0x17860e): undefined reference to `da9055_device_exit'

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation