| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4e0dfa1b-3712-430f-a97d-f4763c2740e5@redhat.com>
Date: Thu, 2 May 2024 16:15:59 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Kate Hsuan <hpa@...hat.com>, Pavel Machek <pavel@....cz>,
Lee Jones <lee@...nel.org>, linux-leds@...r.kernel.org,
platform-driver-x86@...r.kernel.org,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
André Apitzsch <git@...tzsch.eu>,
linux-kernel@...r.kernel.org, Andy Shevchenko <andy.shevchenko@...il.com>,
Sebastian Reichel <sre@...nel.org>, linux-pm@...r.kernel.org
Cc: Sebastian Reichel <sebastian.reichel@...labora.com>
Subject: Re: [PATCH v7 5/6] power: supply: power-supply-leds: Add
charging_orange_full_green trigger for RGB LED
Hi,
On 4/29/24 1:10 PM, Hans de Goede wrote:
> Hi,
>
> On 4/24/24 8:52 AM, Kate Hsuan wrote:
>> Add a charging_orange_full_green LED trigger and the trigger is based on
>> led_mc_trigger_event() which can set an RGB LED when the trigger is
>> triggered. The LED will show orange when the battery status is charging.
>> The LED will show green when the battery status is full.
>>
>> Link: https://lore.kernel.org/linux-leds/f40a0b1a-ceac-e269-c2dd-0158c5b4a1ad@gmail.com/
>>
>> Signed-off-by: Kate Hsuan <hpa@...hat.com>
>> Acked-by: Sebastian Reichel <sebastian.reichel@...labora.com>
>
> Thanks, patch looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@...hat.com>
I withdraw this I just noticed that this:
+ unsigned int intensity_green[3] = {255, 0, 0};
+ unsigned int intensity_orange[3] = {128, 0, 255};
Is based on the color order being "green blue red" which
is a pretty exotic color order. The device tree / fwnode bindings
for the ktd2026 with which we are using this allow changing
the order to red green blue simply by listing the child cells
for each color in that order.
Also this:
+ unsigned int intensity_red[3] = {0, 0, 255};
is not necessary since it is only used with LED_OFF
as brightness the code can simply use:
led_trigger_event(psy->charging_orange_full_green_trig, LED_OFF);
to turn the LED off.
I'll prepare a new version of this series with this fixed.
Regards,
Hans
>> ---
>> drivers/power/supply/power_supply_leds.c | 26 ++++++++++++++++++++++++
>> include/linux/power_supply.h | 2 ++
>> 2 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
>> index c7db29d5fcb8..8dd99199c65b 100644
>> --- a/drivers/power/supply/power_supply_leds.c
>> +++ b/drivers/power/supply/power_supply_leds.c
>> @@ -22,6 +22,9 @@
>> static void power_supply_update_bat_leds(struct power_supply *psy)
>> {
>> union power_supply_propval status;
>> + unsigned int intensity_green[3] = {255, 0, 0};
>> + unsigned int intensity_orange[3] = {128, 0, 255};
>> + unsigned int intensity_red[3] = {0, 0, 255};
>>
>> if (power_supply_get_property(psy, POWER_SUPPLY_PROP_STATUS, &status))
>> return;
>> @@ -36,12 +39,20 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
>> /* Going from blink to LED on requires a LED_OFF event to stop blink */
>> led_trigger_event(psy->charging_blink_full_solid_trig, LED_OFF);
>> led_trigger_event(psy->charging_blink_full_solid_trig, LED_FULL);
>> + led_mc_trigger_event(psy->charging_orange_full_green_trig,
>> + intensity_green,
>> + ARRAY_SIZE(intensity_green),
>> + LED_FULL);
>> break;
>> case POWER_SUPPLY_STATUS_CHARGING:
>> led_trigger_event(psy->charging_full_trig, LED_FULL);
>> led_trigger_event(psy->charging_trig, LED_FULL);
>> led_trigger_event(psy->full_trig, LED_OFF);
>> led_trigger_blink(psy->charging_blink_full_solid_trig, 0, 0);
>> + led_mc_trigger_event(psy->charging_orange_full_green_trig,
>> + intensity_orange,
>> + ARRAY_SIZE(intensity_orange),
>> + LED_FULL);
>> break;
>> default:
>> led_trigger_event(psy->charging_full_trig, LED_OFF);
>> @@ -49,6 +60,10 @@ static void power_supply_update_bat_leds(struct power_supply *psy)
>> led_trigger_event(psy->full_trig, LED_OFF);
>> led_trigger_event(psy->charging_blink_full_solid_trig,
>> LED_OFF);
>> + led_mc_trigger_event(psy->charging_orange_full_green_trig,
>> + intensity_red,
>> + ARRAY_SIZE(intensity_red),
>> + LED_OFF);
>> break;
>> }
>> }
>> @@ -74,6 +89,11 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
>> if (!psy->charging_blink_full_solid_trig_name)
>> goto charging_blink_full_solid_failed;
>>
>> + psy->charging_orange_full_green_trig_name = kasprintf(GFP_KERNEL,
>> + "%s-charging-orange-full-green", psy->desc->name);
>> + if (!psy->charging_orange_full_green_trig_name)
>> + goto charging_red_full_green_failed;
>> +
>> led_trigger_register_simple(psy->charging_full_trig_name,
>> &psy->charging_full_trig);
>> led_trigger_register_simple(psy->charging_trig_name,
>> @@ -82,9 +102,13 @@ static int power_supply_create_bat_triggers(struct power_supply *psy)
>> &psy->full_trig);
>> led_trigger_register_simple(psy->charging_blink_full_solid_trig_name,
>> &psy->charging_blink_full_solid_trig);
>> + led_trigger_register_simple(psy->charging_orange_full_green_trig_name,
>> + &psy->charging_orange_full_green_trig);
>>
>> return 0;
>>
>> +charging_red_full_green_failed:
>> + kfree(psy->charging_blink_full_solid_trig_name);
>> charging_blink_full_solid_failed:
>> kfree(psy->full_trig_name);
>> full_failed:
>> @@ -101,10 +125,12 @@ static void power_supply_remove_bat_triggers(struct power_supply *psy)
>> led_trigger_unregister_simple(psy->charging_trig);
>> led_trigger_unregister_simple(psy->full_trig);
>> led_trigger_unregister_simple(psy->charging_blink_full_solid_trig);
>> + led_trigger_unregister_simple(psy->charging_orange_full_green_trig);
>> kfree(psy->charging_blink_full_solid_trig_name);
>> kfree(psy->full_trig_name);
>> kfree(psy->charging_trig_name);
>> kfree(psy->charging_full_trig_name);
>> + kfree(psy->charging_orange_full_green_trig_name);
>> }
>>
>> /* Generated power specific LEDs triggers. */
>> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
>> index c0992a77feea..9b6898085224 100644
>> --- a/include/linux/power_supply.h
>> +++ b/include/linux/power_supply.h
>> @@ -318,6 +318,8 @@ struct power_supply {
>> char *online_trig_name;
>> struct led_trigger *charging_blink_full_solid_trig;
>> char *charging_blink_full_solid_trig_name;
>> + struct led_trigger *charging_orange_full_green_trig;
>> + char *charging_orange_full_green_trig_name;
>> #endif
>> };
>>
Powered by blists - more mailing lists