Re: [PATCH v16 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[NeilBrown <neilb-IBi9RG/b67k@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2275 bytes --]

On Mon, Sep 12 2016, Mark Brown wrote:

> [ Unknown signature status ]
> On Sat, Sep 10, 2016 at 07:57:26AM +1000, NeilBrown wrote:
>> On Fri, Sep 09 2016, Mark Brown wrote:
>
>> > The wm831x driver in the patch series is an example of such hardware -
>> > it is purely a power manager, it has no USB PHY hardware at all.  It's a
>
>> The "probe" routine calls
>
>>  +			usb_charger_find_by_name(wm831x_pdata->usb_gadget);
>
>> Presumably wm831x_pdata is initialised by a board file?
>
> Yes.
>
>> I strongly suspect it is initialized to  "usb-charger.0" because the
>> names given to usb chargers are "usb-charger.%d" in discovery order.
>> I don't see this being at all useful if there is ever more than one
>> usb-charger.
>> Do you?
>
> It's no worse than any other board file situation - if someone has that
> problem they get to fix it.

My point is that the present design does not appear to scale beyond a
single USB power supply (as if there were two, they would be named in
discovery order, which is not reliably stable).

Your point is, I think, that when someone actually cares about that lack
of scaling, they can fix it.

I am perfectly happy with that approach.  However if the code doesn't
scale beyond one charger, it shouldn't pretend that it does.
i.e.
  Don't have "usb_charger_find_by_name()", just a global "usb_charger"
  (or similar).
  The first charger to register gets to be the "usb_charger".  The
  second one gets an error.
I could be quite happy with that sort of interface.

>
>> So how can this wm831x driver actually find out what sort of charger is
>> connected and so set the power limit?  I just don't see this working *at*
>> *all*.
>
> The whole point from the point of view of the wm831x driver is that it
> just wants something to tell it how much current it's allowed to draw, I
> appreciate that doesn't change your analysis of the bit in the middle
> but the consumer driver bit seems fine here.

Yes, the wm831x driver probably does the right thing.
Other drivers might want to know not only the minimum they are allowed
to draw, but also the maximum they should try even if they are carefully
monitoring the voltage.
So wm831x is doing the right thing with the wrong interface.  Maybe you
can describe that as "fine".

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]