Messages in this thread | | | Date | Tue, 30 Apr 2024 16:33:16 +0200 | From | Sebastian Reichel <> | Subject | Re: [PATCH] power: supply: max8903: configure USUS as output |
| |
Hi,
On Wed, Apr 17, 2024 at 03:35:00PM +0200, Herman van Hazendonk wrote: > The USUS pin was mistakenly configured as an input, when it should be an > output that specifies whether the USB power input is suspended. In addition > to fixing the pin mode, this patch also suspends the USB input when a DC > charger is connected, which should result in a slight reduction in USB > quiescent current. > > Signed-off-by: Ben Wolsieffer <benwolsieffer@gmail.com> > Signed-off-by: Herman van Hazendonk <github.com@herrie.org> > ---
Thanks for your patch. I think this should get
Fixes: 50da8d04ee52 ("power: supply: max8903: Convert to GPIO descriptors")
Also what's going on with the Signed-off-by? Did you forward the patch from Ben Wolsieffer (i.e. the author is wrong) or did you co-develop it (so there should be a "Co-developed-by: Ben Wolsieffer <benwolsieffer@gmail.com>" tag)?
> drivers/power/supply/max8903_charger.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/max8903_charger.c b/drivers/power/supply/max8903_charger.c > index e65d0141f260..15dc3a5239e2 100644 > --- a/drivers/power/supply/max8903_charger.c > +++ b/drivers/power/supply/max8903_charger.c > @@ -102,6 +102,10 @@ static irqreturn_t max8903_dcin(int irq, void *_data) > if (data->dcm) > gpiod_set_value(data->dcm, ta_in); > > + /* Set USB-Suspend 1:Suspended 0:Active */ > + if (data->usus) > + gpiod_set_value(data->usus, ta_in); > + > /* Charger Enable / Disable */ > if (data->cen) { > int val; > @@ -310,7 +314,15 @@ static int max8903_setup_gpios(struct platform_device *pdev) > "failed to get FLT GPIO"); > gpiod_set_consumer_name(data->flt, data->psy_desc.name); > > - data->usus = devm_gpiod_get_optional(dev, "usus", GPIOD_IN); > + /* > + * Suspend the USB input if the DC charger is connected. > + * > + * The USUS line should be marked GPIO_ACTIVE_HIGH in the > + * device tree. Driving it low will enable the USB charger > + * input.
I think the above 3 lines should be dropped. They added more confusion to me than actually helping and the information is already in the DT binding file.
Greetings,
-- Sebastian
> + */ > + flags = ta_in ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW; > + data->usus = devm_gpiod_get_optional(dev, "usus", flags); > if (IS_ERR(data->usus)) > return dev_err_probe(dev, PTR_ERR(data->usus), > "failed to get USUS GPIO"); > > --- > base-commit: 96fca68c4fbf77a8185eb10f7557e23352732ea2 > change-id: 20240417-max8903-ace97d7d3407 > > Best regards, > -- > Herman van Hazendonk <github.com@herrie.org> > [unhandled content-type:application/pgp-signature] | |