[PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[pramod.gurav.etc]

From: Pramod Gurav <pramod.gurav.etc@gmail.com>

REGMAP_SPMI module calls some functions from SPMI hence build breaks
when SPMI is not enabled while compiling REGMAP_SPMI with below linker
errors:

drivers/built-in.o: In function `regmap_spmi_ext_read':
:(.text+0x1143ec): undefined reference to `spmi_ext_register_read'
:(.text+0x11443c): undefined reference to `spmi_ext_register_readl'
drivers/built-in.o: In function `regmap_spmi_ext_gather_write':
:(.text+0x1144c4): undefined reference to `spmi_ext_register_write'
:(.text+0x114520): undefined reference to `spmi_ext_register_writel'
drivers/built-in.o: In function `regmap_spmi_base_read':
:(.text+0x1145b8): undefined reference to `spmi_register_read'
drivers/built-in.o: In function `regmap_spmi_base_gather_write':
:(.text+0x114630): undefined reference to `spmi_register_write'
:(.text+0x11465c): undefined reference to `spmi_register_zero_write'

Signed-off-by: Pramod Gurav <pramod.gurav.etc@gmail.com>
CC: Josh Cartwright <joshc@codeaurora.org>
CC: Mark Brown <broonie@linaro.org>
---
This was found when I enabled support for Qualcomm QPNP PMICs and was
compiling it. It selects REGMAP_SPMI and hence the crash.

 drivers/base/regmap/Kconfig |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
index 4251570..1aa9d71 100644
--- a/drivers/base/regmap/Kconfig
+++ b/drivers/base/regmap/Kconfig
@@ -16,6 +16,7 @@ config REGMAP_SPI
 	tristate
 
 config REGMAP_SPMI
+	select SPMI
 	tristate
 
 config REGMAP_MMIO
-- 
1.7.9.5

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Stanimir Varbanov]

Hi,

<snip>
> 
> Signed-off-by: Pramod Gurav <pramod.gurav.etc@gmail.com>
> CC: Josh Cartwright <joshc@codeaurora.org>
> CC: Mark Brown <broonie@linaro.org>
> ---
> This was found when I enabled support for Qualcomm QPNP PMICs and was
> compiling it. It selects REGMAP_SPMI and hence the crash.
> 
>  drivers/base/regmap/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
> index 4251570..1aa9d71 100644
> --- a/drivers/base/regmap/Kconfig
> +++ b/drivers/base/regmap/Kconfig
> @@ -16,6 +16,7 @@ config REGMAP_SPI
>  	tristate
>  
>  config REGMAP_SPMI
> +	select SPMI

NO, IMO the CONFIG_SPMI should be enabled by qcom_defconfig and
multi_v7_defconfig. See CONFIG_I2C and REGMAP_I2C for example.

-- 
regards,
Stan

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Pramod Gurav]

Hi,

On Wednesday, 16 July, 2014 5:37pm, "Stanimir Varbanov" <svarbanov@mm-sol.com> said:

> Hi,
> 
> <snip>
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav.etc@gmail.com>
>> CC: Josh Cartwright <joshc@codeaurora.org>
>> CC: Mark Brown <broonie@linaro.org>
>> ---
>> This was found when I enabled support for Qualcomm QPNP PMICs and was
>> compiling it. It selects REGMAP_SPMI and hence the crash.
>>
>>  drivers/base/regmap/Kconfig |    1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/base/regmap/Kconfig b/drivers/base/regmap/Kconfig
>> index 4251570..1aa9d71 100644
>> --- a/drivers/base/regmap/Kconfig
>> +++ b/drivers/base/regmap/Kconfig
>> @@ -16,6 +16,7 @@ config REGMAP_SPI
>>  	tristate
>>
>>  config REGMAP_SPMI
>> +	select SPMI
> 
> NO, IMO the CONFIG_SPMI should be enabled by qcom_defconfig and
> multi_v7_defconfig. See CONFIG_I2C and REGMAP_I2C for example.
> 

I am using multi_v7_defconfig but its not enabling it. I ran qcom_defconfig which does.
 
> --
> regards,
> Stan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Stanimir Varbanov]

<snip>

>>>
>>>  config REGMAP_SPMI
>>> +	select SPMI
>>
>> NO, IMO the CONFIG_SPMI should be enabled by qcom_defconfig and
>> multi_v7_defconfig. See CONFIG_I2C and REGMAP_I2C for example.
>>
> 
> I am using multi_v7_defconfig but its not enabling it. I ran qcom_defconfig which does.

yes, it seems reasonable to add it in multi_v7_defconfig also.


-- 
regards,
Stan

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Pramod Gurav]

Hi,

On Wednesday, 16 July, 2014 5:55pm, "Stanimir Varbanov" <svarbanov@mm-sol.com> said:

> <snip>
> 
>>>>
>>>>  config REGMAP_SPMI
>>>> +	select SPMI
>>>
>>> NO, IMO the CONFIG_SPMI should be enabled by qcom_defconfig and
>>> multi_v7_defconfig. See CONFIG_I2C and REGMAP_I2C for example.
>>>
>>
>> I am using multi_v7_defconfig but its not enabling it. I ran qcom_defconfig which
>> does.
> 
> yes, it seems reasonable to add it in multi_v7_defconfig also.
> 

Thanks.
I misunderstood the Kconfig documentation which says, "Reverse dependencies can only be used with boolean or tristate symbols". In the note following this statement doc says, "In general use select only for non-visible symbols".

The CONFIG_SPMI option is visible in menuconfig hence either it should be set by default in multi_v7_defconfig(like in qcom_defconfig) or driver owner should mention a 'depneds on CONFIG_SPMI' as suggested by Lars-Peter Clausen. 
I prefer the former (defconfig).

Shall I send a new patch adding it in multi_v7_defconfig?

> 
> --
> regards,
> Stan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Stanimir Varbanov]

<snip>

> 
> Thanks.
> I misunderstood the Kconfig documentation which says, "Reverse dependencies can only be used with boolean or tristate symbols". In the note following this statement doc says, "In general use select only for non-visible symbols".
> 
> The CONFIG_SPMI option is visible in menuconfig hence either it should be set by default in multi_v7_defconfig(like in qcom_defconfig) or driver owner should mention a 'depneds on CONFIG_SPMI' as suggested by Lars-Peter Clausen. 
> I prefer the former (defconfig).
> 
> Shall I send a new patch adding it in multi_v7_defconfig?

yes, I think it should be enabled. On the other side I'm not sure that
it will be acceptable because presently there are no users if it in
multi platform build. Might be worth to do it ones the MFD driver is in.

And I/we should add a dependency to SPMI in the MFD driver as Lars
pointed out.

-- 
regards,
Stan

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

On Wed, Jul 16, 2014 at 06:26:15PM +0530, Pramod Gurav wrote:

Fix your mailer to word wrap within paragraphs, this will make your
mails more legible - see Documentation/email-clients.txt.

> On Wednesday, 16 July, 2014 5:55pm, "Stanimir Varbanov" <svarbanov@mm-sol.com> said:

> The CONFIG_SPMI option is visible in menuconfig hence either it should
> be set by default in multi_v7_defconfig(like in qcom_defconfig) or
> driver owner should mention a 'depneds on CONFIG_SPMI' as suggested by
> Lars-Peter Clausen. 

> I prefer the former (defconfig).

No, this isn't an either/or thing - the dependency is absolutely
mandatory if the device needs SPMI.  The defconfigs are a separate
thing, they just exist to give people a starting point for configuring
their kernel so if the device using SPMI is important for relevant
systems the defconfig needs to be set up to enable it but that's
separate to the dependency since there's no need for people to ever even
look at defconfigs.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Ivan T. Ivanov]

On Wed, 2014-07-16 at 14:53 +0100, Mark Brown wrote:
> On Wed, Jul 16, 2014 at 06:26:15PM +0530, Pramod Gurav wrote:
> 
> Fix your mailer to word wrap within paragraphs, this will make your
> mails more legible - see Documentation/email-clients.txt.
> 
> > On Wednesday, 16 July, 2014 5:55pm, "Stanimir Varbanov" <svarbanov@mm-sol.com> said:
> 
> > The CONFIG_SPMI option is visible in menuconfig hence either it should
> > be set by default in multi_v7_defconfig(like in qcom_defconfig) or
> > driver owner should mention a 'depneds on CONFIG_SPMI' as suggested by
> > Lars-Peter Clausen. 
> 
> > I prefer the former (defconfig).
> 
> No, this isn't an either/or thing - the dependency is absolutely
> mandatory if the device needs SPMI.  The defconfigs are a separate
> thing, they just exist to give people a starting point for configuring
> their kernel so if the device using SPMI is important for relevant
> systems the defconfig needs to be set up to enable it but that's
> separate to the dependency since there's no need for people to ever even
> look at defconfigs.

Then config REGMAP_SPMI should depend on SPMI, right?

Regards,
Ivan

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 907 bytes --]

On Wed, Jul 16, 2014 at 05:00:54PM +0300, Ivan T. Ivanov wrote:
> On Wed, 2014-07-16 at 14:53 +0100, Mark Brown wrote:

> > No, this isn't an either/or thing - the dependency is absolutely
> > mandatory if the device needs SPMI.  The defconfigs are a separate
> > thing, they just exist to give people a starting point for configuring
> > their kernel so if the device using SPMI is important for relevant
> > systems the defconfig needs to be set up to enable it but that's
> > separate to the dependency since there's no need for people to ever even
> > look at defconfigs.

> Then config REGMAP_SPMI should depend on SPMI, right?

No, REGMAP_SPMI is not something that can be enabled directly - it is
selected by its users which should be ensuring that SPMI is enabled as
it is difficult to see a use case for REGMAP_SPMI which does not do
this.

A dependency from a selected symbol will have no effect.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[pramod gurav]

On Wed, Jul 16, 2014 at 7:48 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Jul 16, 2014 at 05:00:54PM +0300, Ivan T. Ivanov wrote:
>> On Wed, 2014-07-16 at 14:53 +0100, Mark Brown wrote:
>
>> > No, this isn't an either/or thing - the dependency is absolutely
>> > mandatory if the device needs SPMI.  The defconfigs are a separate
>> > thing, they just exist to give people a starting point for configuring
>> > their kernel so if the device using SPMI is important for relevant
>> > systems the defconfig needs to be set up to enable it but that's
>> > separate to the dependency since there's no need for people to ever even
>> > look at defconfigs.
>
>> Then config REGMAP_SPMI should depend on SPMI, right?
>
> No, REGMAP_SPMI is not something that can be enabled directly - it is
> selected by its users which should be ensuring that SPMI is enabled as
> it is difficult to see a use case for REGMAP_SPMI which does not do
> this.
>
> A dependency from a selected symbol will have no effect.

Thanks Mark. So essentially in this case PMIC driver should 'select SPMI'.
Right?

-- 
Thanks and Regards
Pramod

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Wed, Jul 16, 2014 at 08:12:04PM +0530, pramod gurav wrote:
> On Wed, Jul 16, 2014 at 7:48 PM, Mark Brown <broonie@kernel.org> wrote:

> > A dependency from a selected symbol will have no effect.

> Thanks Mark. So essentially in this case PMIC driver should 'select SPMI'.
> Right?

I would expect it to depend on SPMI and select REGMAP_SPMI, assuming
SPMI is a user-selectable Kconfig symbol.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] regmap: Kconfig: Select SPMI when REGMAP_SPMI is selected

[Lars-Peter Clausen]

On 07/16/2014 01:39 PM, pramod.gurav.etc@gmail.com wrote:
> From: Pramod Gurav <pramod.gurav.etc@gmail.com>
>
> REGMAP_SPMI module calls some functions from SPMI hence build breaks
> when SPMI is not enabled while compiling REGMAP_SPMI with below linker
> errors:
>
> drivers/built-in.o: In function `regmap_spmi_ext_read':
> :(.text+0x1143ec): undefined reference to `spmi_ext_register_read'
> :(.text+0x11443c): undefined reference to `spmi_ext_register_readl'
> drivers/built-in.o: In function `regmap_spmi_ext_gather_write':
> :(.text+0x1144c4): undefined reference to `spmi_ext_register_write'
> :(.text+0x114520): undefined reference to `spmi_ext_register_writel'
> drivers/built-in.o: In function `regmap_spmi_base_read':
> :(.text+0x1145b8): undefined reference to `spmi_register_read'
> drivers/built-in.o: In function `regmap_spmi_base_gather_write':
> :(.text+0x114630): undefined reference to `spmi_register_write'
> :(.text+0x11465c): undefined reference to `spmi_register_zero_write'
>
> Signed-off-by: Pramod Gurav <pramod.gurav.etc@gmail.com>
> CC: Josh Cartwright <joshc@codeaurora.org>
> CC: Mark Brown <broonie@linaro.org>
> ---
> This was found when I enabled support for Qualcomm QPNP PMICs and was
> compiling it. It selects REGMAP_SPMI and hence the crash.


Any driver that does select REGMAP_SPMI needs to depends on SPMI. So the 
correct fix for this issue is to make sure that the driver can only be 
enabled if SPMI is enabled.

- Lars