diff mbox

hwmon/sensors: fix SENSORS_LM75 dependencies

Message ID 52CB53DC.4030901@infradead.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Randy Dunlap Jan. 7, 2014, 1:09 a.m. UTC
On 01/06/14 12:32, Guenter Roeck wrote:
> On Mon, Jan 06, 2014 at 11:51:36AM -0800, Randy Dunlap wrote:
>> On 01/06/14 01:40, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> This tree fails (more than usual) the powerpc allyesconfig build.
>>>
>>> Changes since 20131224:
>>>
>>
>>
>> on i386:
>>
>> drivers/built-in.o: In function `lm75_remove':
>> lm75.c:(.text+0x12bd8c): undefined reference to `thermal_zone_of_sensor_unregister'
>> drivers/built-in.o: In function `lm75_probe':
>> lm75.c:(.text+0x12c123): undefined reference to `thermal_zone_of_sensor_register'
>>
> 
> AFAICS that is a dependency problem in thermal code.
> CONFIG_THERMAL=m defines the symbols, but CONFIG_SENSORS_LM75=y.

CONFIG_THERMAL_OF defines the symbols.

It looks to me like drivers/hwmon/Kconfig SENSORS_LM75 entry needs a
	depends on THERMAL_OF
line, but that won't fix this build error.
The problem (once again, not the first time that I have seen this one) is that
we have a tristate =m that depends on a bool =y.

Hm, I bet a "depends on THERMAL" for SENSORS_LM75 will fix this.
[test]
Yes, that restricts (limits) SENSORS_LM75 to n/m; y is not possible.

Patch for your consideration is below.

> 
> Up to the thermal folks to fix; if the code ends up in mainline we'll have to
> revert the patch introducing the calls to the lm75 driver (and the tmp102 driver
> which has the same issue).
> 
> Another oddity is the help text to THERMAL_OF, which states that it supports
> reading and parsing thermal data definitions out of dt but fails to mention
> that it also provides the above API functions.

---
From: Randy Dunlap <rdunlap@infradead.org>

Fix SENSORS_LM75 dependencies to eliminate build errors:

drivers/built-in.o: In function `lm75_remove':
lm75.c:(.text+0x12bd8c): undefined reference to `thermal_zone_of_sensor_unregister'
drivers/built-in.o: In function `lm75_probe':
lm75.c:(.text+0x12c123): undefined reference to `thermal_zone_of_sensor_register'

Add depends on THERMAL_OF since that is what provides the
register/unregister functions above.

Add depends on THERMAL since THERMAL is a tristate (while THERMAL_OF
is a bool) and SENSORS_LM75 (tristate) needs to be limited to modular
builds when THERMAL=m.

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
---
 drivers/hwmon/Kconfig |    2 ++
 1 file changed, 2 insertions(+)

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

Comments

