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

From: Subbaraya Sundeep Bhatta
Date: Wed Oct 28 2015 - 07:55:26 EST


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