Re: [PATCH 2/2] power: supply: leds: add a not-charging trigger

[Steffen Dirkwinkel]

Hi,

On Tue, 2026-07-21 at 00:23 +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Thu, Jun 25, 2026 at 07:07:35PM +0200, Steffen Dirkwinkel wrote:
> > From: Steffen Dirkwinkel <s.dirkwinkel@xxxxxxxxxxxx>
> > 
> > We intend to use this with a gpio-charger ups device that reports
> > charging or not-charging based on a gpio to set a "power failure" led.
> > 
> > Signed-off-by: Steffen Dirkwinkel <s.dirkwinkel@xxxxxxxxxxxx>
> > ---
> >  drivers/power/supply/power_supply_leds.c | 7 +++++++
> >  include/linux/power_supply.h             | 1 +
> >  2 files changed, 8 insertions(+)
> 
> POWER_SUPPLY_STATUS_NOT_CHARGING means, that a battery is neither
> charged **nor discharged**. I think the NOT_CHARGING status has a
> bad name, but it's ABI and cannot be changed easily. But I certainly
> don't want it to spread further. Let's find a better name for this
> trigger. Maybe idle?

Hm, I guess I was also mislead by the name, as it's somewhat correct for my
usecase, our gpio connected ups is indicating "not-charging" via a gpio and I'm
using it as a gpio-charger charge-status-gpio.

I guess it would be more correct if I could map the gpio-charger charge status
to switch between charging and discharging instead of indicating not-charging.
If you agree I'd add that as an option via device tree binding there and add a
discharging trigger instead of the not charging one to the led triggers:

  charge-status-discharging:
      type: boolean
      description:
        Interpret a deasserted charge-status-gpio as "discharging" instead of
	"not charging".

Thanks,
Steffen

> 
> Greetings,
> 
> -- Sebastian
> 
> > diff --git a/drivers/power/supply/power_supply_leds.c b/drivers/power/supply/power_supply_leds.c
> > index 538cc0440e53..c82a3628765c 100644
> > --- a/drivers/power/supply/power_supply_leds.c
> > +++ b/drivers/power/supply/power_supply_leds.c
> > @@ -131,6 +131,10 @@ static void power_supply_update_status_leds(struct power_supply *psy)
> >  			LED_OFF);
> >  		break;
> >  	}
> > +
> > +	led_trigger_event(psy->not_charging_trig,
> > +			  status.intval == POWER_SUPPLY_STATUS_NOT_CHARGING ?
> > +			  LED_FULL : LED_OFF);
> >  }
> >  
> >  static int power_supply_create_status_triggers(struct power_supply *psy)
> > @@ -150,6 +154,8 @@ static int power_supply_create_status_triggers(struct power_supply *psy)
> >  					  &psy->charging_blink_full_solid_trig, &err);
> >  	power_supply_register_led_trigger(psy, "%s-charging-orange-full-green",
> >  					  &psy->charging_orange_full_green_trig, &err);
> > +	power_supply_register_led_trigger(psy, "%s-not-charging",
> > +					  &psy->not_charging_trig, &err);
> >  
> >  	return err;
> >  }
> > @@ -209,6 +215,7 @@ void power_supply_remove_triggers(struct power_supply *psy)
> >  	power_supply_unregister_led_trigger(psy->online_trig);
> >  	power_supply_unregister_led_trigger(psy->charging_or_full_trig);
> >  	power_supply_unregister_led_trigger(psy->charging_trig);
> > +	power_supply_unregister_led_trigger(psy->not_charging_trig);
> >  	power_supply_unregister_led_trigger(psy->full_trig);
> >  	power_supply_unregister_led_trigger(psy->charging_blink_full_solid_trig);
> >  	power_supply_unregister_led_trigger(psy->charging_orange_full_green_trig);
> > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> > index 68b65682b14d..9052610e3910 100644
> > --- a/include/linux/power_supply.h
> > +++ b/include/linux/power_supply.h
> > @@ -349,6 +349,7 @@ struct power_supply {
> >  	struct led_trigger *charging_or_full_trig;
> >  	struct led_trigger *online_trig;
> >  	struct led_trigger *charging_trig;
> > +	struct led_trigger *not_charging_trig;
> >  	struct led_trigger *full_trig;
> >  	struct led_trigger *charging_blink_full_solid_trig;
> >  	struct led_trigger *charging_orange_full_green_trig;
> > 
> > -- 
> > 2.54.0
> >