Guenter Roeck Jan. 7, 2014, 2:26 a.m. UTC | #1
On 01/06/2014 05:09 PM, Randy Dunlap wrote:
> On 01/06/14 12:32, Guenter Roeck wrote:
>> On Mon, Jan 06, 2014 at 11:51:36AM -0800, Randy Dunlap wrote:
>>> On 01/06/14 01:40, Stephen Rothwell wrote:
>>>> Hi all,
>>>>
>>>> This tree fails (more than usual) the powerpc allyesconfig build.
>>>>
>>>> Changes since 20131224:
>>>>
>>>
>>>
>>> on i386:
>>>
>>> drivers/built-in.o: In function `lm75_remove':
>>> lm75.c:(.text+0x12bd8c): undefined reference to `thermal_zone_of_sensor_unregister'
>>> drivers/built-in.o: In function `lm75_probe':
>>> lm75.c:(.text+0x12c123): undefined reference to `thermal_zone_of_sensor_register'
>>>
>>
>> AFAICS that is a dependency problem in thermal code.
>> CONFIG_THERMAL=m defines the symbols, but CONFIG_SENSORS_LM75=y.
>
> CONFIG_THERMAL_OF defines the symbols.
>
> It looks to me like drivers/hwmon/Kconfig SENSORS_LM75 entry needs a
> 	depends on THERMAL_OF
> line, but that won't fix this build error.
> The problem (once again, not the first time that I have seen this one) is that
> we have a tristate =m that depends on a bool =y.
>
> Hm, I bet a "depends on THERMAL" for SENSORS_LM75 will fix this.
> [test]
> Yes, that restricts (limits) SENSORS_LM75 to n/m; y is not possible.
>
> Patch for your consideration is below.
>
>>
>> Up to the thermal folks to fix; if the code ends up in mainline we'll have to
>> revert the patch introducing the calls to the lm75 driver (and the tmp102 driver
>> which has the same issue).
>>
>> Another oddity is the help text to THERMAL_OF, which states that it supports
>> reading and parsing thermal data definitions out of dt but fails to mention
>> that it also provides the above API functions.
>
> ---
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fix SENSORS_LM75 dependencies to eliminate build errors:
>
> drivers/built-in.o: In function `lm75_remove':
> lm75.c:(.text+0x12bd8c): undefined reference to `thermal_zone_of_sensor_unregister'
> drivers/built-in.o: In function `lm75_probe':
> lm75.c:(.text+0x12c123): undefined reference to `thermal_zone_of_sensor_register'
>
> Add depends on THERMAL_OF since that is what provides the
> register/unregister functions above.
>
> Add depends on THERMAL since THERMAL is a tristate (while THERMAL_OF
> is a bool) and SENSORS_LM75 (tristate) needs to be limited to modular
> builds when THERMAL=m.
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> ---
>   drivers/hwmon/Kconfig |    2 ++
>   1 file changed, 2 insertions(+)
>
> --- linux-next-20140106.orig/drivers/hwmon/Kconfig
> +++ linux-next-20140106/drivers/hwmon/Kconfig
> @@ -650,6 +650,8 @@ config SENSORS_LM73
>   config SENSORS_LM75
>   	tristate "National Semiconductor LM75 and compatibles"
>   	depends on I2C
> +	depends on THERMAL
> +	depends on THERMAL_OF
>   	help
>   	  If you say yes here you get support for one common type of
>   	  temperature sensor chip, with models including:
>
>
NACK. The driver does not and must not depend on THERMAL. This needs to be addressed
in the thermal code, for example with dummy declarations if THERMAL=m but
SENSORS_LM75=y. The functions are already declared as dummies if THERMAL_OF=n.

Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eduardo Valentin Jan. 7, 2014, 11:35 a.m. UTC | #2
On 06-01-2014 22:26, Guenter Roeck wrote:
> On 01/06/2014 05:09 PM, Randy Dunlap wrote:
>> On 01/06/14 12:32, Guenter Roeck wrote:
>>> On Mon, Jan 06, 2014 at 11:51:36AM -0800, Randy Dunlap wrote:
>>>> On 01/06/14 01:40, Stephen Rothwell wrote:
>>>>> Hi all,
>>>>>
>>>>> This tree fails (more than usual) the powerpc allyesconfig build.
>>>>>
>>>>> Changes since 20131224:
>>>>>
>>>>
>>>>
>>>> on i386:
>>>>
>>>> drivers/built-in.o: In function `lm75_remove':
>>>> lm75.c:(.text+0x12bd8c): undefined reference to
>>>> `thermal_zone_of_sensor_unregister'
>>>> drivers/built-in.o: In function `lm75_probe':
>>>> lm75.c:(.text+0x12c123): undefined reference to
>>>> `thermal_zone_of_sensor_register'
>>>>
>>>
>>> AFAICS that is a dependency problem in thermal code.
>>> CONFIG_THERMAL=m defines the symbols, but CONFIG_SENSORS_LM75=y.
>>
>> CONFIG_THERMAL_OF defines the symbols.
>>
>> It looks to me like drivers/hwmon/Kconfig SENSORS_LM75 entry needs a
>>     depends on THERMAL_OF
>> line, but that won't fix this build error.
>> The problem (once again, not the first time that I have seen this one)
>> is that
>> we have a tristate =m that depends on a bool =y.
>>
>> Hm, I bet a "depends on THERMAL" for SENSORS_LM75 will fix this.
>> [test]
>> Yes, that restricts (limits) SENSORS_LM75 to n/m; y is not possible.
>>
>> Patch for your consideration is below.
>>
>>>
>>> Up to the thermal folks to fix; if the code ends up in mainline we'll
>>> have to
>>> revert the patch introducing the calls to the lm75 driver (and the
>>> tmp102 driver
>>> which has the same issue).
>>>
>>> Another oddity is the help text to THERMAL_OF, which states that it
>>> supports
>>> reading and parsing thermal data definitions out of dt but fails to
>>> mention
>>> that it also provides the above API functions.
>>
>> ---
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> Fix SENSORS_LM75 dependencies to eliminate build errors:
>>
>> drivers/built-in.o: In function `lm75_remove':
>> lm75.c:(.text+0x12bd8c): undefined reference to
>> `thermal_zone_of_sensor_unregister'
>> drivers/built-in.o: In function `lm75_probe':
>> lm75.c:(.text+0x12c123): undefined reference to
>> `thermal_zone_of_sensor_register'
>>
>> Add depends on THERMAL_OF since that is what provides the
>> register/unregister functions above.
>>
>> Add depends on THERMAL since THERMAL is a tristate (while THERMAL_OF
>> is a bool) and SENSORS_LM75 (tristate) needs to be limited to modular
>> builds when THERMAL=m.
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> ---
>>   drivers/hwmon/Kconfig |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> --- linux-next-20140106.orig/drivers/hwmon/Kconfig
>> +++ linux-next-20140106/drivers/hwmon/Kconfig
>> @@ -650,6 +650,8 @@ config SENSORS_LM73
>>   config SENSORS_LM75
>>       tristate "National Semiconductor LM75 and compatibles"
>>       depends on I2C
>> +    depends on THERMAL
>> +    depends on THERMAL_OF
>>       help
>>         If you say yes here you get support for one common type of
>>         temperature sensor chip, with models including:
>>
>>
> NACK. The driver does not and must not depend on THERMAL. This needs to
> be addressed
> in the thermal code, for example with dummy declarations if THERMAL=m but
> SENSORS_LM75=y. The functions are already declared as dummies if
> THERMAL_OF=n.

