LKML Archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] usb: phy: Add platform driver support for ULPI phys
@ 2023-09-29  6:48 Piyush Mehta
  2023-09-29  6:48 ` [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding Piyush Mehta
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Piyush Mehta @ 2023-09-29  6:48 UTC (permalink / raw
  To: gregkh, michal.simek, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peter.chen, linus.walleij, paul, arnd
  Cc: piyush.mehta, linux-usb, devicetree, linux-kernel, git

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
  2023-09-29  6:48 [RFC PATCH 0/3] usb: phy: Add platform driver support for ULPI phys Piyush Mehta
@ 2023-09-29  6:48 ` Piyush Mehta
  2023-09-29 14:04   ` Conor Dooley
                     ` (2 more replies)
  2023-09-29  6:48 ` [RFC PATCH 2/3] usb: chipidea: add usb2 phy interface for Zynq platform Piyush Mehta
  2023-09-29  6:48 ` [RFC PATCH 3/3] usb: phy: Add platform driver support for ULPI phys Piyush Mehta
  2 siblings, 3 replies; 9+ messages in thread
From: Piyush Mehta @ 2023-09-29  6:48 UTC (permalink / raw
  To: gregkh, michal.simek, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peter.chen, linus.walleij, paul, arnd
  Cc: piyush.mehta, linux-usb, devicetree, linux-kernel, git

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@amd.com>
---
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@amd.com>
+
+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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 2/3] usb: chipidea: add usb2 phy interface for Zynq platform
  2023-09-29  6:48 [RFC PATCH 0/3] usb: phy: Add platform driver support for ULPI phys Piyush Mehta
  2023-09-29  6:48 ` [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding Piyush Mehta
@ 2023-09-29  6:48 ` Piyush Mehta
  2023-09-29  6:48 ` [RFC PATCH 3/3] usb: phy: Add platform driver support for ULPI phys Piyush Mehta
  2 siblings, 0 replies; 9+ messages in thread
From: Piyush Mehta @ 2023-09-29  6:48 UTC (permalink / raw
  To: gregkh, michal.simek, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peter.chen, linus.walleij, paul, arnd
  Cc: piyush.mehta, linux-usb, devicetree, linux-kernel, git

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 <piyush.mehta@amd.com>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH 3/3] usb: phy: Add platform driver support for ULPI phys
  2023-09-29  6:48 [RFC PATCH 0/3] usb: phy: Add platform driver support for ULPI phys Piyush Mehta
  2023-09-29  6:48 ` [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding Piyush Mehta
  2023-09-29  6:48 ` [RFC PATCH 2/3] usb: chipidea: add usb2 phy interface for Zynq platform Piyush Mehta
@ 2023-09-29  6:48 ` Piyush Mehta
  2 siblings, 0 replies; 9+ messages in thread
From: Piyush Mehta @ 2023-09-29  6:48 UTC (permalink / raw
  To: gregkh, michal.simek, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peter.chen, linus.walleij, paul, arnd
  Cc: piyush.mehta, linux-usb, devicetree, linux-kernel, git

Added platform driver support to ULPI Phys for Zynq. This modification
enables external five volt supply to drive 5-volts on VBUS. This signal
is or’ed with DrvVbus. Some Phys requires ULPI (OTG Control) register
DrvVbusExternal and DrvVbus bit to operate properly to drive the CPEN
pin/ external VBUS power supply.

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. Zynq platform access ULPI PHY via viewport.

Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
---
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? Here, we are modifying the phy-ulpi framework by adapting
the platform driver to fulfill our requirements. ULPI PHY register read/write
should be performed via ULPI framework using read/write API call.

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.
---
 drivers/usb/phy/Kconfig    |  2 +-
 drivers/usb/phy/Kconfig    |  2 +-
 drivers/usb/phy/phy-ulpi.c | 90 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 5f629d7cad64..38ae5458528c 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -160,7 +160,7 @@ config USB_TEGRA_PHY
 
 config USB_ULPI
 	bool "Generic ULPI Transceiver Driver"
-	depends on ARM || ARM64 || COMPILE_TEST
+	depends on ARM || ARM64 || COMPILE_TEST || USB_PHY
 	select USB_ULPI_VIEWPORT
 	help
 	  Enable this to support ULPI connected USB OTG transceivers which
diff --git a/drivers/usb/phy/phy-ulpi.c b/drivers/usb/phy/phy-ulpi.c
index e683a37e3a7a..61e15a19ea8c 100644
--- a/drivers/usb/phy/phy-ulpi.c
+++ b/drivers/usb/phy/phy-ulpi.c
@@ -13,9 +13,16 @@
 #include <linux/kernel.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
 #include <linux/usb.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/ulpi.h>
+#include <linux/usb/phy.h>
 
 
 struct ulpi_info {
@@ -39,6 +46,13 @@ static struct ulpi_info ulpi_ids[] = {
 	ULPI_INFO(ULPI_ID(0x0451, 0x1507), "TI TUSB1210"),
 };
 
+struct ulpi_phy {
+	struct usb_phy	*usb_phy;
+	void __iomem *regs;
+	unsigned int vp_offset;
+	unsigned int flags;
+};
+
 static int ulpi_set_otg_flags(struct usb_phy *phy)
 {
 	unsigned int flags = ULPI_OTG_CTRL_DP_PULLDOWN |
@@ -240,6 +254,23 @@ static int ulpi_set_vbus(struct usb_otg *otg, bool on)
 	return usb_phy_io_write(phy, flags, ULPI_OTG_CTRL);
 }
 
+static int usbphy_set_vbus(struct usb_phy *phy, int on)
+{
+	unsigned int flags = usb_phy_io_read(phy, ULPI_OTG_CTRL);
+
+	flags &= ~(ULPI_OTG_CTRL_DRVVBUS | ULPI_OTG_CTRL_DRVVBUS_EXT);
+
+	if (on) {
+		if (phy->flags & ULPI_OTG_DRVVBUS)
+			flags |= ULPI_OTG_CTRL_DRVVBUS;
+
+		if (phy->flags & ULPI_OTG_DRVVBUS_EXT)
+			flags |= ULPI_OTG_CTRL_DRVVBUS_EXT;
+	}
+
+	return usb_phy_io_write(phy, flags, ULPI_OTG_CTRL);
+}
+
 static void otg_ulpi_init(struct usb_phy *phy, struct usb_otg *otg,
 			  struct usb_phy_io_ops *ops,
 			  unsigned int flags)
@@ -249,6 +280,7 @@ static void otg_ulpi_init(struct usb_phy *phy, struct usb_otg *otg,
 	phy->io_ops	= ops;
 	phy->otg	= otg;
 	phy->init	= ulpi_init;
+	phy->set_vbus	= usbphy_set_vbus;
 
 	otg->usb_phy	= phy;
 	otg->set_host	= ulpi_set_host;
@@ -301,3 +333,61 @@ devm_otg_ulpi_create(struct device *dev,
 	return phy;
 }
 EXPORT_SYMBOL_GPL(devm_otg_ulpi_create);
+
+static int ulpi_phy_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct ulpi_phy *uphy;
+	int ret;
+
+	uphy = devm_kzalloc(&pdev->dev, sizeof(*uphy), GFP_KERNEL);
+	if (!uphy)
+		return -ENOMEM;
+
+	uphy->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(uphy->regs))
+		return PTR_ERR(uphy->regs);
+
+	if (of_property_read_bool(np, "external-drv-vbus"))
+		uphy->flags |= ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT;
+
+	ret = of_property_read_u32(np, "view-port", &uphy->vp_offset);
+	if (ret)
+		return ret;
+
+	uphy->usb_phy = otg_ulpi_create(&ulpi_viewport_access_ops, uphy->flags);
+	if (!uphy->usb_phy) {
+		dev_err(&pdev->dev, "Failed to create ULPI OTG\n");
+		return -ENOMEM;
+	}
+
+	uphy->usb_phy->dev = &pdev->dev;
+	uphy->usb_phy->io_priv = uphy->regs + uphy->vp_offset;
+	return usb_add_phy_dev(uphy->usb_phy);
+}
+
+static void ulpi_phy_remove(struct platform_device *pdev)
+{
+	struct ulpi_phy *uphy = platform_get_drvdata(pdev);
+
+	usb_remove_phy(uphy->usb_phy);
+}
+
+static const struct of_device_id ulpi_phy_table[] = {
+	{ .compatible = "ulpi-phy" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, ulpi_phy_table);
+
+static struct platform_driver ulpi_phy_driver = {
+	.probe		= ulpi_phy_probe,
+	.remove_new	= ulpi_phy_remove,
+	.driver		= {
+		.name	= "ulpi-phy",
+		.of_match_table = ulpi_phy_table,
+	},
+};
+module_platform_driver(ulpi_phy_driver);
+
+MODULE_DESCRIPTION("ULPI PHY driver");
+MODULE_LICENSE("GPL");
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
  2023-09-29  6:48 ` [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding Piyush Mehta
@ 2023-09-29 14:04   ` Conor Dooley
  2023-09-30 15:15   ` Krzysztof Kozlowski
  2023-10-02 17:00   ` Rob Herring
  2 siblings, 0 replies; 9+ messages in thread
From: Conor Dooley @ 2023-09-29 14:04 UTC (permalink / raw
  To: Piyush Mehta
  Cc: gregkh, michal.simek, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	peter.chen, linus.walleij, paul, arnd, linux-usb, devicetree,
	linux-kernel, git

[-- Attachment #1: Type: text/plain, Size: 2893 bytes --]

Hey,

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@amd.com>
> ---
> 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.

I don't understand what the drivers have to do with describing the
hardware here. You have a zynq platform with this phy, so the compatible
should reflect that. Whether you create some software that supports
generic platforms is mostly a separate question IMO.

> 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@amd.com>
> +
> +properties:
> +  compatible:
> +    const: ulpi-phy

Please add a device specific compatible here.

> +
> +  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.

Why would this not just be a second entry in reg?

Cheers,
Conor.

> +
> +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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
  2023-09-29  6:48 ` [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding Piyush Mehta
  2023-09-29 14:04   ` Conor Dooley
@ 2023-09-30 15:15   ` Krzysztof Kozlowski
  2023-10-02 17:00   ` Rob Herring
  2 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2023-09-30 15:15 UTC (permalink / raw
  To: Piyush Mehta, gregkh, michal.simek, robh+dt,
	krzysztof.kozlowski+dt, conor+dt, peter.chen, linus.walleij, paul,
	arnd
  Cc: linux-usb, devicetree, linux-kernel, git

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 <piyush.mehta@amd.com>

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
  2023-09-29  6:48 ` [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding Piyush Mehta
  2023-09-29 14:04   ` Conor Dooley
  2023-09-30 15:15   ` Krzysztof Kozlowski
@ 2023-10-02 17:00   ` Rob Herring
  2023-10-04 11:45     ` Mehta, Piyush
  2 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2023-10-02 17:00 UTC (permalink / raw
  To: Piyush Mehta
  Cc: gregkh, michal.simek, krzysztof.kozlowski+dt, conor+dt,
	peter.chen, linus.walleij, paul, arnd, linux-usb, devicetree,
	linux-kernel, git

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@amd.com>
> ---
> 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@amd.com>
> +
> +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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
  2023-10-02 17:00   ` Rob Herring
@ 2023-10-04 11:45     ` Mehta, Piyush
  2023-12-01 13:07       ` Mehta, Piyush
  0 siblings, 1 reply; 9+ messages in thread
From: Mehta, Piyush @ 2023-10-04 11:45 UTC (permalink / raw
  To: Rob Herring
  Cc: gregkh@linuxfoundation.org, Simek, Michal,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	peter.chen@kernel.org, linus.walleij@linaro.org,
	paul@crapouillou.net, arnd@arndb.de, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	git (AMD-Xilinx)

Hi All,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Monday, October 2, 2023 10:30 PM
> To: Mehta, Piyush <piyush.mehta@amd.com>
> Cc: gregkh@linuxfoundation.org; Simek, Michal <michal.simek@amd.com>;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> peter.chen@kernel.org; linus.walleij@linaro.org; paul@crapouillou.net;
> arnd@arndb.de; linux-usb@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>
> 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@amd.com>
> > ---
> > 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@amd.com>
> > +
> > +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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
  2023-10-04 11:45     ` Mehta, Piyush
@ 2023-12-01 13:07       ` Mehta, Piyush
  0 siblings, 0 replies; 9+ messages in thread
From: Mehta, Piyush @ 2023-12-01 13:07 UTC (permalink / raw
  To: Rob Herring
  Cc: gregkh@linuxfoundation.org, Simek, Michal,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	peter.chen@kernel.org, linus.walleij@linaro.org,
	paul@crapouillou.net, arnd@arndb.de, linux-usb@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	git (AMD-Xilinx)


> -----Original Message-----
> From: Mehta, Piyush
> Sent: Wednesday, October 4, 2023 5:15 PM
> To: Rob Herring <robh@kernel.org>
> Cc: gregkh@linuxfoundation.org; Simek, Michal <michal.simek@amd.com>;
> krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> peter.chen@kernel.org; linus.walleij@linaro.org; paul@crapouillou.net;
> arnd@arndb.de; linux-usb@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>
> Subject: RE: [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding
> 
> Hi All,
> 
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Monday, October 2, 2023 10:30 PM
> > To: Mehta, Piyush <piyush.mehta@amd.com>
> > Cc: gregkh@linuxfoundation.org; Simek, Michal <michal.simek@amd.com>;
> > krzysztof.kozlowski+dt@linaro.org; conor+dt@kernel.org;
> > peter.chen@kernel.org; linus.walleij@linaro.org; paul@crapouillou.net;
> > arnd@arndb.de; linux-usb@vger.kernel.org; devicetree@vger.kernel.org;
> > linux- kernel@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>
> > 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@amd.com>
> > > ---
> > > 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@amd.com>
> > > +
> > > +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



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-12-01 13:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-29  6:48 [RFC PATCH 0/3] usb: phy: Add platform driver support for ULPI phys Piyush Mehta
2023-09-29  6:48 ` [RFC PATCH 1/3] dt-binding: usb: ulpi-phy: add ulpi-phy binding Piyush Mehta
2023-09-29 14:04   ` Conor Dooley
2023-09-30 15:15   ` Krzysztof Kozlowski
2023-10-02 17:00   ` Rob Herring
2023-10-04 11:45     ` Mehta, Piyush
2023-12-01 13:07       ` Mehta, Piyush
2023-09-29  6:48 ` [RFC PATCH 2/3] usb: chipidea: add usb2 phy interface for Zynq platform Piyush Mehta
2023-09-29  6:48 ` [RFC PATCH 3/3] usb: phy: Add platform driver support for ULPI phys Piyush Mehta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).