| Message ID | 20240417-max8903-v1-1-25d4dbf4ce9b@herrie.org |
|---|---|
| State | New |
| Headers | show |
| Series | power: supply: max8903: configure USUS as output | expand |
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> >
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. + */ + 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");