What do you mean by that? The code of an API provider needs to keep
track of all its users?

> 
> Guenter
> 
> 
>
Jean Delvare Jan. 7, 2014, 12:04 p.m. UTC | #3
Hi Guenter, Randy,

On Mon, 06 Jan 2014 18:26:34 -0800, Guenter Roeck wrote:
> On 01/06/2014 05:09 PM, Randy Dunlap wrote:
> > From: Randy Dunlap <rdunlap@infradead.org>
> >
> > Fix SENSORS_LM75 dependencies to eliminate build errors:
> >
> > drivers/built-in.o: In function `lm75_remove':
> > lm75.c:(.text+0x12bd8c): undefined reference to `thermal_zone_of_sensor_unregister'
> > drivers/built-in.o: In function `lm75_probe':
> > lm75.c:(.text+0x12c123): undefined reference to `thermal_zone_of_sensor_register'
> >
> > Add depends on THERMAL_OF since that is what provides the
> > register/unregister functions above.
> >
> > Add depends on THERMAL since THERMAL is a tristate (while THERMAL_OF
> > is a bool) and SENSORS_LM75 (tristate) needs to be limited to modular
> > builds when THERMAL=m.
> >
> > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > ---
> >   drivers/hwmon/Kconfig |    2 ++
> >   1 file changed, 2 insertions(+)
> >
> > --- linux-next-20140106.orig/drivers/hwmon/Kconfig
> > +++ linux-next-20140106/drivers/hwmon/Kconfig
> > @@ -650,6 +650,8 @@ config SENSORS_LM73
> >   config SENSORS_LM75
> >   	tristate "National Semiconductor LM75 and compatibles"
> >   	depends on I2C
> > +	depends on THERMAL
> > +	depends on THERMAL_OF
> >   	help
> >   	  If you say yes here you get support for one common type of
> >   	  temperature sensor chip, with models including:
> >
> >
> NACK. The driver does not and must not depend on THERMAL.

