Re: [PATCH 0/5] Enhancements to twl4030 phy to support better charging - V2

[NeilBrown]

On Thu, 26 Mar 2015 05:29:42 +0530 Kishon Vijay Abraham I <kishon@xxxxxx>
wrote:

> Hi NeilBrown,
> 
> On Thursday 26 March 2015 02:52 AM, NeilBrown wrote:
> > On Thu, 26 Mar 2015 02:46:32 +0530 Kishon Vijay Abraham I <kishon@xxxxxx>
> > wrote:
> > 
> >> Hi,
> >>
> >> On Monday 23 March 2015 04:05 AM, NeilBrown wrote:
> >>> Hi Kishon,
> >>>  I wonder if you could queue the following for the next merge window.
> >>>  They allow the twl4030 phy to provide more information to the
> >>>  twl4030 battery charger.
> >>>  There are only minimal changes since the first version, particularly
> >>>  documentation has been improved.
> >>
> >> There are quite a few things in this series which use the USB PHY library
> >> interface which is kindof deprecated. We should try and use the Generic PHY
> >> library for all of them. It would also be better to add features to the
> >> PHY framework if the we can't achieve something with the existing PHY
> >> framework.
> > 
> > Hi,
> >  are you able to more specific at all?  What is the "USB PHY library"?
> > Where exactly is the "PHY framework"?
> 
> There is a USB PHY library that exists in drivers/usb/phy/phy.c and there is
> a Generic PHY framework that is present in drivers/phy/phy-core.c. twl4030
> actually supports both the framework.
> 
> In your patch whatever uses struct usb_phy uses the old USB PHY library and
> whatever uses struct phy uses the generic PHY framework. (Actually your patch
> does not use the PHY framework at all). We want to deprecate using the USB PHY
> library and make everyone use the generic PHY framework. Adding features
> to a driver using the USB PHY library will make the transition to generic PHY
> framework a bit more difficult.
> 
> Now all the features that is supported in the USB PHY library may not be
> supported by the PHY framework. So we should start extending the PHY framework
> instead of using the USB PHY library.
> 
> One think I noticed in your driver is using atomic notifier chain. IMO extcon
> framework should be used in twl4030 USB driver to notify the controller driver
> instead of using USB PHY notifier. For all other things we have to see if it
> can be added in the PHY framework.

Thanks a lot - exactly what I wanted.
I agree about extcon - I'll be very happy to make that work properly for twl.

I'll let you know when I have something for review.

Thanks,
NeilBrown

Attachment: pgpUoi7Yj2hHj.pgp
Description: OpenPGP digital signature