linux-next bad Kconfig for drivers/hid

linux-next bad Kconfig for drivers/hid

[Tony Breeds]

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


Commit 4f5ca836bef3 (HID: hid-input: add support for HID devices
reporting Battery Strength) went into linux-next on Dec 1st since then a
ppc6xx_defconfig has been failing with:

---
drivers/built-in.o: In function `hidinput_cleanup_battery':
/scratch/tony/working/drivers/hid/hid-input.c:351: undefined reference to `power_supply_unregister'
drivers/built-in.o: In function `hidinput_setup_battery':
/scratch/tony/working/drivers/hid/hid-input.c:338: undefined reference to `power_supply_register'
make[1]: *** [.tmp_vmlinux1] Error 1
---

http://kisskb.ellerman.id.au/kisskb/buildresult/5012563/
vs
http://kisskb.ellerman.id.au/kisskb/buildresult/5017366/

The defconfig in question doens't mention either option
(CONFIG_POWER_SUPPLY or CONFIG_HID_BATTERY_STRENGTH) and kbuild is
genertaing
CONFIG_HID_BATTERY_STRENGTH=y
CONFIG_POWER_SUPPLY=m
which clearly isn't going to work.

The following change to HID_BATTERY_STRENGTH Kconfig "works" but seems a
little gross.

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 5ed64f6..d2a94e6 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -33,7 +33,7 @@ config HID
 
 config HID_BATTERY_STRENGTH
        bool
-       depends on POWER_SUPPLY
+       depends on POWER_SUPPLY=y
        default y
 
 config HIDRAW

Any chance we can get a fix into linux-next?

Yours Tony

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

Re: linux-next bad Kconfig for drivers/hid

[Jeremy Fitzhardinge]

On 12/08/2011 05:27 PM, Tony Breeds wrote:
> Commit 4f5ca836bef3 (HID: hid-input: add support for HID devices
> reporting Battery Strength) went into linux-next on Dec 1st since then a
> ppc6xx_defconfig has been failing with:
>
> ---
> drivers/built-in.o: In function `hidinput_cleanup_battery':
> /scratch/tony/working/drivers/hid/hid-input.c:351: undefined reference to `power_supply_unregister'
> drivers/built-in.o: In function `hidinput_setup_battery':
> /scratch/tony/working/drivers/hid/hid-input.c:338: undefined reference to `power_supply_register'
> make[1]: *** [.tmp_vmlinux1] Error 1
> ---
>
> http://kisskb.ellerman.id.au/kisskb/buildresult/5012563/
> vs
> http://kisskb.ellerman.id.au/kisskb/buildresult/5017366/
>
> The defconfig in question doens't mention either option
> (CONFIG_POWER_SUPPLY or CONFIG_HID_BATTERY_STRENGTH) and kbuild is
> genertaing
> CONFIG_HID_BATTERY_STRENGTH=y
> CONFIG_POWER_SUPPLY=m
> which clearly isn't going to work.
>
> The following change to HID_BATTERY_STRENGTH Kconfig "works" but seems a
> little gross.
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 5ed64f6..d2a94e6 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -33,7 +33,7 @@ config HID
>  
>  config HID_BATTERY_STRENGTH
>         bool
> -       depends on POWER_SUPPLY
> +       depends on POWER_SUPPLY=y
>         default y
>  
>  config HIDRAW
>
> Any chance we can get a fix into linux-next?

Hm.  How about making it "depends on HID && POWER_SUPPLY"?  I think that
would needlessly disable it if HID is also modular, but I'm not sure how
to fix that.  "depends on HID && POWER_SUPPLY && HID == POWER_SUPPLY"?

    J

Re: linux-next bad Kconfig for drivers/hid

[Randy Dunlap]

