| Message ID | 20240121014057.1042466-7-aren@peacevolution.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: supply: axp20x_usb_power: prepare for external power delivery negotiation | expand |
Hello Aren, On Sat, Jan 20, 2024 at 08:40:04PM -0500, Aren Moynihan wrote: > axp803 sets the current limit to 3A by default if it doesn't detect a > battery. The datasheet says that reg 0x2D bit 6 is used to indicate > first power on status. According to it, if that bit is 0 and the > battery is not detected, it will set the input current limit to 3A, > however setting that bit to 1 doesn't to prevent the pmic from setting > the current limit to 3A. > > Fixes: c279adafe6ab ("power: supply: axp20x_usb_power: add support for AXP813") > > Signed-off-by: Aren Moynihan <aren@peacevolution.org> > --- > I'm not sure if the pmic simply ignores the first power on field, or if > it needs to be set in a specific way / at a specific time. I tried > setting it in arm-trusted-firmware, and the pmic still set the input > current limit to 3A. > > The datasheet for axp813 says it has the same first power on bit, but I > don't have hardware to test if it behaves the same way. This driver uses > the same platform data for axp803 and axp813. > > drivers/power/supply/axp20x_usb_power.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c > index 5656f6e57d54..95b136ee20f2 100644 > --- a/drivers/power/supply/axp20x_usb_power.c > +++ b/drivers/power/supply/axp20x_usb_power.c > @@ -52,6 +52,7 @@ struct axp_data { > const int *curr_lim_table; > int input_curr_lim_table_size; > const int *input_curr_lim_table; > + int force_input_curr_lim; > struct reg_field curr_lim_fld; > struct reg_field input_curr_lim_fld; > struct reg_field vbus_valid_bit; > @@ -599,6 +600,7 @@ static const struct axp_data axp813_data = { > .input_curr_lim_table_size = ARRAY_SIZE(axp_813_usb_input_curr_lim_table), > .input_curr_lim_table = axp_813_usb_input_curr_lim_table, > .input_curr_lim_fld = REG_FIELD(AXP22X_CHRG_CTRL3, 4, 7), > + .force_input_curr_lim = 500000, > .usb_bc_en_bit = REG_FIELD(AXP288_BC_GLOBAL, 0, 0), > .usb_bc_det_fld = REG_FIELD(AXP288_BC_DET_STAT, 5, 7), > .vbus_disable_bit = REG_FIELD(AXP20X_VBUS_IPSOUT_MGMT, 7, 7), > @@ -786,6 +788,17 @@ static int axp20x_usb_power_probe(struct platform_device *pdev) > return ret; > } > > + if (power->axp_data->force_input_curr_lim) { > + /* > + * Certain chips set the input current limit to 3A when there is > + * no battery connected. Normally the default is 500mA. > + */ > + ret = axp20x_usb_power_set_input_current_limit(power, > + power->axp_data->force_input_curr_lim); > + if (ret) > + return ret; > + } This will break the usecase of powering Pinephone from USB without a battery present (or fully discharged). The phone can require more than 500mA before USB power supply detection manages to run and sets the higher limit. Linux should keep the limit set by the bootloader until it figures out something better based on BC 1.2 or USB-PD. You should only change the limit as the result of negotiation. Kind regards, o. > if (power->usb_bc_en_bit) { > /* Enable USB Battery Charging specification detection */ > ret = regmap_field_write(power->usb_bc_en_bit, 1); > -- > 2.43.0 >
On Sun, Jan 21, 2024 at 12:08:37PM +0100, Ondřej Jirman wrote: > Hello Aren, > > On Sat, Jan 20, 2024 at 08:40:04PM -0500, Aren Moynihan wrote: > > axp803 sets the current limit to 3A by default if it doesn't detect a > > battery. The datasheet says that reg 0x2D bit 6 is used to indicate > > first power on status. According to it, if that bit is 0 and the > > battery is not detected, it will set the input current limit to 3A, > > however setting that bit to 1 doesn't to prevent the pmic from setting > > the current limit to 3A. > > > > Fixes: c279adafe6ab ("power: supply: axp20x_usb_power: add support for AXP813") > > > > Signed-off-by: Aren Moynihan <aren@peacevolution.org> > > --- > > I'm not sure if the pmic simply ignores the first power on field, or if > > it needs to be set in a specific way / at a specific time. I tried > > setting it in arm-trusted-firmware, and the pmic still set the input > > current limit to 3A. > > > > The datasheet for axp813 says it has the same first power on bit, but I > > don't have hardware to test if it behaves the same way. This driver uses > > the same platform data for axp803 and axp813. > > > > drivers/power/supply/axp20x_usb_power.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c > > index 5656f6e57d54..95b136ee20f2 100644 > > --- a/drivers/power/supply/axp20x_usb_power.c > > +++ b/drivers/power/supply/axp20x_usb_power.c > > @@ -52,6 +52,7 @@ struct axp_data { > > const int *curr_lim_table; > > int input_curr_lim_table_size; > > const int *input_curr_lim_table; > > + int force_input_curr_lim; > > struct reg_field curr_lim_fld; > > struct reg_field input_curr_lim_fld; > > struct reg_field vbus_valid_bit; > > @@ -599,6 +600,7 @@ static const struct axp_data axp813_data = { > > .input_curr_lim_table_size = ARRAY_SIZE(axp_813_usb_input_curr_lim_table), > > .input_curr_lim_table = axp_813_usb_input_curr_lim_table, > > .input_curr_lim_fld = REG_FIELD(AXP22X_CHRG_CTRL3, 4, 7), > > + .force_input_curr_lim = 500000, > > .usb_bc_en_bit = REG_FIELD(AXP288_BC_GLOBAL, 0, 0), > > .usb_bc_det_fld = REG_FIELD(AXP288_BC_DET_STAT, 5, 7), > > .vbus_disable_bit = REG_FIELD(AXP20X_VBUS_IPSOUT_MGMT, 7, 7), > > @@ -786,6 +788,17 @@ static int axp20x_usb_power_probe(struct platform_device *pdev) > > return ret; > > } > > > > + if (power->axp_data->force_input_curr_lim) { > > + /* > > + * Certain chips set the input current limit to 3A when there is > > + * no battery connected. Normally the default is 500mA. > > + */ > > + ret = axp20x_usb_power_set_input_current_limit(power, > > + power->axp_data->force_input_curr_lim); > > + if (ret) > > + return ret; > > + } > > This will break the usecase of powering Pinephone from USB without a battery > present (or fully discharged). > > The phone can require more than 500mA before USB power supply detection manages > to run and sets the higher limit. Linux should keep the limit set by the > bootloader until it figures out something better based on BC 1.2 or USB-PD. > > You should only change the limit as the result of negotiation. Unfortunately BC 1.2 detection doesn't seem to be performed without a battery, at least I wasn't able to trigger it. This will be worth revising once we have a driver that can provide a signal that USB-PD is in progress and/or finished, but until then I'd prefer not take on that complexity. - Aren > Kind regards, > o. > > > if (power->usb_bc_en_bit) { > > /* Enable USB Battery Charging specification detection */ > > ret = regmap_field_write(power->usb_bc_en_bit, 1); > > -- > > 2.43.0 > >
diff --git a/drivers/power/supply/axp20x_usb_power.c b/drivers/power/supply/axp20x_usb_power.c index 5656f6e57d54..95b136ee20f2 100644 --- a/drivers/power/supply/axp20x_usb_power.c +++ b/drivers/power/supply/axp20x_usb_power.c @@ -52,6 +52,7 @@ struct axp_data { const int *curr_lim_table; int input_curr_lim_table_size; const int *input_curr_lim_table; + int force_input_curr_lim; struct reg_field curr_lim_fld; struct reg_field input_curr_lim_fld; struct reg_field vbus_valid_bit; @@ -599,6 +600,7 @@ static const struct axp_data axp813_data = { .input_curr_lim_table_size = ARRAY_SIZE(axp_813_usb_input_curr_lim_table), .input_curr_lim_table = axp_813_usb_input_curr_lim_table, .input_curr_lim_fld = REG_FIELD(AXP22X_CHRG_CTRL3, 4, 7), + .force_input_curr_lim = 500000, .usb_bc_en_bit = REG_FIELD(AXP288_BC_GLOBAL, 0, 0), .usb_bc_det_fld = REG_FIELD(AXP288_BC_DET_STAT, 5, 7), .vbus_disable_bit = REG_FIELD(AXP20X_VBUS_IPSOUT_MGMT, 7, 7), @@ -786,6 +788,17 @@ static int axp20x_usb_power_probe(struct platform_device *pdev) return ret; } + if (power->axp_data->force_input_curr_lim) { + /* + * Certain chips set the input current limit to 3A when there is + * no battery connected. Normally the default is 500mA. + */ + ret = axp20x_usb_power_set_input_current_limit(power, + power->axp_data->force_input_curr_lim); + if (ret) + return ret; + } + if (power->usb_bc_en_bit) { /* Enable USB Battery Charging specification detection */ ret = regmap_field_write(power->usb_bc_en_bit, 1);
axp803 sets the current limit to 3A by default if it doesn't detect a battery. The datasheet says that reg 0x2D bit 6 is used to indicate first power on status. According to it, if that bit is 0 and the battery is not detected, it will set the input current limit to 3A, however setting that bit to 1 doesn't to prevent the pmic from setting the current limit to 3A. Fixes: c279adafe6ab ("power: supply: axp20x_usb_power: add support for AXP813") Signed-off-by: Aren Moynihan <aren@peacevolution.org> --- I'm not sure if the pmic simply ignores the first power on field, or if it needs to be set in a specific way / at a specific time. I tried setting it in arm-trusted-firmware, and the pmic still set the input current limit to 3A. The datasheet for axp813 says it has the same first power on bit, but I don't have hardware to test if it behaves the same way. This driver uses the same platform data for axp803 and axp813. drivers/power/supply/axp20x_usb_power.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)