Re: [PATCH 0/8] Add WLED support to TPS68470 LED driver

[Hans de Goede <hdegoede@redhat.com>]

Hi Daniel,

On 3/22/23 17:09, Daniel Scally wrote:
> This series relies on the recent "leds: tps68470: LED driver for TPS68470" set
> from Kate Hsuan [1]
> 
> The TPS68470 provides two additional LED outputs on top of the indicator LEDs.
> Add support for those to the driver. The configuration of the chip is drawn from
> platform data which is expected to be passed to the driver. Additionally update
> the int3472-tps68470 driver to register led lookups from platform data so that
> the right LED is driven for each sensor, and finally define those lookups for
> the Microsoft Surface Go line.
> 
> Kate, Hans, this is the changes I made on top of the tps68470-led series to
> enable the IR LED on my Go2 (plus one additional patch to media). #5 could
> probably just be squashed into the other series though.

Ack for squashing 5 into Kate's patch.

> The last two patches
> cover how I think the LED lookup should work - I unfortunately can't see an
> automatic way to guarantee the right LED goes to the right sensor.

I did not get around to replying to your review of my lookup patch. I believe
adding this to the board-data as you have done here is fine.

I had a few small comments on your patches and I believe that Kate's patches
are ready for merging now.

So I believe that for the next version it would be best to merge the 2 series
into 1 big series. Putting all "leds: tps68470: ..." patches first. Then once
they are all reviewed Lee can merge the entire drivers/leds/ part of
the series and send me an IB branch to pull from before I merge
the drivers/platform/x86/intel/int3472/ parts.

Kate, is it ok with you if Daniel includes your patches (keeping you as
author of course) in the next version of this series ?

Regards,

Hans





> [1] https://lore.kernel.org/platform-driver-x86/20230321153718.1355511-1-hpa@redhat.com/T/
> 
> Daniel Scally (8):
>   platform/x86: int3472: Add platform data for LEDs
>   platform/x86: int3472: Init LED registers using platform data
>   platform/x86: int3472: Add TPS68470 LED Board Data
>   platform/x86: int3472: Add tps68470-led as clock consumer
>   leds: tps68470: Refactor tps68470_brightness_get()
>   leds: tps68470: Support the WLED driver
>   platform/x86: int3472: Support LED lookups in board data
>   platform/x86: int3472: Define LED lookup data for MS Surface Go
> 
>  drivers/leds/leds-tps68470.c                  | 170 +++++++++++++++++-
>  drivers/platform/x86/intel/int3472/tps68470.c |  31 +++-
>  drivers/platform/x86/intel/int3472/tps68470.h |  10 ++
>  .../x86/intel/int3472/tps68470_board_data.c   |  31 ++++
>  include/linux/mfd/tps68470.h                  |  12 ++
>  include/linux/platform_data/tps68470.h        |  11 ++
>  6 files changed, 248 insertions(+), 17 deletions(-)
>