Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1370290 > unrolled thread

[PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-04-03 23:30 +0200
    [PATCH 1/7] drivers/mfd: make max8925-i2c.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-04-03 23:30 +0200
    [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-04-03 23:30 +0200
      Re: [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-04-04 05:50 +0200
        Re: [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-04-04 17:10 +0200
        Re: [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular Javier Martinez Canillas <javier@osg.samsung.com> - 2016-04-05 16:30 +0200
    Re: [PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular Lee Jones <lee.jones@linaro.org> - 2016-04-04 10:20 +0200
      Re: [PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular Paul Gortmaker <paul.gortmaker@windriver.com> - 2016-04-04 16:00 +0200
        Re: [PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular Lee Jones <lee.jones@linaro.org> - 2016-04-04 16:30 +0200

#1370290 — [PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular

For anyone new to the underlying goal of this cleanup, we are trying 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 other 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 spreads like weeds.

There are quite a few in the mfd space, so rather than send a large
series, I'll try and batch them up into reasonable sized queues.  To that
end, here we fix up all the maxim drivers as the 1st mfd batch.

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
the existing Kconfig/Makefile settings that restrict them to "bool".

Build tested on linux-next for arm, arm64 and x86-64 to ensure no typos
or similar issues crept in.

Paul.
---

Cc: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>

Paul Gortmaker (7):
  drivers/mfd: make max8925-i2c.c explicitly non-modular
  drivers/mfd: make max8997.c explicitly non-modular
  drivers/mfd: make max8998.c explicitly non-modular
  drivers/mfd: make max14577.c explicitly non-modular
  drivers/mfd: make max77686.c explicitly non-modular
  drivers/mfd: make max77693.c explicitly non-modular
  drivers/mfd: make max77843.c explicitly non-modular

 drivers/mfd/max14577.c    | 13 +------------
 drivers/mfd/max77686.c    | 26 ++------------------------
 drivers/mfd/max77693.c    | 34 +++-------------------------------
 drivers/mfd/max77843.c    | 24 ++----------------------
 drivers/mfd/max8925-i2c.c | 14 +-------------
 drivers/mfd/max8997.c     | 30 +++---------------------------
 drivers/mfd/max8998.c     | 27 +--------------------------
 7 files changed, 13 insertions(+), 155 deletions(-)

-- 
2.8.0

[toc] | [next] | [standalone]

#1370291 — [PATCH 1/7] drivers/mfd: make max8925-i2c.c explicitly non-modular

The Kconfig currently controlling compilation of this code is:

drivers/mfd/Kconfig: config MFD_MAX8925
drivers/mfd/Kconfig:    bool "Maxim Semiconductor MAX8925 PMIC Support"

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

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

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: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/max8925-i2c.c | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/drivers/mfd/max8925-i2c.c b/drivers/mfd/max8925-i2c.c
index 70443b161a5b..5c80aea3211f 100644
--- a/drivers/mfd/max8925-i2c.c
+++ b/drivers/mfd/max8925-i2c.c
@@ -9,7 +9,7 @@
  * published by the Free Software Foundation.
  */
 #include <linux/kernel.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
 #include <linux/mfd/max8925.h>
@@ -133,7 +133,6 @@ static const struct i2c_device_id max8925_id_table[] = {
 	{ "max8925", 0 },
 	{ },
 };
-MODULE_DEVICE_TABLE(i2c, max8925_id_table);
 
 static int max8925_dt_init(struct device_node *np, struct device *dev,
 			   struct max8925_platform_data *pdata)
@@ -240,7 +239,6 @@ static const struct of_device_id max8925_dt_ids[] = {
 	{ .compatible = "maxim,max8925", },
 	{},
 };
-MODULE_DEVICE_TABLE(of, max8925_dt_ids);
 
 static struct i2c_driver max8925_driver = {
 	.driver	= {
@@ -264,13 +262,3 @@ static int __init max8925_i2c_init(void)
 	return ret;
 }
 subsys_initcall(max8925_i2c_init);
-
-static void __exit max8925_i2c_exit(void)
-{
-	i2c_del_driver(&max8925_driver);
-}
-module_exit(max8925_i2c_exit);
-
-MODULE_DESCRIPTION("I2C Driver for Maxim 8925");
-MODULE_AUTHOR("Haojian Zhuang <haojian.zhuang@marvell.com>");
-MODULE_LICENSE("GPL");
-- 
2.8.0

[toc] | [prev] | [next] | [standalone]

#1370292 — [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular

The Kconfig currently controlling compilation of this code is:

mfd/Kconfig:config MFD_MAX14577
mfd/Kconfig:   bool "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"

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

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

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: Chanwoo Choi <cw00.choi@samsung.com>
Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/mfd/max14577.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index 2280b3fdcf68..9c0eb4efe246 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -19,7 +19,7 @@
  */
 
 #include <linux/err.h>
-#include <linux/module.h>
+#include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/of_device.h>
 #include <linux/mfd/core.h>
@@ -490,7 +490,6 @@ static const struct i2c_device_id max14577_i2c_id[] = {
 	{ "max77836", MAXIM_DEVICE_TYPE_MAX77836, },
 	{ }
 };
-MODULE_DEVICE_TABLE(i2c, max14577_i2c_id);
 
 #ifdef CONFIG_PM_SLEEP
 static int max14577_suspend(struct device *dev)
@@ -562,13 +561,3 @@ static int __init max14577_i2c_init(void)
 	return i2c_add_driver(&max14577_i2c_driver);
 }
 subsys_initcall(max14577_i2c_init);
-
-static void __exit max14577_i2c_exit(void)
-{
-	i2c_del_driver(&max14577_i2c_driver);
-}
-module_exit(max14577_i2c_exit);
-
-MODULE_AUTHOR("Chanwoo Choi <cw00.choi@samsung.com>, Krzysztof Kozlowski <k.kozlowski@samsung.com>");
-MODULE_DESCRIPTION("Maxim 14577/77836 multi-function core driver");
-MODULE_LICENSE("GPL");
-- 
2.8.0

[toc] | [prev] | [next] | [standalone]

#1370335 — Re: [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular

On 04.04.2016 06:24, Paul Gortmaker wrote:
> The Kconfig currently controlling compilation of this code is:
> 
> mfd/Kconfig:config MFD_MAX14577
> mfd/Kconfig:   bool "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
> 
> ...meaning that it currently is not being built as a module by anyone.
> 
> Lets remove the modular code that is essentially orphaned, so that
> when reading the driver there is no doubt it is builtin-only.
> 
> 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: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
>  drivers/mfd/max14577.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)

+Cc Javier,

In general I agree with the patchset except maybe here because it
conflicts with Javier's work to make it a module:
mfd: max14577: Use module_init() instead of subsys_initcall()
http://www.spinics.net/lists/kernel/msg2213747.html

Javier's patchset is however missing a tested-by tag so maybe this is
why it was not merged.

Unfortunately I cannot provide testing because my current device with
max14577/max77836 is dead and its recovery is not easy. Maybe I will get
a proper device in a few weeks after going back to Poland.

My proposal would be to wait with this max14577 change a little bit.

Best regards,
Krzysztof

[toc] | [prev] | [next] | [standalone]

#1370680 — Re: [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular

[Re: [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular] On 04/04/2016 (Mon 12:42) Krzysztof Kozlowski wrote:

> On 04.04.2016 06:24, Paul Gortmaker wrote:
> > The Kconfig currently controlling compilation of this code is:
> > 
> > mfd/Kconfig:config MFD_MAX14577
> > mfd/Kconfig:   bool "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
> > 
> > ...meaning that it currently is not being built as a module by anyone.

[...]

> > ---
> >  drivers/mfd/max14577.c | 13 +------------
> >  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> +Cc Javier,
> 
> In general I agree with the patchset except maybe here because it
> conflicts with Javier's work to make it a module:
> mfd: max14577: Use module_init() instead of subsys_initcall()
> http://www.spinics.net/lists/kernel/msg2213747.html

No problem, I'll shelf this one as well and assume it is on the
way to being tristate.

Thanks,
Paul.
--

> 
> Javier's patchset is however missing a tested-by tag so maybe this is
> why it was not merged.
> 
> Unfortunately I cannot provide testing because my current device with
> max14577/max77836 is dead and its recovery is not easy. Maybe I will get
> a proper device in a few weeks after going back to Poland.
> 
> My proposal would be to wait with this max14577 change a little bit.
> 
> Best regards,
> Krzysztof

[toc] | [prev] | [next] | [standalone]

#1371668 — Re: [PATCH 4/7] drivers/mfd: make max14577.c explicitly non-modular

Hello Krzysztof,

On 04/03/2016 11:42 PM, Krzysztof Kozlowski wrote:
> On 04.04.2016 06:24, Paul Gortmaker wrote:
>> The Kconfig currently controlling compilation of this code is:
>>
>> mfd/Kconfig:config MFD_MAX14577
>> mfd/Kconfig:   bool "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
>>
>> ...meaning that it currently is not being built as a module by anyone.
>>
>> Lets remove the modular code that is essentially orphaned, so that
>> when reading the driver there is no doubt it is builtin-only.
>>
>> 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: Chanwoo Choi <cw00.choi@samsung.com>
>> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Cc: Samuel Ortiz <sameo@linux.intel.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>> ---
>>  drivers/mfd/max14577.c | 13 +------------
>>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> +Cc Javier,
> 
> In general I agree with the patchset except maybe here because it
> conflicts with Javier's work to make it a module:
> mfd: max14577: Use module_init() instead of subsys_initcall()
> http://www.spinics.net/lists/kernel/msg2213747.html
> 
> Javier's patchset is however missing a tested-by tag so maybe this is
> why it was not merged.
> 
> Unfortunately I cannot provide testing because my current device with
> max14577/max77836 is dead and its recovery is not easy. Maybe I will get
> a proper device in a few weeks after going back to Poland.
> 
> My proposal would be to wait with this max14577 change a little bit.
> 

Chanwoo said he should be able to test: https://lkml.org/lkml/2016/3/18/78
So hopefully I should have some Tested-by tags for that series soon.

> Best regards,
> Krzysztof
> 

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [next] | [standalone]

#1370431

How did you come up with the subject lines for these patches?

Please do `git log --oneline -- <subsystem>`, as is normal when
submitting to mainline.

> For anyone new to the underlying goal of this cleanup, we are trying 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 other 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 spreads like weeds.
> 
> There are quite a few in the mfd space, so rather than send a large
> series, I'll try and batch them up into reasonable sized queues.  To that
> end, here we fix up all the maxim drivers as the 1st mfd batch.
> 
> 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
> the existing Kconfig/Makefile settings that restrict them to "bool".
> 
> Build tested on linux-next for arm, arm64 and x86-64 to ensure no typos
> or similar issues crept in.
> 
> Paul.
> ---
> 
> Cc: Chanwoo Choi <cw00.choi@samsung.com>
> Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
> Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> Cc: Samuel Ortiz <sameo@linux.intel.com>
> 
> Paul Gortmaker (7):
>   drivers/mfd: make max8925-i2c.c explicitly non-modular
>   drivers/mfd: make max8997.c explicitly non-modular
>   drivers/mfd: make max8998.c explicitly non-modular
>   drivers/mfd: make max14577.c explicitly non-modular
>   drivers/mfd: make max77686.c explicitly non-modular
>   drivers/mfd: make max77693.c explicitly non-modular
>   drivers/mfd: make max77843.c explicitly non-modular
> 
>  drivers/mfd/max14577.c    | 13 +------------
>  drivers/mfd/max77686.c    | 26 ++------------------------
>  drivers/mfd/max77693.c    | 34 +++-------------------------------
>  drivers/mfd/max77843.c    | 24 ++----------------------
>  drivers/mfd/max8925-i2c.c | 14 +-------------
>  drivers/mfd/max8997.c     | 30 +++---------------------------
>  drivers/mfd/max8998.c     | 27 +--------------------------
>  7 files changed, 13 insertions(+), 155 deletions(-)
> 

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

[toc] | [prev] | [next] | [standalone]

#1370637

[Re: [PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular] On 04/04/2016 (Mon 09:11) Lee Jones wrote:

> How did you come up with the subject lines for these patches?

I just used the general default of:
   <path_to_subsystem>: rest of subject

> 
> Please do `git log --oneline -- <subsystem>`, as is normal when
> submitting to mainline.

Just to be clear, looking at other mfd history, you want me to just
drop the "drivers/" prefix ; i.e. change from:

   drivers/mfd: text text text driver.c text

...to:

   mfd: text text text driver.c text

If so, that is no problem and I can do that for the next revision when I
drop the patches pending tristate conversion that I wasn't aware of.

Thanks,
Paul.
--

> 
> > For anyone new to the underlying goal of this cleanup, we are trying 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 other 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 spreads like weeds.
> > 
> > There are quite a few in the mfd space, so rather than send a large
> > series, I'll try and batch them up into reasonable sized queues.  To that
> > end, here we fix up all the maxim drivers as the 1st mfd batch.
> > 
> > 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
> > the existing Kconfig/Makefile settings that restrict them to "bool".
> > 
> > Build tested on linux-next for arm, arm64 and x86-64 to ensure no typos
> > or similar issues crept in.
> > 
> > Paul.
> > ---
> > 
> > Cc: Chanwoo Choi <cw00.choi@samsung.com>
> > Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
> > Cc: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: MyungJoo Ham <myungjoo.ham@samsung.com>
> > Cc: Samuel Ortiz <sameo@linux.intel.com>
> > 
> > Paul Gortmaker (7):
> >   drivers/mfd: make max8925-i2c.c explicitly non-modular
> >   drivers/mfd: make max8997.c explicitly non-modular
> >   drivers/mfd: make max8998.c explicitly non-modular
> >   drivers/mfd: make max14577.c explicitly non-modular
> >   drivers/mfd: make max77686.c explicitly non-modular
> >   drivers/mfd: make max77693.c explicitly non-modular
> >   drivers/mfd: make max77843.c explicitly non-modular
> > 
> >  drivers/mfd/max14577.c    | 13 +------------
> >  drivers/mfd/max77686.c    | 26 ++------------------------
> >  drivers/mfd/max77693.c    | 34 +++-------------------------------
> >  drivers/mfd/max77843.c    | 24 ++----------------------
> >  drivers/mfd/max8925-i2c.c | 14 +-------------
> >  drivers/mfd/max8997.c     | 30 +++---------------------------
> >  drivers/mfd/max8998.c     | 27 +--------------------------
> >  7 files changed, 13 insertions(+), 155 deletions(-)
> > 
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1370663

On Mon, 04 Apr 2016, Paul Gortmaker wrote:

> [Re: [PATCH 0/7] drivers/mfd: make max drivers explicitly non-modular] On 04/04/2016 (Mon 09:11) Lee Jones wrote:
> 
> > How did you come up with the subject lines for these patches?
> 
> I just used the general default of:
>    <path_to_subsystem>: rest of subject
> 
> > 
> > Please do `git log --oneline -- <subsystem>`, as is normal when
> > submitting to mainline.
> 
> Just to be clear, looking at other mfd history, you want me to just
> drop the "drivers/" prefix ; i.e. change from:
> 
>    drivers/mfd: text text text driver.c text
> 
> ...to:
> 
>    mfd: text text text driver.c text
> 
> If so, that is no problem and I can do that for the next revision when I
> drop the patches pending tristate conversion that I wasn't aware of.

Current subsystem format is:

 mfd: sub-driver-name: Thing you're doing

Sub-driver-name doesn't require file extensions and 'Think that you're
doing" should start with an uppercase character.

(Royal-)We like straight lines and conformity. ;)

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web