At the risk of repeating the same boiler plate, in this cleanup we continue
to make driver code consistent with the Makefiles/Kconfigs that control them.
This means not using modular functions/macros for drivers that can never
be built as a module. Some of the main downfalls this leads to are:
(1) it is easy to accidentally write unused module_exit and remove code
(2) it can be misleading when reading the source, thinking it can be
modular when the Makefile and/or Kconfig prohibit it
(3) it requires the include of the module.h header file which in turn
includes nearly everything else, thus adding to CPP overhead.
(4) it gets copied/replicated into other drivers and thus spreads.
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".
Build tested on linux-next for arm, arm64 and x86-64 to ensure no typos
or similar issues crept in. The diffstat summary speaks for itself.
Paul Gortmaker (4):
mfd: da903x: Make it explicitly non-modular
mfd: da9052-*: Make it explicitly non-modular
mfd: da9055-core: make it explicitly non-modular
mfd: da9055-i2c: Make it explicitly non-modular
drivers/mfd/da903x.c | 26 +++-----------------------
drivers/mfd/da9052-core.c | 11 -----------
drivers/mfd/da9052-i2c.c | 22 ++--------------------
drivers/mfd/da9052-irq.c | 1 -
drivers/mfd/da9052-spi.c | 22 ++--------------------
drivers/mfd/da9055-core.c | 13 ++-----------
drivers/mfd/da9055-i2c.c | 24 ++----------------------
include/linux/mfd/da9052/da9052.h | 1 -
8 files changed, 11 insertions(+), 109 deletions(-)
--
2.11.0
The Kconfig currently controlling compilation of this code is:
drivers/mfd/Kconfig:config MFD_DA9055
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9055 PMIC Support"
...meaning that it is never being built as a module.
Remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
The exit function deleted here wasn't even registered with module_exit,
so it truly was dead 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.
We replace module.h with init.h and export.h ; the latter since the
file does export some symbols
Cc: Support Opensource <[email protected]>
Cc: Lee Jones <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/mfd/da9055-core.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/mfd/da9055-core.c b/drivers/mfd/da9055-core.c
index 177e65a12c12..b55f13061547 100644
--- a/drivers/mfd/da9055-core.c
+++ b/drivers/mfd/da9055-core.c
@@ -11,7 +11,8 @@
* option) any later version.
*/
-#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/export.h>
#include <linux/device.h>
#include <linux/input.h>
#include <linux/irq.h>
@@ -416,13 +417,3 @@ int da9055_device_init(struct da9055 *da9055)
mfd_remove_devices(da9055->dev);
return ret;
}
-
-void da9055_device_exit(struct da9055 *da9055)
-{
- regmap_del_irq_chip(da9055->chip_irq, da9055->irq_data);
- mfd_remove_devices(da9055->dev);
-}
-
-MODULE_DESCRIPTION("Core support for the DA9055 PMIC");
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("David Dajun Chen <[email protected]>");
--
2.11.0
The Kconfigis currently controlling compilation of this code are:
mfd/Kconfig:config MFD_DA9052_SPI
mfd/Kconfig: bool "Dialog Semiconductor DA9052/53 PMIC variants with SPI"
mfd/Kconfig:config MFD_DA9052_I2C
mfd/Kconfig: bool "Dialog Semiconductor DA9052/53 PMIC variants with I2C"
drivers/mfd/Kconfig:config PMIC_DA9052
drivers/mfd/Kconfig: bool
...meaning that this code is never built as a module.
Remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
In doing so, da9052_device_exit() becomes orphaned, so it gets
removed as well.
We 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 where required, i.e. if the the file
did not include that already.
We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the files in the comments.
Cc: Support Opensource <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: David Dajun Chen <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/mfd/da9052-core.c | 11 -----------
drivers/mfd/da9052-i2c.c | 22 ++--------------------
drivers/mfd/da9052-irq.c | 1 -
drivers/mfd/da9052-spi.c | 22 ++--------------------
include/linux/mfd/da9052/da9052.h | 1 -
5 files changed, 4 insertions(+), 53 deletions(-)
diff --git a/drivers/mfd/da9052-core.c b/drivers/mfd/da9052-core.c
index a88c2065d8ab..ca878d9ab712 100644
--- a/drivers/mfd/da9052-core.c
+++ b/drivers/mfd/da9052-core.c
@@ -17,7 +17,6 @@
#include <linux/interrupt.h>
#include <linux/mfd/core.h>
#include <linux/slab.h>
-#include <linux/module.h>
#include <linux/mfd/da9052/da9052.h>
#include <linux/mfd/da9052/pdata.h>
@@ -626,13 +625,3 @@ int da9052_device_init(struct da9052 *da9052, u8 chip_id)
return ret;
}
-
-void da9052_device_exit(struct da9052 *da9052)
-{
- mfd_remove_devices(da9052->dev);
- da9052_irq_exit(da9052);
-}
-
-MODULE_AUTHOR("David Dajun Chen <[email protected]>");
-MODULE_DESCRIPTION("DA9052 MFD Core");
-MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/da9052-i2c.c b/drivers/mfd/da9052-i2c.c
index 578e881067a5..cf881907b4b3 100644
--- a/drivers/mfd/da9052-i2c.c
+++ b/drivers/mfd/da9052-i2c.c
@@ -13,7 +13,7 @@
*/
#include <linux/device.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/input.h>
#include <linux/mfd/core.h>
#include <linux/i2c.h>
@@ -177,20 +177,12 @@ static int da9052_i2c_probe(struct i2c_client *client,
return da9052_device_init(da9052, id->driver_data);
}
-static int da9052_i2c_remove(struct i2c_client *client)
-{
- struct da9052 *da9052 = i2c_get_clientdata(client);
-
- da9052_device_exit(da9052);
- return 0;
-}
-
static struct i2c_driver da9052_i2c_driver = {
.probe = da9052_i2c_probe,
- .remove = da9052_i2c_remove,
.id_table = da9052_i2c_id,
.driver = {
.name = "da9052",
+ .suppress_bind_attrs = true,
#ifdef CONFIG_OF
.of_match_table = dialog_dt_ids,
#endif
@@ -210,13 +202,3 @@ static int __init da9052_i2c_init(void)
return 0;
}
subsys_initcall(da9052_i2c_init);
-
-static void __exit da9052_i2c_exit(void)
-{
- i2c_del_driver(&da9052_i2c_driver);
-}
-module_exit(da9052_i2c_exit);
-
-MODULE_AUTHOR("David Dajun Chen <[email protected]>");
-MODULE_DESCRIPTION("I2C driver for Dialog DA9052 PMIC");
-MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/da9052-irq.c b/drivers/mfd/da9052-irq.c
index cd4ca849ca44..6e0db46f71a9 100644
--- a/drivers/mfd/da9052-irq.c
+++ b/drivers/mfd/da9052-irq.c
@@ -20,7 +20,6 @@
#include <linux/irq.h>
#include <linux/irqdomain.h>
#include <linux/slab.h>
-#include <linux/module.h>
#include <linux/mfd/da9052/da9052.h>
#include <linux/mfd/da9052/reg.h>
diff --git a/drivers/mfd/da9052-spi.c b/drivers/mfd/da9052-spi.c
index b9ea1b27db64..322f31f5c522 100644
--- a/drivers/mfd/da9052-spi.c
+++ b/drivers/mfd/da9052-spi.c
@@ -13,7 +13,7 @@
*/
#include <linux/device.h>
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/input.h>
#include <linux/mfd/core.h>
#include <linux/spi/spi.h>
@@ -59,14 +59,6 @@ static int da9052_spi_probe(struct spi_device *spi)
return da9052_device_init(da9052, id->driver_data);
}
-static int da9052_spi_remove(struct spi_device *spi)
-{
- struct da9052 *da9052 = spi_get_drvdata(spi);
-
- da9052_device_exit(da9052);
- return 0;
-}
-
static struct spi_device_id da9052_spi_id[] = {
{"da9052", DA9052},
{"da9053-aa", DA9053_AA},
@@ -78,10 +70,10 @@ static struct spi_device_id da9052_spi_id[] = {
static struct spi_driver da9052_spi_driver = {
.probe = da9052_spi_probe,
- .remove = da9052_spi_remove,
.id_table = da9052_spi_id,
.driver = {
.name = "da9052",
+ .suppress_bind_attrs = true,
},
};
@@ -98,13 +90,3 @@ static int __init da9052_spi_init(void)
return 0;
}
subsys_initcall(da9052_spi_init);
-
-static void __exit da9052_spi_exit(void)
-{
- spi_unregister_driver(&da9052_spi_driver);
-}
-module_exit(da9052_spi_exit);
-
-MODULE_AUTHOR("David Dajun Chen <[email protected]>");
-MODULE_DESCRIPTION("SPI driver for Dialog DA9052 PMIC");
-MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/da9052/da9052.h b/include/linux/mfd/da9052/da9052.h
index ce9230af09c2..a5857f9fccab 100644
--- a/include/linux/mfd/da9052/da9052.h
+++ b/include/linux/mfd/da9052/da9052.h
@@ -209,7 +209,6 @@ static inline int da9052_reg_update(struct da9052 *da9052, unsigned char reg,
}
int da9052_device_init(struct da9052 *da9052, u8 chip_id);
-void da9052_device_exit(struct da9052 *da9052);
extern const struct regmap_config da9052_regmap_config;
--
2.11.0
The Kconfig currently controlling compilation of this code is:
drivers/mfd/Kconfig:config MFD_DA9055
drivers/mfd/Kconfig: bool "Dialog Semiconductor DA9055 PMIC Support"
...meaning that it is never being built as a module.
Remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
We 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.
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: David Dajun Chen <[email protected]>
Signed-off-by: Paul Gortmaker <[email protected]>
---
drivers/mfd/da9055-i2c.c | 24 ++----------------------
1 file changed, 2 insertions(+), 22 deletions(-)
diff --git a/drivers/mfd/da9055-i2c.c b/drivers/mfd/da9055-i2c.c
index b53e100f577c..7c66f82522f0 100644
--- a/drivers/mfd/da9055-i2c.c
+++ b/drivers/mfd/da9055-i2c.c
@@ -11,7 +11,7 @@
*
*/
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/device.h>
#include <linux/i2c.h>
#include <linux/err.h>
@@ -46,15 +46,6 @@ static int da9055_i2c_probe(struct i2c_client *i2c,
return da9055_device_init(da9055);
}
-static int da9055_i2c_remove(struct i2c_client *i2c)
-{
- struct da9055 *da9055 = i2c_get_clientdata(i2c);
-
- da9055_device_exit(da9055);
-
- return 0;
-}
-
/*
* DO NOT change the device Ids. The naming is intentionally specific as both
* the PMIC and CODEC parts of this chip are instantiated separately as I2C
@@ -66,7 +57,6 @@ static struct i2c_device_id da9055_i2c_id[] = {
{"da9055-pmic", 0},
{ }
};
-MODULE_DEVICE_TABLE(i2c, da9055_i2c_id);
static const struct of_device_id da9055_of_match[] = {
{ .compatible = "dlg,da9055-pmic", },
@@ -75,11 +65,11 @@ static const struct of_device_id da9055_of_match[] = {
static struct i2c_driver da9055_i2c_driver = {
.probe = da9055_i2c_probe,
- .remove = da9055_i2c_remove,
.id_table = da9055_i2c_id,
.driver = {
.name = "da9055-pmic",
.of_match_table = of_match_ptr(da9055_of_match),
+ .suppress_bind_attrs = true,
},
};
@@ -96,13 +86,3 @@ static int __init da9055_i2c_init(void)
return 0;
}
subsys_initcall(da9055_i2c_init);
-
-static void __exit da9055_i2c_exit(void)
-{
- i2c_del_driver(&da9055_i2c_driver);
-}
-module_exit(da9055_i2c_exit);
-
-MODULE_AUTHOR("David Dajun Chen <[email protected]>");
-MODULE_DESCRIPTION("I2C driver for Dialog DA9055 PMIC");
-MODULE_LICENSE("GPL");
--
2.11.0
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(-)
diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
index 09f367571c58..78edb0558a0f 100644
--- a/drivers/mfd/da903x.c
+++ b/drivers/mfd/da903x.c
@@ -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,
[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':
>> 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
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: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-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=arm
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 `alpine_msix_middle_domain_alloc':
supp.c:(.text+0xb8): relocation truncated to fit: R_ARM_CALL against symbol `_raw_spin_lock' defined in .spinlock.text section in kernel/built-in.o
supp.c:(.text+0xf0): relocation truncated to fit: R_ARM_CALL against symbol `_raw_spin_unlock' defined in .spinlock.text section in kernel/built-in.o
supp.c:(.text+0x114): relocation truncated to fit: R_ARM_CALL against symbol `_raw_spin_unlock' defined in .spinlock.text section in kernel/built-in.o
supp.c:(.text+0x214): relocation truncated to fit: R_ARM_CALL against symbol `_raw_spin_lock' defined in .spinlock.text section in kernel/built-in.o
supp.c:(.text+0x22c): relocation truncated to fit: R_ARM_CALL against symbol `_raw_spin_unlock' defined in .spinlock.text section in kernel/built-in.o
drivers/built-in.o: In function `alpine_msix_init_domains':
supp.c:(.text+0x2d8): relocation truncated to fit: R_ARM_CALL against symbol `of_irq_find_parent' defined in .text section in drivers/built-in.o
drivers/built-in.o: In function `alpine_msix_init':
supp.c:(.text+0x414): relocation truncated to fit: R_ARM_CALL against symbol `of_address_to_resource' defined in .text section in drivers/built-in.o
supp.c:(.text+0x454): relocation truncated to fit: R_ARM_CALL against symbol `of_property_read_variable_u32_array' defined in .text section in drivers/built-in.o
supp.c:(.text+0x4c4): relocation truncated to fit: R_ARM_CALL against symbol `of_property_read_variable_u32_array' defined in .text section in drivers/built-in.o
drivers/built-in.o: In function `alpine_msix_middle_domain_free':
supp.c:(.text+0x5a4): relocation truncated to fit: R_ARM_CALL against symbol `_raw_spin_lock' defined in .spinlock.text section in kernel/built-in.o
supp.c:(.text+0x5bc): additional relocation overflows omitted from the output
drivers/built-in.o: In function `da9055_i2c_remove':
>> supp.c:(.text+0xa29d98): undefined reference to `da9055_device_exit'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Paul,
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
> 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(-)
>
> diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
> index 09f367571c58..78edb0558a0f 100644
> --- a/drivers/mfd/da903x.c
> +++ b/drivers/mfd/da903x.c
> @@ -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
On Sat, 03 Jun 2017, 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.
>
> 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.
Do this device resides only on platforms which cannot be shut down,
right?
> 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(-)
>
> diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
> index 09f367571c58..78edb0558a0f 100644
> --- a/drivers/mfd/da903x.c
> +++ b/drivers/mfd/da903x.c
> @@ -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");
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
[RE: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular] On 05/06/2017 (Mon 10:30) Steve Twiss wrote:
> Hi Paul,
>
> 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?
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".
Thanks,
Paul.
--
>
> Previously, something similar was suggested by Geert Uytterhoeven for DA9063:
> https://patchwork.kernel.org/patch/7788001/
>
> Regards,
> Steve
>
> > 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(-)
> >
> > diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
> > index 09f367571c58..78edb0558a0f 100644
> > --- a/drivers/mfd/da903x.c
> > +++ b/drivers/mfd/da903x.c
> > @@ -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
>
[Re: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular] On 05/06/2017 (Mon 11:36) Lee Jones wrote:
> On Sat, 03 Jun 2017, 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.
> >
> > 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.
>
> Do this device resides only on platforms which cannot be shut down,
> right?
Looking at the spec, the chip is handling power routing (and audio) so
it would seem that if your platform has this chip, you'd always need
this code loaded and present.
Paul.
--
>
> > 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(-)
> >
> > diff --git a/drivers/mfd/da903x.c b/drivers/mfd/da903x.c
> > index 09f367571c58..78edb0558a0f 100644
> > --- a/drivers/mfd/da903x.c
> > +++ b/drivers/mfd/da903x.c
> > @@ -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");
>
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog
[Re: [PATCH 3/4] mfd: da9055-core: make it explicitly non-modular] On 04/06/2017 (Sun 13:51) kbuild test robot wrote:
> 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.
Thanks -- I'll simply squish #3 and #4 together for v2.
Paul.
--
>
> All errors (new ones prefixed by >>):
>
> drivers/built-in.o: In function `da9055_i2c_remove':
> >> 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
Hi Paul,
On 05 June 2017 20:30, Paul Gortmaker wrote:
> Subject: Re: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular
>
> [RE: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular]
> On 05/06/2017 (Mon 10:30) Steve Twiss wrote:
>
> > 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
> >
> > 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.
> 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
[RE: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular] On 06/06/2017 (Tue 15:27) Steve Twiss wrote:
> Hi Paul,
>
> On 05 June 2017 20:30, Paul Gortmaker wrote:
>
> > Subject: Re: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular
> >
> > [RE: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular]
> > On 05/06/2017 (Mon 10:30) Steve Twiss wrote:
> >
> > > 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
> > >
> > > 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.
That was my fault; looking back I see I neglected to glue all the Cc:
into the 0/N e-mail like I normally do - sorry about that.
>
> > 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.
> 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.
Thanks,
Paul.
--
>
> Regards,
> Steve
Hi Paul,
On 06 June 2017 21:23, Paul Gortmaker wrote:
> Subject: Re: [PATCH 1/4] mfd: da903x: Make it explicitly non-modular
>
> > > 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.
>
> > 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.
Sure, agreed.
The responsibility can fall back to Dialog for this change. We will submit
for these "Make it explicitly non-modular" patches and remove the ambiguity
caused by the Kconfig bool options.
Regards,
Steve