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

[NeilBrown]

On Thu, Sep 08 2016, Baolin Wang wrote:

> On 8 September 2016 at 15:31, NeilBrown <neilb@xxxxxxxx> wrote:
>> On Thu, Sep 08 2016, Baolin Wang wrote:
>>>
>>> Now the usb charger will not get charger type from 'extcon' subsystem,
>>> we get the charger type from 'power_supply' and calllback
>>> 'get_charger_type' for users.
>>
>> I understand this.  I think it is wrong because, in general, the
>> power_supply doesn't know what the charger_type is (it might know it is
>> USB, but most don't know which sort of USB).  The PHY knows that, not
>> the power_supply.
>
> I don't think so. Now many platforms will detect the charger type by
> PMIC hardware, and we can get the charger type by PMIC hardware
> register. Then power supply driver can access the PMIC register to get
> the charger type. Here USB charger just considers if the accessing the
> PMIC register to get charger type is implemented in power supply, it
> is optional depending on what your platform designed.
>

In practice, the USB PHY and the Power manager will often be in the same
IC (the PMIC) so the driver for one could look at the registers for the
other.
But there is no guarantee that the hardware works like that.  It is
best to create a generally design.
Conceptually, the PHY is separate from the power manager and a solution
which recognises that will be more universal.

If the power manager can always just look at that phy registers to know
what sort of charger is connected, why does you framework need to work
with charger types at all?

>>>
>>> Yes, but you must think about some special cases on some platforms.
>>> Users may need to change the current in some situations, thus we
>>> should export one API for users to change the current. (I think you
>>> misunderstand the current limit here, that is the current for power
>>> driver  to draw).
>>
>> Can you be specific about these "special cases" please?
>> I cannot think of any.
>
> Suppose the USB configuration requests 100mA, then we should set the
> USB charger current is 100mA by __usb_charger_set_cur_limit_by_type()
> funtion, then notify this to power driver.

ahh.... I had missed something there.  It's a while since I looked
closely at these patches.

Only.... this usage of usb_charger_set_cur_limit_by_type() is really
nonsensical.

If the cable is detected as being DCP or CDP or ACA (or ACA/DOCK) then
the number negotiated with the USB configuration is not relevant and
should be ignored.  There is a guaranteed minimum which is at least the
maximum that *can* be negotiated.

It is only when the cable appears to be a SDP (standard downstream
port) that the usb-config negotiation is relevant.  That is because the
minimum guaranteed for SDP is only 100mA.

NeilBrown

Attachment: signature.asc
Description: PGP signature