[PATCH v3 0/6] leds: Fix/Add is31fl319{0,1,3} support

Vincent Knecht posted 6 patches 3 years, 9 months ago
There is a newer version of this series
.../bindings/leds/issi,is31fl319x.yaml        | 193 +++++++++
.../bindings/leds/leds-is31fl319x.txt         |  61 ---
drivers/leds/leds-is31fl319x.c                | 406 +++++++++++++-----
3 files changed, 488 insertions(+), 172 deletions(-)
create mode 100644 Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
delete mode 100644 Documentation/devicetree/bindings/leds/leds-is31fl319x.txt
[PATCH v3 0/6] leds: Fix/Add is31fl319{0,1,3} support
Posted by Vincent Knecht 3 years, 9 months ago
v3:
- pick up Rob's R-b for patches 1 and 2
- reinstate bindings docs license and maintainer
  changes with Nikolaus agreement
- took Andy's comments on patch 4 into account

v2:
- keep original bindings license and maintainer/owner (Rob)
- squash bindings patches 2 & 4 (Krzysztof)

v1-resend:
- no change, resending after configuring git to accomodate
  for smtp provider limit of 5 emails per batch
- just change cover-letter to mention si-en chip for idol347


The is31fl3190, is31fl3191 and is31fl3193 chips (1 or 3 PWM channels)
cannot be handled the same as is31fl3196 and is31fl3199,
if only because the register map is different.
Also:
- the software shutdown bit is reversed
- and additional field needs to be set to enable all channels
- the led-max-microamp current values and setting are not the same

Datasheets:
https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3191_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf
https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf

This series:

- converts dt-bindings to dtschema, adding all si-en compatibles
  for convenience and consistency, and adding constraints on
  supported values for eg. reg address and led-max-microamp

- changes vars, structs and defines to not use 319X suffix
  but 3190 for 319{0,1,3} and 3196 for 319{6,9}

- adds fields in chipdef struct for chip-specific values

- only in the last patch, adds is31fl319{0,1,3} specific values
  so those chips can work.

Tested on msm8916-alcatel-idol347, which probably has an
si-en,sn3190 or si-en,sn3191 (only one white led indicator).

Vincent Knecht (6):
  dt-bindings: leds: Convert is31fl319x to dtschema
  dt-bindings: leds: is31fl319x: Document variants specificities
  leds: is31fl319x: Add missing si-en compatibles
  leds: is31fl319x: Use non-wildcard names for vars, structs and defines
  leds: is31fl319x: Move chipset-specific values in chipdef struct
  leds: is31fl319x: Add support for is31fl319{0,1,3} chips

 .../bindings/leds/issi,is31fl319x.yaml        | 193 +++++++++
 .../bindings/leds/leds-is31fl319x.txt         |  61 ---
 drivers/leds/leds-is31fl319x.c                | 406 +++++++++++++-----
 3 files changed, 488 insertions(+), 172 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/issi,is31fl319x.yaml
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-is31fl319x.txt

-- 
2.35.3
Re: [PATCH v3 0/6] leds: Fix/Add is31fl319{0,1,3} support
Posted by Andy Shevchenko 3 years, 9 months ago
On Tue, Jul 5, 2022 at 6:32 PM Vincent Knecht <vincent.knecht@mailoo.org> wrote:
>
> v3:
> - pick up Rob's R-b for patches 1 and 2
> - reinstate bindings docs license and maintainer
>   changes with Nikolaus agreement
> - took Andy's comments on patch 4 into account

Thanks for the update. Nothing serious, but a few comments.

Also a question here. Do you have plans to convert it to use device properties?

> The is31fl3190, is31fl3191 and is31fl3193 chips (1 or 3 PWM channels)
> cannot be handled the same as is31fl3196 and is31fl3199,
> if only because the register map is different.
> Also:
> - the software shutdown bit is reversed
> - and additional field needs to be set to enable all channels
> - the led-max-microamp current values and setting are not the same
>
> Datasheets:
> https://lumissil.com/assets/pdf/core/IS31FL3190_DS.pdf
> https://lumissil.com/assets/pdf/core/IS31FL3191_DS.pdf
> https://lumissil.com/assets/pdf/core/IS31FL3193_DS.pdf
> https://lumissil.com/assets/pdf/core/IS31FL3196_DS.pdf
> https://lumissil.com/assets/pdf/core/IS31FL3199_DS.pdf
>
> This series:
>
> - converts dt-bindings to dtschema, adding all si-en compatibles
>   for convenience and consistency, and adding constraints on
>   supported values for eg. reg address and led-max-microamp
>
> - changes vars, structs and defines to not use 319X suffix
>   but 3190 for 319{0,1,3} and 3196 for 319{6,9}
>
> - adds fields in chipdef struct for chip-specific values
>
> - only in the last patch, adds is31fl319{0,1,3} specific values
>   so those chips can work.
>
> Tested on msm8916-alcatel-idol347, which probably has an
> si-en,sn3190 or si-en,sn3191 (only one white led indicator).

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 0/6] leds: Fix/Add is31fl319{0,1,3} support
Posted by Vincent Knecht 3 years, 9 months ago
Le mardi 05 juillet 2022 à 20:57 +0200, Andy Shevchenko a écrit :
> On Tue, Jul 5, 2022 at 6:32 PM Vincent Knecht <vincent.knecht@mailoo.org> wrote:
> > 
> > v3:
> > - pick up Rob's R-b for patches 1 and 2
> > - reinstate bindings docs license and maintainer
> >   changes with Nikolaus agreement
> > - took Andy's comments on patch 4 into account
> 
> Thanks for the update. Nothing serious, but a few comments.
> 
> Also a question here. Do you have plans to convert it to use device properties?

Hi Andy, and thank you for the reviews.
Just sent a v4:
https://lore.kernel.org/linux-leds/20220709094642.4078222-1-vincent.knecht@mailoo.org/

As for converting to device properties, it will take me some more time
since I'm not too familiar with the concepts and how to do it exactly.
Got some hints from git history, will look into that.
Also I'd like to add blink support...
Re: [PATCH v3 0/6] leds: Fix/Add is31fl319{0,1,3} support
Posted by Andy Shevchenko 3 years, 9 months ago
On Sat, Jul 9, 2022 at 11:51 AM Vincent Knecht
<vincent.knecht@mailoo.org> wrote:
> Le mardi 05 juillet 2022 à 20:57 +0200, Andy Shevchenko a écrit :
> > On Tue, Jul 5, 2022 at 6:32 PM Vincent Knecht <vincent.knecht@mailoo.org> wrote:
> > >
> > > v3:
> > > - pick up Rob's R-b for patches 1 and 2
> > > - reinstate bindings docs license and maintainer
> > >   changes with Nikolaus agreement
> > > - took Andy's comments on patch 4 into account
> >
> > Thanks for the update. Nothing serious, but a few comments.
> >
> > Also a question here. Do you have plans to convert it to use device properties?
>
> Hi Andy, and thank you for the reviews.
> Just sent a v4:
> https://lore.kernel.org/linux-leds/20220709094642.4078222-1-vincent.knecht@mailoo.org/
>
> As for converting to device properties, it will take me some more time
> since I'm not too familiar with the concepts and how to do it exactly.

I just sent a series [1] which is based on the top of yours. You can
rebase it (it fixes a couple of subtle bugs: 1) GPIO error messaging
during error probe, and 2) mutex destruction is out of order) or
handle it together with yours. It seems you possess hardware, so I
assume you at least can test it.

> Got some hints from git history, will look into that.
> Also I'd like to add blink support...

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v3 0/6] leds: Fix/Add is31fl319{0,1,3} support
Posted by Andy Shevchenko 3 years, 9 months ago
On Mon, Jul 11, 2022 at 11:39 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Sat, Jul 9, 2022 at 11:51 AM Vincent Knecht
> <vincent.knecht@mailoo.org> wrote:
> > Le mardi 05 juillet 2022 à 20:57 +0200, Andy Shevchenko a écrit :
> > > On Tue, Jul 5, 2022 at 6:32 PM Vincent Knecht <vincent.knecht@mailoo.org> wrote:
> > > >
> > > > v3:
> > > > - pick up Rob's R-b for patches 1 and 2
> > > > - reinstate bindings docs license and maintainer
> > > >   changes with Nikolaus agreement
> > > > - took Andy's comments on patch 4 into account
> > >
> > > Thanks for the update. Nothing serious, but a few comments.
> > >
> > > Also a question here. Do you have plans to convert it to use device properties?
> >
> > Hi Andy, and thank you for the reviews.
> > Just sent a v4:
> > https://lore.kernel.org/linux-leds/20220709094642.4078222-1-vincent.knecht@mailoo.org/
> >
> > As for converting to device properties, it will take me some more time
> > since I'm not too familiar with the concepts and how to do it exactly.
>
> I just sent a series [1] which is based on the top of yours. You can
> rebase it (it fixes a couple of subtle bugs: 1) GPIO error messaging
> during error probe, and 2) mutex destruction is out of order) or
> handle it together with yours. It seems you possess hardware, so I
> assume you at least can test it.

...missed link...

[1]: https://lore.kernel.org/linux-leds/20220711213524.3587-1-andriy.shevchenko@linux.intel.com/

> > Got some hints from git history, will look into that.
> > Also I'd like to add blink support...

-- 
With Best Regards,
Andy Shevchenko