[RFC/RFT PATCH 0/2] mfd: max14577: Allow the driver to be built as a module

[RFC/RFT PATCH 0/2] mfd: max14577: Allow the driver to be built as a module

[Javier Martinez Canillas]

Hello,

This series is similar to [0] and allows the max14577 PMIC MFD driver to
be built as a module. Currently the Kconfig symbol for the driver is a
boolean but there isn't really a reason for this restriction.

The patches have been just built tested because I don't have any of the
boards using this driver, so testing will be highly appreciated.

[0]: https://lkml.org/lkml/2016/2/11/857

Best regards,
Javier


Javier Martinez Canillas (2):
  mfd: max14577: Use module_init() instead of subsys_initcall()
  mfd: max14577: Allow driver to be built as a module

 drivers/mfd/Kconfig    | 4 ++--
 drivers/mfd/max14577.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.5.0

[RFC/RFT PATCH 1/2] mfd: max14577: Use module_init() instead of subsys_initcall()

[Javier Martinez Canillas]

The driver's init function is called at subsys init call level but the
dependencies provided by the driver are looked up by drivers that have
probe deferral support, so manual ordering of init calls isn't needed.

Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---
Hello,

I checked an the only users in mainline for this driver are the Exynos3250
Monk and Rinato boards. In both, only two regulators are used (safeout_reg
and motor_reg) and these are looked up by the drivers phy-samsung-usb2 and
regulator-haptic respectively, and both support probe deferral.

Best regards,
Javier

 drivers/mfd/max14577.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/max14577.c b/drivers/mfd/max14577.c
index 2280b3fdcf68..6c245128ab2e 100644
--- a/drivers/mfd/max14577.c
+++ b/drivers/mfd/max14577.c
@@ -561,7 +561,7 @@ static int __init max14577_i2c_init(void)
 
 	return i2c_add_driver(&max14577_i2c_driver);
 }
-subsys_initcall(max14577_i2c_init);
+module_init(max14577_i2c_init);
 
 static void __exit max14577_i2c_exit(void)
 {
-- 
2.5.0

[RFC/RFT PATCH 2/2] mfd: max14577: Allow driver to be built as a module

[Javier Martinez Canillas]

The driver's Kconfig symbol is a boolean but nothing prevents the driver
to be built as a module instead of built-in. It is true that most system
integrators will choose the latter but the config should not restrict it.

Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>

---

 drivers/mfd/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index eea61e349e26..be0ff820621b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -514,8 +514,8 @@ config MFD_88PM860X
 	  battery-charger under the corresponding menus.
 
 config MFD_MAX14577
-	bool "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
-	depends on I2C=y
+	tristate "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
+	depends on I2C
 	select MFD_CORE
 	select REGMAP_I2C
 	select REGMAP_IRQ
-- 
2.5.0

Re: [RFC/RFT PATCH 0/2] mfd: max14577: Allow the driver to be built as a module

[Krzysztof Kozlowski]

On 17.03.2016 01:48, Javier Martinez Canillas wrote:
> Hello,
> 
> This series is similar to [0] and allows the max14577 PMIC MFD driver to
> be built as a module. Currently the Kconfig symbol for the driver is a
> boolean but there isn't really a reason for this restriction.
> 
> The patches have been just built tested because I don't have any of the
> boards using this driver, so testing will be highly appreciated.
>

Thanks for the patch (and checking of regulator users) but one of the
reasons of putting this item on todo list was to check and test. The
change itself is trivial, I could do it on myself, right? :) But the
point is that it needs some testing...

BR,
Krzysztof

Re: [RFC/RFT PATCH 2/2] mfd: max14577: Allow driver to be built as a module

[Chanwoo Choi]

On 2016년 03월 17일 01:48, Javier Martinez Canillas wrote:
> The driver's Kconfig symbol is a boolean but nothing prevents the driver
> to be built as a module instead of built-in. It is true that most system
> integrators will choose the latter but the config should not restrict it.
> 
> Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
> 
> ---
> 
>  drivers/mfd/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index eea61e349e26..be0ff820621b 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -514,8 +514,8 @@ config MFD_88PM860X
>  	  battery-charger under the corresponding menus.
>  
>  config MFD_MAX14577
> -	bool "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
> -	depends on I2C=y
> +	tristate "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
> +	depends on I2C
>  	select MFD_CORE
>  	select REGMAP_I2C
>  	select REGMAP_IRQ
> 

When I test the kernel build with these patch-set on next-20160316 tag,
the following errors happen. 

ERROR: "maxim_charger_calc_reg_current" [drivers/regulator/max14577.ko] undefined!
ERROR: "maxim_charger_currents" [drivers/regulator/max14577.ko] undefined!
ERROR: "maxim_charger_currents" [drivers/power/max14577_charger.ko] undefined!
ERROR: "maxim_charger_calc_reg_current" [drivers/power/max14577_charger.ko] undefined!

Best Regards,
Chanwoo Choi

Re: [RFC/RFT PATCH 0/2] mfd: max14577: Allow the driver to be built as a module

[Javier Martinez Canillas]

