[PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Josh Boyer]

Commit 4f5ca836b "HID: hid-input: add support for HID devices reporting
Battery Strength" added the CONFIG_HID_BATTERY_STRENGTH option to report
the battery strength of HID devices.  The commit log explicitly mentions
it not working properly with recent userspace, but it is default y
anyway.  This is rather odd, and actually causes problems on real
systems.

This works around Fedora bug
https://bugzilla.redhat.com/show_bug.cgi?id=806295

Signed-off-by: Josh Boyer <jwboyer@redhat.com>

---

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a3d0332..ffddcba 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -34,7 +34,7 @@ config HID
 config HID_BATTERY_STRENGTH
 	bool
 	depends on HID && POWER_SUPPLY && HID = POWER_SUPPLY
-	default y
+	default n
 
 config HIDRAW
 	bool "/dev/hidraw raw HID device support"

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Bastien Nocera]

On Wed, 2012-04-18 at 10:05 -0400, Josh Boyer wrote:
> Commit 4f5ca836b "HID: hid-input: add support for HID devices reporting
> Battery Strength" added the CONFIG_HID_BATTERY_STRENGTH option to report
> the battery strength of HID devices.  The commit log explicitly mentions
> it not working properly with recent userspace, but it is default y
> anyway.  This is rather odd, and actually causes problems on real
> systems.
> 
> This works around Fedora bug
> https://bugzilla.redhat.com/show_bug.cgi?id=806295

Don't. The only bit of user-space that's broken is upower, and it's
getting a fixed release shortly.

Richard?

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Josh Boyer]

On Wed, Apr 18, 2012 at 03:08:54PM +0100, Bastien Nocera wrote:
> On Wed, 2012-04-18 at 10:05 -0400, Josh Boyer wrote:
> > Commit 4f5ca836b "HID: hid-input: add support for HID devices reporting
> > Battery Strength" added the CONFIG_HID_BATTERY_STRENGTH option to report
> > the battery strength of HID devices.  The commit log explicitly mentions
> > it not working properly with recent userspace, but it is default y
> > anyway.  This is rather odd, and actually causes problems on real
> > systems.
> > 
> > This works around Fedora bug
> > https://bugzilla.redhat.com/show_bug.cgi?id=806295
> 
> Don't. The only bit of user-space that's broken is upower, and it's
> getting a fixed release shortly.

Matthew suggested that additional kernel patches were required to add a
scope parameter so upower could understand that the battery is powering
a device and not the system.

Also, when did it become OK to force-enable a new feature that is
_known_ to not work properly on recent userspace?  That makes no sense
to me.

josh

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Matthew Garrett]

On Wed, Apr 18, 2012 at 10:11:31AM -0400, Josh Boyer wrote:
> On Wed, Apr 18, 2012 at 03:08:54PM +0100, Bastien Nocera wrote:
> > Don't. The only bit of user-space that's broken is upower, and it's
> > getting a fixed release shortly.
> 
> Matthew suggested that additional kernel patches were required to add a
> scope parameter so upower could understand that the battery is powering
> a device and not the system.
> 
> Also, when did it become OK to force-enable a new feature that is
> _known_ to not work properly on recent userspace?  That makes no sense
> to me.

I agree. It's a great feature, but it shouldn't be default y if 
userspace isn't ready for it yet.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Jeremy Fitzhardinge]

On 04/18/2012 07:24 AM, Matthew Garrett wrote:
> On Wed, Apr 18, 2012 at 10:11:31AM -0400, Josh Boyer wrote:
>> On Wed, Apr 18, 2012 at 03:08:54PM +0100, Bastien Nocera wrote:
>>> Don't. The only bit of user-space that's broken is upower, and it's
>>> getting a fixed release shortly.
>> Matthew suggested that additional kernel patches were required to add a
>> scope parameter so upower could understand that the battery is powering
>> a device and not the system.
>>
>> Also, when did it become OK to force-enable a new feature that is
>> _known_ to not work properly on recent userspace?  That makes no sense
>> to me.
> I agree. It's a great feature, but it shouldn't be default y if 
> userspace isn't ready for it yet.

Dammit, I should have pushed harder to get the scope into the same
release.  Oh well.

Does upower trigger a shutdown/hibernation if *any* power supply is low,
or just if all of them are?  Are the failing cases ones where the BT
keyboard is the only visible power supply?

In any case, no objections to disabling it if its the only way to
prevent regressions for now.

    J

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Matthew Garrett]

On Wed, Apr 18, 2012 at 10:12:33AM -0700, Jeremy Fitzhardinge wrote:
> Does upower trigger a shutdown/hibernation if *any* power supply is low,
> or just if all of them are?  Are the failing cases ones where the BT
> keyboard is the only visible power supply?

We're seeing shutdowns on desktop machines using bluetooth devices, yes.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Jiri Kosina]

On Wed, 18 Apr 2012, Matthew Garrett wrote:

> > Matthew suggested that additional kernel patches were required to add a
> > scope parameter so upower could understand that the battery is powering
> > a device and not the system.
> > 
> > Also, when did it become OK to force-enable a new feature that is
> > _known_ to not work properly on recent userspace?  That makes no sense
> > to me.
> 
> I agree. It's a great feature, but it shouldn't be default y if 
> userspace isn't ready for it yet.

Fully agreed, the 'default y' part escaped my attention. I will be 
applying the patch and pushing out to Linus.

I even wonder whether we shouldn't put a warning into the Kconfig entry, 
explaining the consequences of having userspace not ready for the feature.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Geert Uytterhoeven]

On Wed, Apr 18, 2012 at 19:28, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 18 Apr 2012, Matthew Garrett wrote:
>> > Matthew suggested that additional kernel patches were required to add a
>> > scope parameter so upower could understand that the battery is powering
>> > a device and not the system.
>> >
>> > Also, when did it become OK to force-enable a new feature that is
>> > _known_ to not work properly on recent userspace?  That makes no sense
>> > to me.
>>
>> I agree. It's a great feature, but it shouldn't be default y if
>> userspace isn't ready for it yet.
>
> Fully agreed, the 'default y' part escaped my attention. I will be
> applying the patch and pushing out to Linus.
>
> I even wonder whether we shouldn't put a warning into the Kconfig entry,
> explaining the consequences of having userspace not ready for the feature.

I saw it move from HID_BATTERY_STRENGTH=y to "# HID_BATTERY_STRENGTH
is not set" in my max-all build.

As there's no help entry, and no other option selects it, HID_BATTERY_STRENGTH
cannot be set anyway in v3.4-rc4?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] HID: hid-input: default HID_BATTERY_STRENGTH to no

[Jiri Kosina]

On Sun, 22 Apr 2012, Geert Uytterhoeven wrote:

> >> > Matthew suggested that additional kernel patches were required to add a
> >> > scope parameter so upower could understand that the battery is powering
> >> > a device and not the system.
> >> >
> >> > Also, when did it become OK to force-enable a new feature that is
> >> > _known_ to not work properly on recent userspace?  That makes no sense
> >> > to me.
> >>
> >> I agree. It's a great feature, but it shouldn't be default y if
> >> userspace isn't ready for it yet.
> >
> > Fully agreed, the 'default y' part escaped my attention. I will be
> > applying the patch and pushing out to Linus.
> >
> > I even wonder whether we shouldn't put a warning into the Kconfig entry,
> > explaining the consequences of having userspace not ready for the feature.
> 
> I saw it move from HID_BATTERY_STRENGTH=y to "# HID_BATTERY_STRENGTH
> is not set" in my max-all build.

Good point, thanks Geert. I will fix that up.

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