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

[Peter Chen]

On Tue, Apr 05, 2016 at 05:34:02PM +0800, Baolin Wang wrote:
> On 5 April 2016 at 16:12, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
> > On Tue, Apr 05, 2016 at 03:54:31PM +0800, Baolin Wang wrote:
> >> Hi Peter,
> >>
> >> On 5 April 2016 at 14:46, Peter Chen <hzpeterchen@xxxxxxxxx> wrote:
> >> >
> >> > We are thinking USB charger framework for Freescale i.mx SoC series,
> >> > since our internal framework is not good enough.
> >> > So I have more questions for your framework since there are many
> >> > different USB charger designs, and I hope it is universal.
> >>
> >> Great, thanks for your attention and suggestions.
> >>
> >> >
> >> > - I would like to see your all code to let the charger work, eg
> >> > you have said the charger detection is done by PMIC automatically,
> >> > but I did not find your PMIC code to read charger type.
> >>
> >> Yeah, this patchset did not give an example to read charger type from
> >> PMIC registers. (Cause now the user 'wm831x_power' don't need this.)
> >> But I think user can get it easily by implementing below callbacks.
> >> (1) gadget->ops->get_charger_type();
> >> (2) power_supply_get_property(uchger->psy, POWER_SUPPLY_PROP_CHARGE_TYPE, &val);
> >> (3) uchger->get_charger_type();
> >>
> >
> > I just would like if you can have this, then, we (you) can test it, eg,
> > you can test if the wm831x can charge more than 1500mA for DCP.
> 
> Mark, could you please address Peter's comments about if the the
> wm831x can charge more than 1500mA for DCP? (I have no environment to
> test wm831x) Thanks.
> 

I don't want you or Mark to test at hardware, I just would like to see
some code that how PMIC, wm831x, and USB gadget driver work together.

> >> >
> >> > Besides, how you can make sure the charger detection has finished
> >> > before the framework handles USB_CHARGER_PRESENT event?
> >>
> >> I think we don't need to care about this situation. If the charger
> >> type is 'UNKNOWN_TYPE' (maybe charger detection is not finished yet),
> >> the charger framework will not set current (current is 0) for power
> >> driver.
> >
> > Then, when we notify the charger IC for larger current, eg, for DCP.
> 
> I suppose If the usb charger framework gets the charger type, then
> notify the charger IC for larger current.

It is no problem for software detection, but for hardware one, it must
make sure it sends "USB_CHARGER_PRESENT" event after detection has
finished, otherwise no one will notify charger framework for DCP.

> 
> >
> >>
> >> >
> >> > - I commented the current limit at different situations for USB
> >> > charger last time, but I have not seen your further comments.
> >> > I would like give it again. For DCP, you can notify charger IC
> >> > once you get the charger type. But for CDP/SDP, you need to
> >> > notify charger IC after set configuration has finished, since
> >> > the host may still not be ready to give high current.
> >>
> >> As my understanding, if the usb charger framework get the charger
> >> type, it means we can notify the power driver to set the current. If
> >> you don't ready for setting current, please don't give the charger
> >> type to usb charger framework.
> >>
> >> The framework does not want to focus on charger detection too much,
> >> and just supplies one callback '->charger_detect()' for user to be
> >> implemented if they ensure they need to do the SW charger detection
> >> manually (Note: must at the right time to do the SW detection.). So
> >> the usb charger just focus on dealing with the usb gadget power
> >> negotiation, and it does not need to care much how to do charger
> >> detection on your platform.
> >
> > No, this comment is common one, but only for SW detection. Eg, when
> > the PMIC tells you it is a SDP, you can't notify to charger IC about
> > 500mA at once, you need to do it after host allows you to do it.
> 
> Well. Sounds reasonable. I just give an ugly example to implement the
> 'gadget->ops->get_charger_type()' method to get the charger type.
> 
> enum usb_charger_type get_charger_type(struct usb_gadget *gadget)
> {
>         if (host is allowed)
>                 read charger type from PMIC;
>         else
>                 return UNKNOWN_TYPE;
> }
> 
> So that will makes usb charger do not need to care about too much to
> make things more complicated. Or do you have any other good
> suggestions? Thanks.
> 

Since it is a USB charger, you had to be involved with USB stuffs:).

-- 

Best Regards,
Peter Chen