On Thu, December 8, 2011 9:33 pm, Jeremy Fitzhardinge wrote:
> On 12/08/2011 05:27 PM, Tony Breeds wrote:
>> Commit 4f5ca836bef3 (HID: hid-input: add support for HID devices
>> reporting Battery Strength) went into linux-next on Dec 1st since then a
>> ppc6xx_defconfig has been failing with:
>>
>> ---
>> drivers/built-in.o: In function `hidinput_cleanup_battery':
>> /scratch/tony/working/drivers/hid/hid-input.c:351: undefined reference
>> to `power_supply_unregister'
>> drivers/built-in.o: In function `hidinput_setup_battery':
>> /scratch/tony/working/drivers/hid/hid-input.c:338: undefined reference
>> to `power_supply_register'
>> make[1]: *** [.tmp_vmlinux1] Error 1
>> ---
>>
>> http://kisskb.ellerman.id.au/kisskb/buildresult/5012563/
>> vs
>> http://kisskb.ellerman.id.au/kisskb/buildresult/5017366/
>>
>> The defconfig in question doens't mention either option
>> (CONFIG_POWER_SUPPLY or CONFIG_HID_BATTERY_STRENGTH) and kbuild is
>> genertaing
>> CONFIG_HID_BATTERY_STRENGTH=y
>> CONFIG_POWER_SUPPLY=m
>> which clearly isn't going to work.
>>
>> The following change to HID_BATTERY_STRENGTH Kconfig "works" but seems a
>> little gross.
>>
>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>> index 5ed64f6..d2a94e6 100644
>> --- a/drivers/hid/Kconfig
>> +++ b/drivers/hid/Kconfig
>> @@ -33,7 +33,7 @@ config HID
>>
>>  config HID_BATTERY_STRENGTH
>>         bool
>> -       depends on POWER_SUPPLY
>> +       depends on POWER_SUPPLY=y
>>         default y
>>
>>  config HIDRAW
>>
>> Any chance we can get a fix into linux-next?
>
> Hm.  How about making it "depends on HID && POWER_SUPPLY"?  I think that
> would needlessly disable it if HID is also modular, but I'm not sure how
> to fix that.  "depends on HID && POWER_SUPPLY && HID == POWER_SUPPLY"?

The last suggestion looks good to me.

-- 
~Randy

Re: linux-next bad Kconfig for drivers/hid

[Jiri Kosina]

On Thu, 8 Dec 2011, Jeremy Fitzhardinge wrote:

> > Commit 4f5ca836bef3 (HID: hid-input: add support for HID devices
> > reporting Battery Strength) went into linux-next on Dec 1st since then a
> > ppc6xx_defconfig has been failing with:
> >
> > ---
> > drivers/built-in.o: In function `hidinput_cleanup_battery':
> > /scratch/tony/working/drivers/hid/hid-input.c:351: undefined reference to `power_supply_unregister'
> > drivers/built-in.o: In function `hidinput_setup_battery':
> > /scratch/tony/working/drivers/hid/hid-input.c:338: undefined reference to `power_supply_register'
> > make[1]: *** [.tmp_vmlinux1] Error 1
> > ---
> >
> > http://kisskb.ellerman.id.au/kisskb/buildresult/5012563/
> > vs
> > http://kisskb.ellerman.id.au/kisskb/buildresult/5017366/
> >
> > The defconfig in question doens't mention either option
> > (CONFIG_POWER_SUPPLY or CONFIG_HID_BATTERY_STRENGTH) and kbuild is
> > genertaing
> > CONFIG_HID_BATTERY_STRENGTH=y
> > CONFIG_POWER_SUPPLY=m
> > which clearly isn't going to work.
> >
> > The following change to HID_BATTERY_STRENGTH Kconfig "works" but seems a
> > little gross.
> >
> > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> > index 5ed64f6..d2a94e6 100644
> > --- a/drivers/hid/Kconfig
> > +++ b/drivers/hid/Kconfig
> > @@ -33,7 +33,7 @@ config HID
> >  
> >  config HID_BATTERY_STRENGTH
> >         bool
> > -       depends on POWER_SUPPLY
> > +       depends on POWER_SUPPLY=y
> >         default y
> >  
> >  config HIDRAW
> >
> > Any chance we can get a fix into linux-next?
> 
> Hm.  How about making it "depends on HID && POWER_SUPPLY"?  I think that
> would needlessly disable it if HID is also modular, but I'm not sure how
> to fix that.  "depends on HID && POWER_SUPPLY && HID == POWER_SUPPLY"?

How about making it 'default POWER_SUPPLY' instead?

I somehow feel that we'd rather avoid too much 'default y's.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: linux-next bad Kconfig for drivers/hid

[Tony Breeds]

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

On Mon, Dec 12, 2011 at 12:21:16AM +0100, Jiri Kosina wrote:
> On Thu, 8 Dec 2011, Jeremy Fitzhardinge wrote:
> > 
> > Hm.  How about making it "depends on HID && POWER_SUPPLY"?  I think that
> > would needlessly disable it if HID is also modular, but I'm not sure how
> > to fix that.  "depends on HID && POWER_SUPPLY && HID == POWER_SUPPLY"?