Hello Krzysztof,

On 03/16/2016 10:09 PM, Krzysztof Kozlowski wrote:
> On 17.03.2016 01:48, Javier Martinez Canillas wrote:
>> Hello,
>>
>> This series is similar to [0] and allows the max14577 PMIC MFD driver to
>> be built as a module. Currently the Kconfig symbol for the driver is a
>> boolean but there isn't really a reason for this restriction.
>>
>> The patches have been just built tested because I don't have any of the
>> boards using this driver, so testing will be highly appreciated.
>>
> 
> Thanks for the patch (and checking of regulator users) but one of the
> reasons of putting this item on todo list was to check and test. The
> change itself is trivial, I could do it on myself, right? :) But the
> point is that it needs some testing...
>

Well, IMHO there is some value in posting a patch even if not tested
(provided that you make that clear like I did) since it is easier to
test a patch than write and test it ;)
 
> BR,
> Krzysztof
> 

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

Re: [RFC/RFT PATCH 2/2] mfd: max14577: Allow driver to be built as a module

[Javier Martinez Canillas]

Hello Chanwoo,

On 03/16/2016 10:58 PM, Chanwoo Choi wrote:
> On 2016년 03월 17일 01:48, Javier Martinez Canillas wrote:
>> The driver's Kconfig symbol is a boolean but nothing prevents the driver
>> to be built as a module instead of built-in. It is true that most system
>> integrators will choose the latter but the config should not restrict it.
>>
>> Suggested-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> ---
>>
>>  drivers/mfd/Kconfig | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> index eea61e349e26..be0ff820621b 100644
>> --- a/drivers/mfd/Kconfig
>> +++ b/drivers/mfd/Kconfig
>> @@ -514,8 +514,8 @@ config MFD_88PM860X
>>  	  battery-charger under the corresponding menus.
>>  
>>  config MFD_MAX14577
>> -	bool "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
>> -	depends on I2C=y
>> +	tristate "Maxim Semiconductor MAX14577/77836 MUIC + Charger Support"
>> +	depends on I2C
>>  	select MFD_CORE
>>  	select REGMAP_I2C
>>  	select REGMAP_IRQ
>>
> 
> When I test the kernel build with these patch-set on next-20160316 tag,
> the following errors happen. 
>

You are absolutely right, there was an error on my test script and always
built it as built-in instead of as a module. I'm so sorry about that...

> ERROR: "maxim_charger_calc_reg_current" [drivers/regulator/max14577.ko] undefined!
> ERROR: "maxim_charger_currents" [drivers/regulator/max14577.ko] undefined!
> ERROR: "maxim_charger_currents" [drivers/power/max14577_charger.ko] undefined!
> ERROR: "maxim_charger_calc_reg_current" [drivers/power/max14577_charger.ko] undefined!
> 

This seems to be a latent bug that was exposed by making the max14577 MFD
Kconfig symbol tristate. Since I'm able to reproduce it even without the
patches by enabling the max14577 regulator and power drivers as a module.

These steps reproduce it on just next-20160316 without any other changes:

$ make exynos_defconfig
$ ./scripts/config --module CONFIG_REGULATOR_MAX14577
$ ./scripts/config --module CONFIG_CHARGER_MAX14577
$ make modules_prepare

$ make M=drivers/regulator/
  ...
  CC [M]  drivers/regulator//max14577.o
  Building modules, stage 2.
  MODPOST 1 modules
