2014-07-16 11:36:37

by Pramod Gurav

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

From: Pramod Gurav <[email protected]>

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 <[email protected]>
CC: Josh Cartwright <[email protected]>
CC: Mark Brown <[email protected]>
---
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


2014-07-16 12:07:14

by Stanimir Varbanov

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

Hi,

<snip>
>
> Signed-off-by: Pramod Gurav <[email protected]>
> CC: Josh Cartwright <[email protected]>
> CC: Mark Brown <[email protected]>
> ---
> 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

2014-07-16 12:14:32

by Pramod Gurav

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

Hi,

On Wednesday, 16 July, 2014 5:37pm, "Stanimir Varbanov" <[email protected]> said:

> Hi,
>
> <snip>
>>
>> Signed-off-by: Pramod Gurav <[email protected]>
>> CC: Josh Cartwright <[email protected]>
>> CC: Mark Brown <[email protected]>
>> ---
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-07-16 12:19:48

by Lars-Peter Clausen

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

On 07/16/2014 01:39 PM, [email protected] wrote:
> From: Pramod Gurav <[email protected]>
>
> 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 <[email protected]>
> CC: Josh Cartwright <[email protected]>
> CC: Mark Brown <[email protected]>
> ---
> 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

2014-07-16 12:25:36

by Stanimir Varbanov

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

<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

2014-07-16 12:56:18

by Pramod Gurav

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

Hi,

On Wednesday, 16 July, 2014 5:55pm, "Stanimir Varbanov" <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2014-07-16 13:53:46

by Stanimir Varbanov

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

<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

2014-07-16 13:54:05

by Mark Brown

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

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" <[email protected]> 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.


Attachments:
(No filename) (981.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-07-16 14:01:20

by Ivan T. Ivanov

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

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" <[email protected]> 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

2014-07-16 14:19:10

by Mark Brown

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

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.


Attachments:
(No filename) (907.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2014-07-16 14:42:08

by Pramod Gurav

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

On Wed, Jul 16, 2014 at 7:48 PM, Mark Brown <[email protected]> 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

2014-07-16 14:48:45

by Mark Brown

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

On Wed, Jul 16, 2014 at 08:12:04PM +0530, pramod gurav wrote:
> On Wed, Jul 16, 2014 at 7:48 PM, Mark Brown <[email protected]> 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.


Attachments:
(No filename) (397.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments