mbox series

[v8,0/7] KTD2026 indicator LED for X86 Xiaomi Pad2

Message ID 20240502211425.8678-1-hdegoede@redhat.com
Headers show
Series KTD2026 indicator LED for X86 Xiaomi Pad2 | expand

Message

Hans de Goede May 2, 2024, 9:14 p.m. UTC
Hi All,

Here is v8 of Kate's series to add support for Xiaomi Pad2 indicator LED.

I believe this is ready for merging now. Patch 6/7 has an Acked-by from
Sebastien for merging this patch through the leds tree since it depends
on the earlier patches. LEDs tree maintainers please merge patches 1-6,
then patch 7 can be merged through the pdx86 tree indepdently.

This work includes:
1. Added the KTD2026 swnode description to describe the LED controller.
2. Migrated the original driver to fwnode to support x86 platform.
3. Support for multi-color LED trigger event.
4. The LED shows orange  when charging and the LED shows green when the
   battery is full.

Moreover, the LED trigger is set to the new trigger, called
"bq27520-0-charging-orange-full-green" for Xiaomi Pad2 so the LED shows
orange when charging and the LED shows green when the battery is full.

--
Changes in v8:
1. New bugfix: "leds: rgb: leds-ktd202x: Initialize mutex earlier"
2. Make charging_orange_full_green triggers set the colors in RGB order
3. Modify the Pad2 ktd202x fwnode to have the colors in RGB order

Changes in v7:
1. Platform: x86-android-tablets: other: Add swnode for Xiaomi pad2
   indicator LED was included in Hans' branch.
2. Included the tags from the previous version in the commit message.
3. Fixed the comma issue for the structure initialiser.

Changes in v6:
1. The I2C ID table was moved to a separate patch.
2. The LED shows orange when charging.
3. The trigger name was renamed to charging-orange-full-green.
4. The default trigger of Xiaomi Pad2 is
   "bq27520-0-charging-orange-full-green".

Changes in v5:
1. Fix swnode LED color settings.
2. Improve the driver based on the comments.
3. Introduce a LED new API- led_mc_trigger_event() to make the LED
   color can be changed according to the trigger.
4. Introduced a new trigger "charging-red-full-green". The LED will be
   red when charging and the LED will be green when the battery is full.
5. Set the default trigger to "bq27520-0-charging-red-full-green" for
   Xiaomi Pad2.

Changes in v4:
1. Fix double casting.
2. Since force casting a pointer value to int will trigger a compiler
   warning, the type of num_leds was changed to unsigned long.

Changes in v3:
1. Drop the patch "leds-ktd202x: Skip regulator settings for Xiaomi
   pad2"

Changes in v2:
1. Typo and style fixes.
2. The patch 0003 skips all the regulator setup for Xiaomi pad2 since
   KTD2026 on Xiaomi pad2 is already powered by BP25890RTWR. So, the
   sleep can be removed when removing the module.

Regards,

Hans


Hans de Goede (3):
  leds: rgb: leds-ktd202x: Initialize mutex earlier
  leds: core: Add led_mc_set_brightness() function
  leds: trigger: Add led_mc_trigger_event() function

Kate Hsuan (4):
  leds: rgb: leds-ktd202x: Get device properties through fwnode to
    support ACPI
  leds: rgb: leds-ktd202x: I2C ID tables for KTD2026 and 2027
  power: supply: power-supply-leds: Add charging_orange_full_green
    trigger for RGB LED
  platform: x86-android-tablets: Xiaomi pad2 RGB LED fwnode updates

 drivers/leds/led-class-multicolor.c           |  1 +
 drivers/leds/led-core.c                       | 31 +++++++
 drivers/leds/led-triggers.c                   | 20 +++++
 drivers/leds/rgb/Kconfig                      |  1 -
 drivers/leds/rgb/leds-ktd202x.c               | 84 +++++++++++--------
 .../platform/x86/x86-android-tablets/other.c  |  6 +-
 drivers/power/supply/power_supply_leds.c      | 23 +++++
 include/linux/leds.h                          | 26 ++++++
 include/linux/power_supply.h                  |  2 +
 9 files changed, 156 insertions(+), 38 deletions(-)

Comments

Andy Shevchenko May 3, 2024, 3:43 a.m. UTC | #1
On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> The mutex must be initialized before the LED class device is registered
> otherwise there is a race where it may get used before it is initialized:
>
>  DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>  WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock
>  ...
>  RIP: 0010:__mutex_lock+0x7db/0xc10
>  ...
>  set_brightness_delayed_set_brightness.part.0+0x17/0x60
>  set_brightness_delayed+0xf1/0x100
>  process_one_work+0x222/0x5a0

...

> +       mutex_init(&chip->mutex);

devm_mutex_init() ?

...

There is an immutable branch (in case of this series going behind LED
subsystem):
https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git/log/?h=ib-leds-locking-6.10
Andy Shevchenko May 3, 2024, 4:12 a.m. UTC | #2
On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi All,
>
> Here is v8 of Kate's series to add support for Xiaomi Pad2 indicator LED.
>
> I believe this is ready for merging now. Patch 6/7 has an Acked-by from
> Sebastien for merging this patch through the leds tree since it depends
> on the earlier patches. LEDs tree maintainers please merge patches 1-6,
> then patch 7 can be merged through the pdx86 tree indepdently.

independently

>
> This work includes:
> 1. Added the KTD2026 swnode description to describe the LED controller.
> 2. Migrated the original driver to fwnode to support x86 platform.
> 3. Support for multi-color LED trigger event.

for a / events

> 4. The LED shows orange  when charging and the LED shows green when the
>    battery is full.
>
> Moreover, the LED trigger is set to the new trigger, called
> "bq27520-0-charging-orange-full-green" for Xiaomi Pad2 so the LED shows
> orange when charging and the LED shows green when the battery is full.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

One comment regarding use of devm_mutex_init() instead.
Hans de Goede May 3, 2024, 6:53 a.m. UTC | #3
Hi,

On 5/3/24 5:43 AM, Andy Shevchenko wrote:
> On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> The mutex must be initialized before the LED class device is registered
>> otherwise there is a race where it may get used before it is initialized:
>>
>>  DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>>  WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock
>>  ...
>>  RIP: 0010:__mutex_lock+0x7db/0xc10
>>  ...
>>  set_brightness_delayed_set_brightness.part.0+0x17/0x60
>>  set_brightness_delayed+0xf1/0x100
>>  process_one_work+0x222/0x5a0
> 
> ...
> 
>> +       mutex_init(&chip->mutex);
> 
> devm_mutex_init() ?

That is not in Torvald's tree yet.

Regards,

Hans
Lee Jones May 3, 2024, 7:03 a.m. UTC | #4
On Fri, 03 May 2024, Hans de Goede wrote:

> Hi,
> 
> On 5/3/24 5:43 AM, Andy Shevchenko wrote:
> > On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> The mutex must be initialized before the LED class device is registered
> >> otherwise there is a race where it may get used before it is initialized:
> >>
> >>  DEBUG_LOCKS_WARN_ON(lock->magic != lock)
> >>  WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock
> >>  ...
> >>  RIP: 0010:__mutex_lock+0x7db/0xc10
> >>  ...
> >>  set_brightness_delayed_set_brightness.part.0+0x17/0x60
> >>  set_brightness_delayed+0xf1/0x100
> >>  process_one_work+0x222/0x5a0
> > 
> > ...
> > 
> >> +       mutex_init(&chip->mutex);
> > 
> > devm_mutex_init() ?
> 
> That is not in Torvald's tree yet.

Neither is this.  :)

Since we're nearly at -rc7, I think it's safe to say you have time.
Hans de Goede May 3, 2024, 1:57 p.m. UTC | #5
Hi,

On 5/3/24 9:03 AM, Lee Jones wrote:
> On Fri, 03 May 2024, Hans de Goede wrote:
> 
>> Hi,
>>
>> On 5/3/24 5:43 AM, Andy Shevchenko wrote:
>>> On Fri, May 3, 2024 at 12:14 AM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>
>>>> The mutex must be initialized before the LED class device is registered
>>>> otherwise there is a race where it may get used before it is initialized:
>>>>
>>>>  DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>>>>  WARNING: CPU: 2 PID: 2045 at kernel/locking/mutex.c:587 __mutex_lock
>>>>  ...
>>>>  RIP: 0010:__mutex_lock+0x7db/0xc10
>>>>  ...
>>>>  set_brightness_delayed_set_brightness.part.0+0x17/0x60
>>>>  set_brightness_delayed+0xf1/0x100
>>>>  process_one_work+0x222/0x5a0
>>>
>>> ...
>>>
>>>> +       mutex_init(&chip->mutex);
>>>
>>> devm_mutex_init() ?
>>
>> That is not in Torvald's tree yet.
> 
> Neither is this.  :)
> 
> Since we're nearly at -rc7, I think it's safe to say you have time.

Ok I'll prepare a v9 with this addressed and Andy's Reviewed-by
added.

Regards,

Hans