Re: [PATCH v2] leds: lm3697: Rename struct into more appropriate name

From: Dan Murphy
Date: Mon Oct 12 2020 - 10:28:25 EST


Pavel

On 10/10/20 4:50 PM, Marek Behun wrote:
On Sat, 10 Oct 2020 20:57:00 +0200
Pavel Machek <pavel@xxxxxx> wrote:

On Fri 2020-10-09 15:51:35, Gabriel David wrote:
The mentioned struct, lm3697_led, was renamed to lm3697_bank since the
structure is representing the control banks. This name, in my opinion,
is more semantically correct. The pointers referring to it were also
renamed.
Signed-off-by: Gabriel David <ultracoolguy4@xxxxxxxxxxxxxx>
---
Yes, this is the same Gabriel David from ultracoolguy@xxxxxxxxxxxx and
ultracoolguy@xxxxxxxxxxx. If you want me to confirm it I'll gladly do
it.
No problem with that, and no need to resend. This can proably wait
for 5.11...

I'd like some comment from Dan... and perhaps I'd want to understand
what the difference between LED and bank is.

...there can be more than one LED connected to the given bank, that's
what you are pointing out?

...but these LEDs will always work in unison, and they are handled as
single LED by Linux, right?

Pavel,

the controller can connect 3 LED strips (to 3 different pins on the
chip). There are 2 LED control banks (this is where you can set
brightness). For each LED strip (each output pin) you can configure to
which control bank it connects. So you have 3 LED strips and 2 control
banks, that is 2^3 = 8 different configurations of connecting LED
control bank to LED strip.

From the perspective of Linux you see the two control banks as 2 LED
class devices (because you are setting brightness for control banks,
not for the LED strips).

The way Marek explains it is correct and the way I wrote the driver intially.  There is no direct control of the LEDs only controlling the 2 banks.

As an example a device can put LED string 1 and 2 on a single bank to control the backlight for a display and put LED string 3 on a different bank to control the backlight of a keyboard. Like in the Droid and Droid 4 devices.  2 strings illuminate the display backlight and 1 string illuminates the keyboard the display backlight can have a independent brightness then the keyboard.

To me the name of the structure does not impose any functional changes just semantic changes.  And it just makes it a bit more difficult to back port functional fixes as this patch would be made mandatory for cherry picking.  But I do not get many requests to back port this driver so it maybe be a moot point.

Dan