RE: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding

From: Mehta, Piyush
Date: Fri Dec 01 2023 - 08:07:51 EST


> -----Original Message-----
> From: Mehta, Piyush
> Sent: Wednesday, October 4, 2023 5:15 PM
> To: Rob Herring <robh@xxxxxxxxxx>
> Cc: gregkh@xxxxxxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>;
> krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> peter.chen@xxxxxxxxxx; linus.walleij@xxxxxxxxxx; paul@xxxxxxxxxxxxxxx;
> arnd@xxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>
> Subject: RE: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
>
> Hi All,
>
> > -----Original Message-----
> > From: Rob Herring <robh@xxxxxxxxxx>
> > Sent: Monday, October 2, 2023 10:30 PM
> > To: Mehta, Piyush <piyush.mehta@xxxxxxx>
> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; Simek, Michal <michal.simek@xxxxxxx>;
> > krzysztof.kozlowski+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx;
> > peter.chen@xxxxxxxxxx; linus.walleij@xxxxxxxxxx; paul@xxxxxxxxxxxxxxx;
> > arnd@xxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> > linux- kernel@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>
> > Subject: Re: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy
> > binding
> >
> > On Fri, Sep 29, 2023 at 12:18:50PM +0530, Piyush Mehta wrote:
> > > Create an ulpi-phy binding to read and write PHY registers with
> > > explicit control of the address and data using the usb.VIEWPORT register.
> > >
> > > Signed-off-by: Piyush Mehta <piyush.mehta@xxxxxxx>
> > > ---
> > > This binding patch was created to support generic platforms. This
> > > binding will be modified in accordance with patch [3/3] procedures.
> > > One of the approch may be Create a zynq phy platform driver in
> > > "driver/usb/phy" with driver source "phy-ulpi-zynq-usb.c" and then
> > > the binding will be particular to the Xilinx/AMD zynq platform.
> > >
> > > This binding was built with the Zynq hardware design example in
> > > consideration of as a generic platform. The viewport provide access
> > > the Chipidea controller to interface with the ULPI PHY.
> > > ---
> > > .../devicetree/bindings/usb/ulpi-phy.yaml | 48 +++++++++++++++++++
> > > 1 file changed, 48 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/usb/ulpi-phy.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/ulpi-phy.yaml
> > > b/Documentation/devicetree/bindings/usb/ulpi-phy.yaml
> > > new file mode 100644
> > > index 000000000000..490b2f610129
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/usb/ulpi-phy.yaml
> > > @@ -0,0 +1,48 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/usb/ulpi-phy.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: ULPI PHY- Generic platform
> > > +
> > > +maintainers:
> > > + - Piyush Mehta <piyush.mehta@xxxxxxx>
> > > +
> > > +properties:
> > > + compatible:
> > > + const: ulpi-phy
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + '#phy-cells':
> > > + const: 0
> > > +
> > > + external-drv-vbus:
> > > + description:
> > > + If present, configure ulpi-phy external supply to drive 5V on VBus.
> > > + type: boolean
> > > +
> > > + view-port:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + description:
> > > + Address to read and write PHY registers with explicit control of
> > > + the address and data using the usb.VIEWPORT register.
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - view-port
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > + phy0@e0002000 {
> > > + compatible = "ulpi-phy";
> > > + #phy-cells = <0x00>;
> > > + reg = <0xe0002000 0x1000>;
> > > + view-port = <0x170>;
> >
> > I don't understand. Do you have an MMIO address and the VIEWPORT
> > address to the PHY? You need both?
>
> Yes, we need both.
>
> The ULPI Link wrapper passes-through packet data and interprets Rx
> commands as well as send Tx commands and provide viewport services to the
> software. The ULPI link wrapper interfaces between the port controller (a bus
> similar to UTMI+ that connects to the rest of the controller and its registers)
> and the ULPI interface.
>
> Name XUSBPS_ULPIVIEW_OFFSET
> Address 0x00000170
>
> Description ULPI Viewport
>
> The register provides indirect access to the ULPI PHY register set. Although the
> core performs access to the ULPI PHY register set, there may be extraordinary
> circumstances where software may need direct access.
>
> ULPI PHY Viewport
> The ULPI viewport provides a mechanism for software to read and write PHY
> registers with explicit control of the address and data using the usb.VIEWPORT
> register. An interrupt is generated when a transaction is complete, including
> when the requested read data is available.
>
> >
> > There's already a defined binding for ULPI bus:
> >
> > Documentation/devicetree/bindings/usb/ulpi.txt
> >
> > Why can't you use/expand that?
> >
> > Rob
>
> We need your input to determine the best approach . We did preliminary
> research and discovered few possibilities:
>
> USB Node {
> .............
> ULPI PHY Node { // child node
> .................
> compatible = "ulpi-phy-";
> #phy-cells = <0x00>;
> ..............
> };
> };
>
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/b
> indings/phy/qcom%2Cusb-hs-phy.yaml#L100
> https://github.com/torvalds/linux/blob/master/Documentation/devicetree/b
> indings/usb/ci-hdrc-usb2.yaml#L338
> https://github.com/torvalds/linux/blob/master/drivers/usb/chipidea/ci_hdrc
> _msm.c#L245
> https://github.com/torvalds/linux/blob/master/drivers/phy/qualcomm/phy-
> qcom-usb-hs.c#L85
> [This implementation is based on ulpi driver:
> https://github.com/torvalds/linux/blob/master/drivers/phy/qualcomm/phy-
> qcom-usb-hs.c#L287C1-L287C19]
>
> OR
>
> https://github.com/torvalds/linux/blob/master/drivers/usb/chipidea/ci_hdrc
> _imx.c#L81
> https://github.com/torvalds/linux/blob/master/drivers/usb/chipidea/ci_hdrc
> _imx.c#L176
> https://github.com/torvalds/linux/blob/master/drivers/usb/chipidea/usbmis
> c_imx.c#L234
> https://github.com/torvalds/linux/blob/master/drivers/usb/phy/phy-mxs-
> usb.c#L191
> [This implementation is based on platform driver:
> https://github.com/torvalds/linux/blob/master/drivers/usb/phy/phy-mxs-
> usb.c#L848]
>
> Note:
> Above examples are to show the interface between the Chipidea Controller
> and PHY.
>
> Are these possibilities aligning with your inputs?
>
Please share your inputs on the above alternate design approaches.

> Regards,
> Piyush Mehta

BR,
PM