Correct.

> This needs to be addressed
> in the thermal code, for example with dummy declarations if THERMAL=m but
> SENSORS_LM75=y. The functions are already declared as dummies if THERMAL_OF=n.

This won't fly I'm afraid, the number of hwmon drivers affected will
grow in the future and you certainly don't want to have to change the
generic thermal code every time a new driver is added/converted.

I've looked at the problem this morning and I will admit I do not even
understand what the problem is. In Randy's config, CONFIG_THERMAL_OF=y
so both thermal_zone_of_sensor_unregister and
thermal_zone_of_sensor_register are built-in. SENSORS_LM75=y so the
calls to these functions are built-in too. I just can't see how this
can be a problem at link time. Can anyone enlighten me?
Guenter Roeck Jan. 7, 2014, 4:02 p.m. UTC | #4
On Tue, Jan 07, 2014 at 01:04:01PM +0100, Jean Delvare wrote:
> Hi Guenter, Randy,
> 
> On Mon, 06 Jan 2014 18:26:34 -0800, Guenter Roeck wrote:
> > On 01/06/2014 05:09 PM, Randy Dunlap wrote:
> > > From: Randy Dunlap <rdunlap@infradead.org>
> > >
> > > Fix SENSORS_LM75 dependencies to eliminate build errors:
> > >
> > > drivers/built-in.o: In function `lm75_remove':
> > > lm75.c:(.text+0x12bd8c): undefined reference to `thermal_zone_of_sensor_unregister'
> > > drivers/built-in.o: In function `lm75_probe':
> > > lm75.c:(.text+0x12c123): undefined reference to `thermal_zone_of_sensor_register'
> > >
> > > Add depends on THERMAL_OF since that is what provides the
> > > register/unregister functions above.
> > >
> > > Add depends on THERMAL since THERMAL is a tristate (while THERMAL_OF
> > > is a bool) and SENSORS_LM75 (tristate) needs to be limited to modular
> > > builds when THERMAL=m.
> > >
> > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> > > ---
> > >   drivers/hwmon/Kconfig |    2 ++
> > >   1 file changed, 2 insertions(+)
> > >
> > > --- linux-next-20140106.orig/drivers/hwmon/Kconfig
> > > +++ linux-next-20140106/drivers/hwmon/Kconfig
> > > @@ -650,6 +650,8 @@ config SENSORS_LM73
> > >   config SENSORS_LM75
> > >   	tristate "National Semiconductor LM75 and compatibles"
> > >   	depends on I2C
> > > +	depends on THERMAL
> > > +	depends on THERMAL_OF
> > >   	help
> > >   	  If you say yes here you get support for one common type of
> > >   	  temperature sensor chip, with models including:
> > >
> > >
> > NACK. The driver does not and must not depend on THERMAL.
> 
> Correct.
> 
> > This needs to be addressed
> > in the thermal code, for example with dummy declarations if THERMAL=m but
> > SENSORS_LM75=y. The functions are already declared as dummies if THERMAL_OF=n.
> 
> This won't fly I'm afraid, the number of hwmon drivers affected will
> grow in the future and you certainly don't want to have to change the
> generic thermal code every time a new driver is added/converted.
> 
That is not really necessary - see the patch fragment I sent as response to
the other patch.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

--- linux-next-20140106.orig/drivers/hwmon/Kconfig
+++ linux-next-20140106/drivers/hwmon/Kconfig
@@ -650,6 +650,8 @@  config SENSORS_LM73
 config SENSORS_LM75
 	tristate "National Semiconductor LM75 and compatibles"
 	depends on I2C
+	depends on THERMAL
+	depends on THERMAL_OF
 	help
 	  If you say yes here you get support for one common type of
 	  temperature sensor chip, with models including: