Re: Re: [PATCH 1/3] power: supply: axp20x_usb_power: add input current limit

[Aren]

On Sun, Jan 21, 2024 at 11:25:04PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> +CC Hans de Goede since the AXP288 is similar to the later AXPs in the USB
> power section.
> 
> On Sun, Jan 21, 2024 at 9:54 AM Aren Moynihan <aren@xxxxxxxxxxxxxxxxx> wrote:
> >
> > Add properties for setting the maximum current that will be drawn from
> > the usb connection.
> >
> > These changes don't apply to all axp20x chips, so we need to add new
> > power_desc and power supply property objects for axp813 specifically.
> > These are copied from the axp22x variants that were used before, with
> > extra fields added.
> >
> > Also add a dev field to the axp20x_usb_power struct, so we can use
> > dev_dbg and dev_err in more places.
> 
> I think this patch highlights some errors in the driver.
> 
> 1. We are likely misusing POWER_SUPPLY_PROP_CURRENT_MAX. Based on the
>    ABI docs, this is supposed to be a read only property. The correct
>    way would be to use POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT as you did.

I think so, yes. I was hesitant to touch that because I wasn't sure if
current_max was relied on anywhere. I'll do some grepping to confirm
that it'll be safe.

> 2. For AXP288 and AXP803/AXP813/AXP818, we are setting the current limit
>    for when BC detection finishes in register 0x30, not the actual active
>    limit in register 0x35. This should be fixed.
> 
> 3. BC detection race condition
> 
> Could you maybe respin the patches to address them separately instead?

Sure, that sounds like a better way to split it up.

 - Aren

> Thanks
> ChenYu