On Thu 2016-11-10 22:34:07, Jacek Anaszewski wrote:
> Hi,
>
> On 11/10/2016 09:29 PM, Pavel Machek wrote:
> >On Thu 2016-11-10 10:55:37, Tony Lindgren wrote:
> >>* Pavel Machek <pavel@xxxxxx> [161110 09:29]:
> >>>Hi!
> >>>
> >>>>>>>Looks like commit 883d32ce3385 ("leds: core: Add support for poll()ing
> >>>>>>>the sysfs brightness attr for changes.") breaks runtime PM for me.
> >>>>>>>
> >>>>>>>On my omap dm3730 based test system, idle power consumption is over 70
> >>>>>>>times higher now with this patch! It goes from about 6mW for the core
> >>>>>>>system to over 440mW during idle meaning there's some busy timer now
> >>>>>>>active.
> >>>>>>>
> >>>>>>>Reverting this patch fixes the issue. Any ideas?
> >>>
> >>>Are you using any LED that toggles with high frequency? Like perhaps
> >>>LED that is lit when CPU is active?
> >>
> >>Yeah one of them seems to have cpu0 as the default trigger.
> >
> >Aha. Its quite obvious we don't want to notify sysfs each time that
> >one is toggled, right?
> >
> >IMO brightness should display max brightness for the trigger, as Hans
> >suggested, anything else is madness for trigger such as cpu activity.
>
> Are you suggesting that we should revert changes introduced
> by below patch?
>
> commit 29d76dfa29fe22583aefddccda0bc56aa81035dc
> Author: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
> Date: Tue Mar 18 09:47:48 2008 +0000
>
> leds: Add support to leds with readable status
>
> Some led hardware allows drivers to query the led state, and this patch
> adds a hook to let the led class take advantage of that information when
> available.
>
> Without this functionality, when access to the led hardware is not
> exclusive (i.e. firmware or hardware might change its state behind the
> kernel's back), reality goes out of sync with the led class' idea of
> what
> the led is doing, which is annoying at best.
Hmm. So userland can read the LED state, and it can get _some_ value
back, but it can not know if it is current state or not.
I don't think that's a good interface. I see it is from 2008... is
someone using it? Maybe it is too late for revert.
But I'd certainly not extend it with poll.
IMO reading/polling should only be available with some triggers. It
does not make sense with "CPU load" trigger. It makes sense with
"keyboard light changeable by hardware" trigger.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: Digital signature