WARNING: "maxim_charger_calc_reg_current" [drivers/regulator//max14577.ko] undefined!
WARNING: "maxim_charger_currents" [drivers/regulator//max14577.ko] undefined!

$ make M=drivers/power
  ...
  CC [M]  drivers/power/max14577_charger.o
  Building modules, stage 2.
  MODPOST 1 modules
WARNING: "maxim_charger_calc_reg_current" [drivers/power/max14577_charger.ko] undefined!
WARNING: "maxim_charger_currents" [drivers/power/max14577_charger.ko] undefined!

Now, from a quick look the functions have EXPORT_SYMBOL_GPL() and the
function prototype declaration is in include/linux/mfd/max14577.h so
that should work AFAICT...

I'll take a look to this since it has to be fixed before the other patches.

> Best Regards,
> Chanwoo Choi
> 

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

Re: [RFC/RFT PATCH 2/2] mfd: max14577: Allow driver to be built as a module

[Javier Martinez Canillas]

Hello Chanwoo,

On 03/17/2016 12:37 PM, Javier Martinez Canillas wrote:
> 
> On 03/16/2016 10:58 PM, Chanwoo Choi wrote:
>> On 2016년 03월 17일 01:48, Javier Martinez Canillas wrote:

[snip]

>>>
>>
>> When I test the kernel build with these patch-set on next-20160316 tag,
>> the following errors happen. 
>>
> 
> You are absolutely right, there was an error on my test script and always
> built it as built-in instead of as a module. I'm so sorry about that...
> 
>> ERROR: "maxim_charger_calc_reg_current" [drivers/regulator/max14577.ko] undefined!
>> ERROR: "maxim_charger_currents" [drivers/regulator/max14577.ko] undefined!
>> ERROR: "maxim_charger_currents" [drivers/power/max14577_charger.ko] undefined!
>> ERROR: "maxim_charger_calc_reg_current" [drivers/power/max14577_charger.ko] undefined!
>>
> 
> This seems to be a latent bug that was exposed by making the max14577 MFD
> Kconfig symbol tristate. Since I'm able to reproduce it even without the
> patches by enabling the max14577 regulator and power drivers as a module.
>

Sorry again, this error goes away after I clean the build directory so it
does not happen without this patch-set. Now, I found what's the issue and
is that both the max14577 MFD and regulator drivers have the same object
file name so they both end being called max14577.ko.

This confuses Kbuild and so in the modpost step, the exported symbols by
the MFD driver don't end into Module.symvers. This doesn't happen when
the driver is not a module since symbols come from the vmlinux binary.

I'll post a patch to rename the regulator driver to max14577-regulator,
this will be necessary anyways to have max14577 as a module since Kbuild
also gets confused and don't copy both modules because have the same name.

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

Re: [RFC/RFT PATCH 2/2] mfd: max14577: Allow driver to be built as a module

[Javier Martinez Canillas]

Hello Chanwoo

On 03/17/2016 01:57 PM, Javier Martinez Canillas wrote:

[snip]

>>
> 
> Sorry again, this error goes away after I clean the build directory so it
> does not happen without this patch-set. Now, I found what's the issue and
> is that both the max14577 MFD and regulator drivers have the same object
> file name so they both end being called max14577.ko.
> 
> This confuses Kbuild and so in the modpost step, the exported symbols by
> the MFD driver don't end into Module.symvers. This doesn't happen when
> the driver is not a module since symbols come from the vmlinux binary.
> 
> I'll post a patch to rename the regulator driver to max14577-regulator,
> this will be necessary anyways to have max14577 as a module since Kbuild
> also gets confused and don't copy both modules because have the same name.
> 

Patch is [0], could you please test the patch-series along with that one
and provide your Tested-by? I plan to re-send this series once [0] lands.

> Best regards,
> 

[0]: https://patchwork.kernel.org/patch/8613691/

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

Re: [RFC/RFT PATCH 2/2] mfd: max14577: Allow driver to be built as a module

[Chanwoo Choi]

Hello Javier,

On 2016년 03월 18일 03:02, Javier Martinez Canillas wrote:
> Hello Chanwoo
> 
> On 03/17/2016 01:57 PM, Javier Martinez Canillas wrote:
> 
> [snip]
> 
>>>
>>
>> Sorry again, this error goes away after I clean the build directory so it
>> does not happen without this patch-set. Now, I found what's the issue and
>> is that both the max14577 MFD and regulator drivers have the same object
>> file name so they both end being called max14577.ko.
>>
>> This confuses Kbuild and so in the modpost step, the exported symbols by
>> the MFD driver don't end into Module.symvers. This doesn't happen when
>> the driver is not a module since symbols come from the vmlinux binary.
>>
>> I'll post a patch to rename the regulator driver to max14577-regulator,
>> this will be necessary anyways to have max14577 as a module since Kbuild
>> also gets confused and don't copy both modules because have the same name.
>>
> 
> Patch is [0], could you please test the patch-series along with that one
> and provide your Tested-by? I plan to re-send this series once [0] lands.

I'll test it on next week and then reply.

Best Regards,
Chanwoo Choi

> 
>> Best regards,
>>
> 
> [0]: https://patchwork.kernel.org/patch/8613691/
> 
> Best regards,
> 

Re: [RFC/RFT PATCH 2/2] mfd: max14577: Allow driver to be built as a module

[Javier Martinez Canillas]

Hello Chanwoo,

On 03/18/2016 04:55 AM, Chanwoo Choi wrote:
> Hello Javier,
> 
> On 2016년 03월 18일 03:02, Javier Martinez Canillas wrote:
>> Hello Chanwoo
>>
>> On 03/17/2016 01:57 PM, Javier Martinez Canillas wrote:
>>
>> [snip]
>>
>>>>
>>>
>>> Sorry again, this error goes away after I clean the build directory so it
>>> does not happen without this patch-set. Now, I found what's the issue and
>>> is that both the max14577 MFD and regulator drivers have the same object
>>> file name so they both end being called max14577.ko.
>>>
>>> This confuses Kbuild and so in the modpost step, the exported symbols by
>>> the MFD driver don't end into Module.symvers. This doesn't happen when
>>> the driver is not a module since symbols come from the vmlinux binary.
>>>
>>> I'll post a patch to rename the regulator driver to max14577-regulator,
>>> this will be necessary anyways to have max14577 as a module since Kbuild
>>> also gets confused and don't copy both modules because have the same name.
>>>
>>
>> Patch is [0], could you please test the patch-series along with that one
>> and provide your Tested-by? I plan to re-send this series once [0] lands.
> 
> I'll test it on next week and then reply.
>

Perfect, thanks a lot for your help!
 
> Best Regards,
> Chanwoo Choi
> 
>>

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