Added USB2 phy for Zynq platform and converted ULPI framework to platform
driver to drive CPEN to enable external 5 volt supply when Zynq USB is in
host mode.
---
On zynq platform chipidea USB controller is capable of fulfilling a wide
range of applications for USB 2.0 implementations as a host, a device, or
On-the-Go. The USB controllers are integrated into the PS IOP to bridge
between the PS interconnect and an external ULPI PHY. The register provides
indirect access to the ULPI PHY register set. The ULPI PHY register I/O
interface uses Viewport to access PHY registers.
In current approach we have extended generic ulpi phy driver and made it a
platform driver. This solves the problem, but would like to know if it is
the right approach?
The another approach would be to have access to the ULPI register via
viewport flow by creating a new platform driver at path "driver/usb/phy"
using "phy-ulpi-zynq-usb.c" source file, where the source driver would be
particular to the Xilinx/AMD zynq platform. And binding patch [1/3] would
be specific to Xilinx/AMD-specific.
We need your inputs in order to have access to the ULPI register via
viewport flow.
---
drivers/usb/phy/Kconfig | 2 +-
Piyush Mehta (3):
dt-binding: usb: ulpi-phy: add ulpi-phy binding
usb: chipidea: add usb2 phy interface for Zynq platform
usb: phy: Add platform driver support for ULPI phys
.../devicetree/bindings/usb/ulpi-phy.yaml | 48 ++++++++++
drivers/usb/chipidea/ci_hdrc_usb2.c | 8 ++
drivers/usb/phy/Kconfig | 2 +-
drivers/usb/phy/phy-ulpi.c | 90 +++++++++++++++++++
4 files changed, 147 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/usb/ulpi-phy.yaml
--
2.17.1
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 <[email protected]>
---
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 <[email protected]>
+
+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>;
+ external-drv-vbus;
+ };
--
2.17.1
USB2 phy has been added to the Zynq platform data. This defines the ULPI
phy interface for accessing the ULPI register.
Signed-off-by: Piyush Mehta <[email protected]>
---
drivers/usb/chipidea/ci_hdrc_usb2.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/usb/chipidea/ci_hdrc_usb2.c b/drivers/usb/chipidea/ci_hdrc_usb2.c
index 1321ee67f3b8..a115383fa1ee 100644
--- a/drivers/usb/chipidea/ci_hdrc_usb2.c
+++ b/drivers/usb/chipidea/ci_hdrc_usb2.c
@@ -65,6 +65,14 @@ static int ci_hdrc_usb2_probe(struct platform_device *pdev)
if (match && match->data) {
/* struct copy */
*ci_pdata = *(struct ci_hdrc_platform_data *)match->data;
+ if (of_device_is_compatible(pdev->dev.of_node,
+ "xlnx,zynq-usb-2.20a")) {
+ ci_pdata->usb_phy = devm_usb_get_phy_by_phandle(dev,
+ "usb-phy",
+ 0);
+ if (IS_ERR(ci_pdata->usb_phy))
+ return PTR_ERR(ci_pdata->usb_phy);
+ }
}
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
--
2.17.1
On 29/09/2023 08:48, 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 <[email protected]>
Subject: dt-bindings, not dt-binding.
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.
> ---
> This binding patch was created to support generic platforms. This binding
> will be modified in accordance with patch [3/3] procedures.
I don't understand this. How binding can be updated by further
procedures? Your patch 3 is a driver, so how driver can modify a binding?
Best regards,
Krzysztof
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 <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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?
There's already a defined binding for ULPI bus:
Documentation/devicetree/bindings/usb/ulpi.txt
Why can't you use/expand that?
Rob
Hi All,
> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Monday, October 2, 2023 10:30 PM
> To: Mehta, Piyush <[email protected]>
> Cc: [email protected]; Simek, Michal <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; git (AMD-Xilinx) <[email protected]>
> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > +
> > +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/bindings/phy/qcom%2Cusb-hs-phy.yaml#L100
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/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/usbmisc_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?
Regards,
Piyush Mehta
> -----Original Message-----
> From: Mehta, Piyush
> Sent: Wednesday, October 4, 2023 5:15 PM
> To: Rob Herring <[email protected]>
> Cc: [email protected]; Simek, Michal <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; git (AMD-Xilinx) <[email protected]>
> Subject: RE: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
>
> Hi All,
>
> > -----Original Message-----
> > From: Rob Herring <[email protected]>
> > Sent: Monday, October 2, 2023 10:30 PM
> > To: Mehta, Piyush <[email protected]>
> > Cc: [email protected]; Simek, Michal <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > linux- [email protected]; git (AMD-Xilinx) <[email protected]>
> > 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 <[email protected]>
> > > ---
> > > 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 <[email protected]>
> > > +
> > > +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