That would work, but I think technically I think you could end up with
HID=m and POWER_SUPPLY=m which would still allow HID_BATTERY_STRENGTH=y
which is the same problem.

I don't know what kind of .config contortions you'd need to do to get
there.
 
> How about making it 'default POWER_SUPPLY' instead?

By itself that wont help as POWER_SUPPLY=m statisfies.

So it looks like we have Jeremy's:
	HID && POWER_SUPPLY && HID == POWER_SUPPLY
or my:
	POWER_SUPPLY=y

Yours Tony

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

Re: linux-next bad Kconfig for drivers/hid

[Jiri Kosina]

On Mon, 12 Dec 2011, Tony Breeds wrote:

> On Mon, Dec 12, 2011 at 12:21:16AM +0100, Jiri Kosina wrote:
> > On Thu, 8 Dec 2011, Jeremy Fitzhardinge wrote:
> > > 
> > > Hm.  How about making it "depends on HID && POWER_SUPPLY"?  I think that
> > > would needlessly disable it if HID is also modular, but I'm not sure how
> > > to fix that.  "depends on HID && POWER_SUPPLY && HID == POWER_SUPPLY"?
> 
> That would work, but I think technically I think you could end up with
> HID=m and POWER_SUPPLY=m which would still allow HID_BATTERY_STRENGTH=y
> which is the same problem.
> 
> I don't know what kind of .config contortions you'd need to do to get
> there.
>  
> > How about making it 'default POWER_SUPPLY' instead?
> 
> By itself that wont help as POWER_SUPPLY=m statisfies.
> 
> So it looks like we have Jeremy's:
> 	HID && POWER_SUPPLY && HID == POWER_SUPPLY

Tony,

have you actually tested this one to work in the configuration you have 
been seeing it to fail?

I don't seem to be able to find any use of '==' in other Kconfig files 
(and never used it myself), so I'd like to have confirmation that it 
actually works and fixes the problem before I apply it :)

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: linux-next bad Kconfig for drivers/hid

[Tony Breeds]

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

On Thu, Dec 15, 2011 at 11:08:08AM +0100, Jiri Kosina wrote:
 
> Tony,
> 
> have you actually tested this one to work in the configuration you have 
> been seeing it to fail?
> 
> I don't seem to be able to find any use of '==' in other Kconfig files 
> (and never used it myself), so I'd like to have confirmation that it 
> actually works and fixes the problem before I apply it :)

Sorry I used '=' instead of '==' and that worked.  I figured the '=='
was just a typo.

Yours Tony

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

Re: linux-next bad Kconfig for drivers/hid

[Randy Dunlap]

On 12/15/2011 02:08 AM, Jiri Kosina wrote:
> On Mon, 12 Dec 2011, Tony Breeds wrote:
> 
>> On Mon, Dec 12, 2011 at 12:21:16AM +0100, Jiri Kosina wrote:
>>> On Thu, 8 Dec 2011, Jeremy Fitzhardinge wrote:
>>>>
>>>> Hm.  How about making it "depends on HID && POWER_SUPPLY"?  I think that
>>>> would needlessly disable it if HID is also modular, but I'm not sure how
>>>> to fix that.  "depends on HID && POWER_SUPPLY && HID == POWER_SUPPLY"?
>>
>> That would work, but I think technically I think you could end up with
>> HID=m and POWER_SUPPLY=m which would still allow HID_BATTERY_STRENGTH=y
>> which is the same problem.
>>
>> I don't know what kind of .config contortions you'd need to do to get
>> there.
>>  
>>> How about making it 'default POWER_SUPPLY' instead?
>>
>> By itself that wont help as POWER_SUPPLY=m statisfies.
>>
>> So it looks like we have Jeremy's:
>> 	HID && POWER_SUPPLY && HID == POWER_SUPPLY
> 
> Tony,
> 
> have you actually tested this one to work in the configuration you have 
> been seeing it to fail?
> 
> I don't seem to be able to find any use of '==' in other Kconfig files 
> (and never used it myself), so I'd like to have confirmation that it 
> actually works and fixes the problem before I apply it :)

Documentation/kbuild/kconfig-language.txt does not list "==":

<expr> ::= <symbol>                             (1)
           <symbol> '=' <symbol>                (2)
           <symbol> '!=' <symbol>               (3)
           '(' <expr> ')'                       (4)
           '!' <expr>                           (5)
           <expr> '&&' <expr>                   (6)
           <expr> '||' <expr>                   (7)


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***