RE: [RFC PATCH 1/2] usb: doc: Add bindings for ULPI platform driver

[Subbaraya Sundeep Bhatta]

Hi Kishon,

> -----Original Message-----
> From: Kishon Vijay Abraham I [mailto:kishon@xxxxxx]
> Sent: Sunday, October 11, 2015 8:11 PM
> To: Punnaiah Choudary Kalluri; balbi@xxxxxx
> Cc: Rob Herring; Subbaraya Sundeep Bhatta; Peter Chen;
> devicetree@xxxxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; linux-
> usb@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [RFC PATCH 1/2] usb: doc: Add bindings for ULPI platform driver
> 
> Hi,
> 
> On Sunday 11 October 2015 04:45 PM, punnaiah choudary kalluri wrote:
> > On Wed, Sep 30, 2015 at 9:48 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> >> On Thu, Sep 24, 2015 at 11:18:01AM -0500, Rob Herring wrote:
> >>> On Thu, Sep 24, 2015 at 4:26 AM, Subbaraya Sundeep Bhatta
> >>> <subbaraya.sundeep.bhatta@xxxxxxxxxx> wrote:
> >>>> Hi Peter,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Peter Chen [mailto:peter.chen@xxxxxxxxxxxxx]
> >>>>> Sent: Thursday, September 24, 2015 2:41 PM
> >>>>> To: Subbaraya Sundeep Bhatta
> >>>>> Cc: balbi@xxxxxx; devicetree@xxxxxxxxxxxxxxx; kishon@xxxxxx;
> >>>>> gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; linux-
> >>>>> kernel@xxxxxxxxxxxxxxx; Punnaiah Choudary Kalluri; Subbaraya
> >>>>> Sundeep Bhatta; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> >>>>> Subject: Re: [RFC PATCH 1/2] usb: doc: Add bindings for ULPI
> >>>>> platform driver
> >>>>>
> >>>>> On Wed, Sep 23, 2015 at 06:24:01PM +0530, Subbaraya Sundeep
> Bhatta
> >>>>> wrote:
> >>>>>> This patch adds binding doc info for generic ULPI PHYs platform
> >>>>>> driver.
> >>>>>>
> >>>>>> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta@xxxxxxxxxx>
> >>>>>> ---
> >>>>>>  .../devicetree/bindings/usb/ulpi-platform-phy.txt  |   34
> >>>>> ++++++++++++++++++++
> >>>>>>  1 files changed, 34 insertions(+), 0 deletions(-)  create mode
> >>>>>> 100644
> >>>>>> Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> >>>>>>
> >>>>>> diff --git
> >>>>>> a/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> >>>>>> b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> >>>>>> new file mode 100644
> >>>>>> index 0000000..7b8cbb4
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/usb/ulpi-platform-phy.txt
> >>>>>> @@ -0,0 +1,34 @@
> >>>>>> +Platform driver for generic ULPI PHYs
> >>>>>> +
> >>>>>> +Required properties:
> >>>>>> +- compatible               : Should be "ulpi-phy"
> >>>>>> +- reg                      : Physical base address and size of the USB
> >>>>>> +                     controller registers map to which this PHY
> >>>>>> +                     is connected.
> >>>>>> +- view-port                : Should contain viewport register offset of the
> >>>>>> +                     USB controller to which this PHY is
> >>>>>> +connected Optional
> >>>>>> +properties:
> >>>>>> +- drv-vbus         : required if turning VBUS on/off has to be driven
> >>>>>> +                     by writing to PHY. This feature depends on board
> >>>>>> +                     design.
> >>>>>> +
> >>>>>> +Example:
> >>>>>> +Below example shows the PHY binding for Chipidea USB controller
> >>>>>> +which has ulpi viewport register at 0x0170
> >>>>>> +
> >>>>>> +           usb_phy0: phy0 {
> >>>>>> +                   compatible = "ulpi-phy";
> >>>>>> +                   reg = <0xe0002000 0x1000>;
> >>>>>> +                   view-port = <0x0170>;
> >>>>>> +                   drv-vbus;
> >>>>>> +           };
> >>>>>> +
> >>>>>> +           usb0: usb@e0002000 {
> >>>>>> +                        compatible = "chipidea,usb2";
> >>>>>> +                        interrupt-parent = <&intc>;
> >>>>>> +                        interrupts = <0 21 4>;
> >>>>>> +                        reg = <0xe0002000 0x1000>;
> >>>>>
> >>>>> Although just call devm_ioremap twice for the same register region
> >>>>> does not cause any errors, I am not sure if it will has other
> >>>>> potential problems. Cc: arm list.
> >>>>
> >>>> Yes Peter I was also in doubt to call devm_ioremap twice for same register
> region.
> >>>> devm_ioremap_resource complained hence modified to devm_ioremap.
> >>>> Thanks for adding arm-list.
> >>>
> >>> Don't put overlapping resources in the DT. Having 2 drivers
> >>> accessing the same registers is not a clean or safe design.
> >>
> >> thanks, saves me the trouble of saying the same thing.
> >>
> >> Bottom line, if devm_ioremap_resource() fails, you're wrong. Just fix
> >> your driver and move on.
> >
> > Any suggestions on how to move further?
> > Chipidea controller provides ulpi view port register for accessing the
> > usb phy registers. Now we want to add new driver for ulpi phy
> > configuration and that obviously it need of ulpi view port register
> > access. So, sharing the register space between these two drivers is necessary
> here.
> 
> Why not program ULPI the same way as DWC3 does?

Sorry for delay in response, back from long vacation.
Can we call generic PHY phy->power_on and power_off functions to do similar function as usb2_phy->set_vbus?
I assume it is to power on whole PHY chip but not to enable 5V on VBUS line.


Thanks,
Sundeep
> 
> Thanks
> Kishon