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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87inu11c5l.fsf@notabene.neil.brown.name>
Date:   Mon, 12 Sep 2016 15:27:18 +0200
From:   NeilBrown <neilb@...e.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     Baolin Wang <baolin.wang@...aro.org>,
        Felipe Balbi <balbi@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Sebastian Reichel <sre@...nel.org>,
        Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
        David Woodhouse <dwmw2@...radead.org>, robh@...nel.org,
        Jun Li <jun.li@....com>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Ruslan Bilovol <ruslan.bilovol@...il.com>,
        Peter Chen <peter.chen@...escale.com>,
        Alan Stern <stern@...land.harvard.edu>, r.baldyga@...sung.com,
        grygorii.strashko@...com,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Lee Jones <lee.jones@...aro.org>,
        Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>,
        patches@...nsource.wolfsonmicro.com,
        Linux PM list <linux-pm@...r.kernel.org>,
        USB <linux-usb@...r.kernel.org>,
        device-mainlining@...ts.linuxfoundation.org,
        LKML <linux-kernel@...r.kernel.org>,
        "Bird\, Timothy" <Tim.Bird@...sony.com>
Subject: Re: [PATCH v16 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

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

Download attachment "signature.asc" of type "application/pgp-signature" (819 bytes)