Messages in this thread | | | Date | Fri, 28 Oct 2016 18:03:02 +0100 | From | Mark Brown <> | Subject | Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation |
| |
On Fri, Oct 28, 2016 at 08:51:41PM +0800, Baolin Wang wrote: > On 28 October 2016 at 06:00, NeilBrown <neilb@suse.com> wrote:
> > 1/ I think we agreed that it doesn't make sense for there to be > > two chargers registered in a system.
> Yes, until now...
> > However usb_charger_register() still allows that, and assigns > > and arbitrary name to each based on discovery order. > > This *cannot* make sense.
> Fine, I can change that to allow only one charger to register.
Yeah, it's a reasonable change. I'm not sure the prior discussion was 100% conclusive on the issue (I remember there being some debate about leaving things there to avoid any need for future refactoring to touch the interface).
> > 2/ Why do you have usb_charger_set_current()?? > > No code ever calls it. > > This updates the min and max current which are defined in a > > standard. It never makes sense to change the min and max > > for a particular cable type.
> Mark, do we have some scenarios which want to change the current > limitation? If not, okay, I agree with you to remove this function.
I'm not aware of any, we can always add it back if the need arises.
> > Related: I don't like charger_type_show(). I don't think > > the usb-charger should export that information to user-space because > > extcon already does that, and duplication is confusing and pointless.
> I think we should combine all charger related information into one > place for user. Moreover if we don't get charger type from extcon, we > should also need one place to export the charger type.
I had also thought there was some software negotation as well as the physical charger in cases where the device is plugged into an active host? I could be wrong.
> > 5/ There is no convincing example usage of this framework. > > wm8931x_power.c just scratches the surface. > > If it is so good, it should be easy to convert a lot of other > > drivers over to it. If you did that it would be much easier > > to see how it works and what the strengths/weaknesses were.
> Jun have send out one patchset[1] based on my patchset, and he tested > mypatchset. Thanks for your comments. > [1]http://www.spinics.net/lists/linux-usb/msg139809.html
I think it's a good idea to pick up Jun's patches into your patch set, that way Jun doesn't need to rebase and it might help with review of your patches too. [unhandled content-type:application/pgp-signature] | |