linux-kernel - Re: [PATCH v2 1/2] leds: Add Intel Cherry Trail Whiskey Cove PMIC LEDs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190217174549.GA13301@amd>
Date:   Sun, 17 Feb 2019 18:45:49 +0100
From:   Pavel Machek <pavel@....cz>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     Jacek Anaszewski <jacek.anaszewski@...il.com>,
        Yauhen Kharuzhy <jekhor@...il.com>,
        linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org
Subject: Re: [PATCH v2 1/2] leds: Add Intel Cherry Trail Whiskey Cove PMIC
 LEDs

Hi!

> >I see... and yes, that would be the easiest solution.
> >
> >But somehow I see "this LED is controlled by charging state" as
> >primary and "it shows pulses instead of staying on" as secondary
> >eye-candy.
> >
> >This week there was another driver for charger LED.. but that one does
> >not do pulses. Ideally, we'd like consistent interface to the
> >userland.
> >
> >(To make it complex, the other driver supports things like:
> >   LED solid on -- fully charged
> >   LED blinking slowly -- charging
> >   LED blinking fast -- charge error
> >   LED off -- not charging).
> 
> Something like that could be supported with my original hw_pattern
> proposal where we simply encode all of this in the hw-pattern file:
> 
> tupple0: charging blinking_on_time
> tupple1: charging blinking_off_time
> tupple2: charging breathing_time
> tupple3: manual blinking_on_time
> tupple4: manual blinking_off_time
> tupple5: manual breathing_time

So the userland would need to know "I'm on yoga, so 3rd tupple sets up
breathing when charging"?

> So for this chip you mention, we do not need the breathing time (no breathing support),
> so we would get the following tupples:
> 
> tupple0: not charging blinking_on_time
> tupple1: not charging blinking_off_time
> tupple2: slow charging blinking_on_time
> tupple3: slow charging blinking_off_time
> tupple4: fast charging blinking_on_time
> tupple5: fast charging blinking_off_time
> tupple6: charging error blinking_on_time
> tupple7: charging error blinking_off_time

Patterns and their meanings are fixed (or almost) on that driver, so
fortunately that will not be neccessary.

> Where by solid on/off can be done by setting one
> of the blinking times to 0.
> 
> Having hw_pattern ABIs like this where some of
> the tupples only activate on certain conditions
> might be better then a hardware_control sysfs
> file as it offers more flexibility.

Agreed about flexibility, but I don't think it does enough to provide
hardware abstraction. It might be too much flexibility.

Also it only works with hw controlled LEDs.

With RGB LED multiple patterns per LED make a lot of sense (as there's
typically just one led.

For example on N900, we have

green: fully charged.
yellow pulsing: charging.
solid yellow: hardware feature, emergency charging.
white pulsing: happy phone, no events
blue fast pulses: message waiting

On N900, I'm handling that in userspace, but...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)