Re: RFC: LED hw triggering API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 03-04-19 21:51, Jacek Anaszewski wrote:
Hi,

On 4/3/19 11:22 AM, Hans de Goede wrote:
Hi,

On 02-04-19 21:17, Jacek Anaszewski wrote:
On 4/1/19 10:21 AM, Hans de Goede wrote:
Hi,

On 31-03-19 20:53, Pavel Machek wrote:
On Thu 2019-03-28 23:47:08, Hans de Goede wrote:
Hi,

On 28-03-19 23:43, Marek Behun wrote:
This indeed seems to give most of the infra, but the whole trigger
mechanism currently only allows setting-up blinking through triggers,
not glowing.  I'm tempted to just use the blink interface to implement
glowing on this hardware, since that is what we really want (blinking
is to distracting for the user IMHO). What is your take on just
implementing glowing through the standard blink interface ?

Optionally we could add a custom sysfs attr to the driver to
switch between the blink interface between actually blinking and
glowing and default to glowing.

Isn't glowing doable via led patterns?

A (software) trigger cannot select / set a led pattern, so either we
need a whole separate glowing API which seems overkill, or we need
to use the existing blink API. I guess one option would be to give
the led_trigger_blink function a "glow_if_possible" bool extra
parameter and pass this along to the actual led-driver.

Call it pattern API and it will be useful for more than glowing :-).

True, but the pattern API is very generic, where as glowing is pretty
specific, having triggers request glowing sounds like something which
I can see more triggers do; as for random patterns not so much.

More specifically we will often want to use hw glowing when available,
but not use a software pattern for this because that will cause too
much wakeups.

What's the problem with defining specific pattern tuple constraints
for enabling glowing for this device via hw_pattern file?

Well we want to extend the trigger(s) in:
drivers/power/supply/power_supply_leds.c

To enable glowing through the trigger, currently the
"battery-charging-blink-full-solid" trigger there calls
led_trigger_event or led_trigger_blink depending on the status of
the battery.

The idea for the PMIC LED controller we are discussing now is to
not focus on exporting all functionality, but only that functionality
for which we actually have a use-case. The primary use-case there will
be a new trigger called: "battery-charging-glow-full-solid", which
will be set as the default trigger for the charging LED controlled by
the PMIC LED controller.

This requires adding a new led_trigger_glow() or led_trigger_hw_pattern()
to the kernel's internal LED API. I prefer the simpler and more
expressive led_trigger_glow() option.  One reason for me preferring this
is that we want to keep the trigger code in power_supply_leds.c

OK, so this constraint was not in the initial proposal.

Note that this thread was originally not about the CHT Whiskey
Cove PMIC LED controller at all. Since we never finished our
discussion in the original thread and this thread seemed related
I sorta re-started the discussion in this thread and I seem to have
hijacked the thread, sorry about this.

The good news though is that we seem to be heading towards a solution
for the CHT WC PMIC case. I now realize that my original take on this,
which was to expose all functionality to allow maximum flexibility
was wrong and that your suggrestion to look at which use-case(s) we
actually care about is correct.

Which is how we've ended up with the "battery-charging-glow-full-solid"
trigger solution.

It immediately eliminates the possibility of setting two triggers
simultaneously, e.g. charging and full.

In this case we end up with battery-charging-glow-full-solid.

I agree that we should not have software glowing support.

For hardware glowing we will need glow_set() op and led_set_glow() API.
We'd need to figure out the most suitable argument list for describing
glowing interval.

I think that having a single interval which describes a full flow
cycle (from off to full-on to off again) makes most sense, so I suggest
having a single cycle_time parameter in milliseconds (like blinking
delay_on/off). So if cylce_time is set to 1000 then the LED glows
at 1 Hz, 500, 2 HZ, etc.

Regards,

Hans



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux