mbox series

[v3,0/4] leds: pca955x: Add HW blink support

Message ID 20220411162033.39613-1-eajames@linux.ibm.com
Headers show
Series leds: pca955x: Add HW blink support | expand

Message

Eddie James April 11, 2022, 4:20 p.m. UTC 
This series adds support for blinking using the PCA955x chip, falling
back to software blinking if another LED on the chip is already blinking
at a different rate, or if the requested rate isn't representable with
the PCA955x.
Also included are some minor clean up and optimization changes that make
the HW blinking a bit easier.

Changes since v2:
 - Split the cleanup patch
 - Prettier dev_err calls
 - Include units for blink period and use defined unit translations
   rather than just a number.
 - Use positive conditionals.

Changes since v1:
 - Rework the blink function to fallback to software blinking if the
   period is out of range of the chip's capabilities or if another LED
   on the chip is already blinking at a different rate.
 - Add the cleanup patch

Eddie James (4):
  leds: pca955x: Refactor with helper functions and renaming
  leds: pca955x: Use pointers to driver data rather than I2C client
  leds: pca955x: Optimize probe led selection
  leds: pca955x: Add HW blink support

 drivers/leds/leds-pca955x.c | 341 ++++++++++++++++++++++++------------
 1 file changed, 232 insertions(+), 109 deletions(-)

Comments

Andy Shevchenko April 11, 2022, 5:06 p.m. UTC | #1 
On Mon, Apr 11, 2022 at 7:20 PM Eddie James <eajames@linux.ibm.com> wrote:
>
> This series adds support for blinking using the PCA955x chip, falling
> back to software blinking if another LED on the chip is already blinking
> at a different rate, or if the requested rate isn't representable with
> the PCA955x.
> Also included are some minor clean up and optimization changes that make
> the HW blinking a bit easier.

Don't see any big issues here, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

But please consider two things for the future improvements:
1) switching to regmap API;
2) switching to bitmap API.

You may find some code in drivers/gpio/gpio-pca953x.c. It might be
that some common code can be split to the generic bitmap/regmap
operations.

> Changes since v2:
>  - Split the cleanup patch
>  - Prettier dev_err calls
>  - Include units for blink period and use defined unit translations
>    rather than just a number.
>  - Use positive conditionals.
>
> Changes since v1:
>  - Rework the blink function to fallback to software blinking if the
>    period is out of range of the chip's capabilities or if another LED
>    on the chip is already blinking at a different rate.
>  - Add the cleanup patch
>
> Eddie James (4):
>   leds: pca955x: Refactor with helper functions and renaming
>   leds: pca955x: Use pointers to driver data rather than I2C client
>   leds: pca955x: Optimize probe led selection
>   leds: pca955x: Add HW blink support
>
>  drivers/leds/leds-pca955x.c | 341 ++++++++++++++++++++++++------------
>  1 file changed, 232 insertions(+), 109 deletions(-)
>
> --
> 2.27.0
>