Received-SPF: pass (google.com: domain of linux-kernel+bounces-34557-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 22 Jan 2024 19:52:44 -0500
From: Aren <aren@peacevolution.org>
To: Chen-Yu Tsai <wens@csie.org>
Cc: linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Aidan MacDonald <aidanmacdonald.0x0@gmail.com>, =?utf-8?Q?Ond=C5=99ej?= Jirman <megi@xff.cz>, 
	Sebastian Reichel <sre@kernel.org>, Hans de Goede <j.w.r.degoede@gmail.com>
Subject: Re: Re: [PATCH 1/3] power: supply: axp20x_usb_power: add input
 current limit
Message-ID: <2dkeq46flco2xukggii3ljlw2zo7wjwuiutha7jlxozmf63swd@xnirwnfvvvcv>
References: <20240121014057.1042466-1-aren@peacevolution.org>
 <20240121014057.1042466-3-aren@peacevolution.org>
 <CAGb2v65QcP1bcT++51n7Ada6QvFmrQzqiA-b3tbzfAr3XC6XJQ@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAGb2v65QcP1bcT++51n7Ada6QvFmrQzqiA-b3tbzfAr3XC6XJQ@mail.gmail.com>

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@peacevolution.org> 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