On 29 October 2016 at 01:03, Mark Brown [off-list ref] wrote:

On Fri, Oct 28, 2016 at 08:51:41PM +0800, Baolin Wang wrote:

quoted

On 28 October 2016 at 06:00, NeilBrown [off-list ref] wrote:

quoted

quoted

1/ I think we agreed that it doesn't make sense for there to be
 two chargers registered in a system.

quoted

Yes, until now...

quoted

quoted

 However usb_charger_register() still allows that, and assigns
 and arbitrary name to each based on discovery order.
 This *cannot* make sense.

quoted

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).

I think we should leave these things to avoid refactoring in future.

quoted

quoted

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.

quoted

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.

OK.

quoted

quoted

 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.

quoted

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.

quoted

quoted

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.

quoted

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.

Yes, I think so. I will ask for Jun's help.


-- 
Baolin.wang
Best Regards