Re: [PATCH v2 2/2] leds: lp50xx: Add the LP50XX family of the RGB LED driver

[Pavel Machek]

Hi!

> On 1/15/19 4:22 PM, Pavel Machek wrote:
> > Hi!
> > 
> >>> +The 24-bit RGB value passed in follows the pattern 0xXXRRGGBB
> >>> +XX - Do not care ignored by the driver
> >>> +RR - is the 8 bit Red LED value
> >>> +GG - is the 8 bit Green LED value
> >>> +BB - is the 8 bit Blue LED value
> >>> +
> >>> +Example:
> >>> +LED module output 4 of the LP5024 will be a yellow color:
> >>> +echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color
> >>> +
> >>> +LED module output 4 of the LP5024 will be dimmed 50%:
> >>> +echo 0x80 > /sys/class/leds/lp5024\:led4_mod/brightness
> >>> +
> >>> +LED banked RGBs of the LP5036 will be a white color:
> >>> +echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color
> >>
> >> This part with example cans remain in Documentation/leds if you
> >>> like.
> > 
> > Does it actually work like that on hardware?
> 
> What?

If you do echo 0xffffff > /sys/class/leds/lp5036\:led_banked/color,
does it actually produce white? With all the different RGB modules
manufacturers can use with lp5024P?

If you do echo 0xe6de00 > /sys/class/leds/lp5024\:led4_mod/color, does
it actually produce yellow, with all the different RGB modules
manufacturers can use with lp5024P?

> > Is it supposed to support "normal" RGB colors as seen on monitors?
> 
> Monitors are not an application for this part.

You did not answer the question. When you talk about yellow, is it
same yellow the rest of world talks about?

> > Because 100% PWM on all channels does not result in white on hardware
> > I have.
> 
> I don't know I am usually blinded by the light and have no diffuser over
> the LEDs to disperse the light so when I look I see all 3 colors.

How can we have useful discussion about colors when you don't see the
colors?

Place a piece of paper over the LEDs....

> > But...
> > 
> > I believe we should have a reasonable design before we do something
> > like this. There's no guarantee someone will not use lp50xx with just
> > the white LEDs for example. How will this work? Plus existing hardware
> > already uses three separate LEDs for RGB LED. Why not provide same
> > interface?
> 
> Which existing hardware?  Are they using this part?

Nokia N900. They are not using this part, but any interface we invent
should work there, too.

> <rant>
> Why are we delaying getting the RGB framework or HSV in?
> I would rather design against something you want instead of having
> everyone complain about every implementation I post.
> </rant>

Because you insist on creating new kernel interfaces, when existing
interfaces work, and are doing that badly.

Because your patches are of lower quality than is acceptable for linux
kernel.

Because you don't seem to be reading the emails.

I sent list of requirements for RGB led support. This does not meet
them.

> It is not a normal RGB driver.  The device collates the individual RGB
> clusters into a single brightness register and you can modify the intensity of the individual
> LEDs via other registers.  If brightness is 0 then the cluster is OFF regardless of the color
> set in the individual registers.

I understand that. So just set cluster brightness to 255 and you have
normal RGB driver you can control with existing interfaces. You don't
have to use every feature your hardware has.

You did not answer the "what if someone uses this with all white LEDs"
question.

You know what? First, submit driver with similar functionality to
existing RGB drivers, using same interface existing drivers are
using. When that is accepted, we can talk about extending
kernel<->user interfaces.

Thanks,
									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