Messages in this thread |  | | From | Chen-Yu Tsai <> | Date | Mon, 6 Jan 2020 10:22:01 +0800 | Subject | Re: [linux-sunxi] [PATCH v2 7/9] power: supply: axp20x_usb_power: Allow offlining |
| |
On Mon, Jan 6, 2020 at 1:47 AM Samuel Holland <samuel@sholland.org> wrote: > > On 1/5/20 4:40 AM, Chen-Yu Tsai wrote: > > On Sun, Jan 5, 2020 at 9:24 AM Samuel Holland <samuel@sholland.org> wrote: > >> > >> AXP803/AXP813 have a flag that enables/disables the USB power supply > >> input. Allow control of this flag via the ONLINE property on those > >> variants. > >> > >> It may be necessary to offline the USB power supply input when using > >> the USB port in OTG mode, or to allow userspace to disable charging. > > > > Any idea how the former would be implemented? AFAIK this isn't allowed > > right now. > > Pinephone currently has AXP N_VBUSEN/DRIVEVBUS floating, so the hardware doesn't > automatically disable the VBUS path when enabling the boost regulator driving > it. This doubles the current draw from the battery. > > The USB PHY driver would need to call: > > union power_supply_propval val = { .intval = false }; > power_supply_set_property(data->vbus_power_supply, > POWER_SUPPLY_PROP_ONLINE, &val); > > or similar to set VBUS offline in sun4i_usb_phy_power_on(), and set it back > online in sun4i_usb_phy_power_off().
Ah, OK. That's a valid use case. I had something else in mind, the OTG host mode with charger one.
> > As for disabling charging, wouldn't it make more sense to disable the > > charger? > > Yes, I see now that there's a bit at 33H[7] for this. I don't see an obvious > property to hook it up to, though. Maybe POWER_SUPPLY_PROP_CHARGE_TYPE == > POWER_SUPPLY_CHARGE_TYPE_NONE?
Maybe? I suppose the sysfs ABI docs would have some clues.
> > Either way, these are not directly related to the changes. I'm just curious. > > > >> When the USB VBUS input is disabled via the PATH_SEL bit, the VBUS_USED > >> bit in PWR_INPUT_STATUS is cleared, so there is no change needed when > >> getting the property. > >> > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > >> --- > >> drivers/power/supply/axp20x_usb_power.c | 27 +++++++++++++++++++++++++ > >> 1 file changed, 27 insertions(+) > >> > >> diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c > >> index 2d7272e19a87..68443f264dff 100644 > >> --- a/drivers/power/supply/axp20x_usb_power.c > >> +++ b/drivers/power/supply/axp20x_usb_power.c > >> @@ -29,6 +29,9 @@ > >> > >> #define AXP20X_USB_STATUS_VBUS_VALID BIT(2) > >> > >> +#define AXP20X_VBUS_PATH_SEL BIT(7) > >> +#define AXP20X_VBUS_PATH_SEL_OFFSET 7 > >> + > >> #define AXP20X_VBUS_VHOLD_uV(b) (4000000 + (((b) >> 3) & 7) * 100000) > >> #define AXP20X_VBUS_VHOLD_MASK GENMASK(5, 3) > >> #define AXP20X_VBUS_VHOLD_OFFSET 3 > >> @@ -263,6 +266,16 @@ static int axp20x_usb_power_get_property(struct power_supply *psy, > >> return 0; > >> } > >> > >> +static int axp813_usb_power_set_online(struct axp20x_usb_power *power, > >> + int intval) > >> +{ > >> + int val = !intval << AXP20X_VBUS_PATH_SEL_OFFSET; > >> + > >> + return regmap_update_bits(power->regmap, > >> + AXP20X_VBUS_IPSOUT_MGMT, > >> + AXP20X_VBUS_PATH_SEL, val); > >> +} > >> + > >> static int axp20x_usb_power_set_voltage_min(struct axp20x_usb_power *power, > >> int intval) > >> { > >> @@ -344,6 +357,9 @@ static int axp20x_usb_power_set_property(struct power_supply *psy, > >> struct axp20x_usb_power *power = power_supply_get_drvdata(psy); > >> > >> switch (psp) { > >> + case POWER_SUPPLY_PROP_ONLINE: > >> + return axp813_usb_power_set_online(power, val->intval); > >> + > > > > I would add a comment here pointing to the next change as to why there's > > only an axp813-specific callback used here. > > I'll add this for v3.
Thanks ChenYu
> >> case POWER_SUPPLY_PROP_VOLTAGE_MIN: > >> return axp20x_usb_power_set_voltage_min(power, val->intval); > >> > >> @@ -363,6 +379,17 @@ static int axp20x_usb_power_set_property(struct power_supply *psy, > >> static int axp20x_usb_power_prop_writeable(struct power_supply *psy, > >> enum power_supply_property psp) > >> { > >> + struct axp20x_usb_power *power = power_supply_get_drvdata(psy); > >> + > >> + /* > >> + * Both AXP2xx and AXP8xx have a VBUS path select flag. > >> + * On AXP2xx, setting the flag enables VBUS (ignoring N_VBUSEN). > >> + * On AXP8xx, setting the flag disables VBUS (ignoring N_VBUSEN). > >> + * So we only expose the control on AXP8xx where it is meaningful. > >> + */ > >> + if (psp == POWER_SUPPLY_PROP_ONLINE) > >> + return power->axp20x_id == AXP813_ID; > >> + > >> return psp == POWER_SUPPLY_PROP_VOLTAGE_MIN || > >> psp == POWER_SUPPLY_PROP_CURRENT_MAX; > >> } > >> -- > > > > Otherwise, > > > > Reviewed-by: Chen-Yu Tsai <wens@csie.org> > > > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/f0c5e260-dcc3-2744-21cd-305e4534f2be%40sholland.org.
|  |