Re: [PATCH 1/2] leds: Add driver for Qualcomm LPG

From: Pavel Machek
Date: Thu Mar 23 2017 - 16:37:57 EST


Hi!

> The Light Pulse Generator (LPG) is a PWM-block found in a wide range of
> PMICs from Qualcomm. It can operate on fixed parameters or based on a
> lookup-table, altering the duty cycle over time - which provides the
> means for e.g. hardware assisted transitions of LED brightness.

Ok, this is not first hardware that supports something like this. We
have similar hardware that can do blinking on Nokia N900 -- please
take a look at leds-lp55*.c

And it would be really good to provide hardware abstraction. We really
don't want to have different userspace for LPG and for N900 and for
...

Which probably means finding subset that makes sense for everyone.

Hmm. What is difference between "ping_pong" and "reverse"? And do we
really want it? That seems little .. too specialized.

How are different channels on RGB LED synchronized?

> +What: /sys/class/leds/<led>/pattern
> +Date: March 2017
> +KernelVersion: 4.12
> +Contact: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> +Description:
> + Comma-separated list of duty cycle values to output from
> + the ramp generator. Values should be in the range of 0
> + to 511.

We normally do "space separated" in sysfs.

Can your engine do "smooth transitions"? For example if you want to
slowly turn on the LED on, can you do something more clever than

0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19,
20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36,
...
496, 497, 498, 499, 500, 501, 502, 503, 504, 505, 506, 507, 508, 509,
510, 511

? What is the maximum length of the pattern?

Could we do patterns in form of "delay brightness delay brightness"
.... ?

> +static enum led_brightness lpg_brightnes_get(struct led_classdev *cdev)
> +{
> + struct lpg *lpg = container_of(cdev, struct lpg, cdev);
> + unsigned long max = (1 << lpg->pwm_size) - 1;
> +
> + if (!lpg->enabled)
> + return LED_OFF;
> +
> + return lpg->pwm_value * cdev->max_brightness / max;
> +}

Does this return something reasonable when pattern is running?

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