[PATCH 0/5] usb: dwc3-uniphier: Replace the driver to use dwc3-generic

[PATCH 0/5] usb: dwc3-uniphier: Replace the driver to use dwc3-generic

[Kunihiko Hayashi]

This series exports the structures and functions from the driver source
to the header, and replaces dwc3-uniphier driver to use them.

This expects dwc3-generic to prevent more SoC-dependent codes.

Kunihiko Hayashi (5):
  usb: dwc3-generic: Export glue structures and functions
  usb: dwc3-generic: Add the size of regs property to glue structure
  usb: dwc3-generic: Add dependency on SIMPLE_BUS
  usb: dwc3-uniphier: Use dwc3-generic instead of xhci-dwc3
  uniphier_defconfig: Disable USB_XHCI_DWC3

 configs/uniphier_v7_defconfig    |  1 -
 configs/uniphier_v8_defconfig    |  1 -
 drivers/usb/dwc3/Kconfig         |  7 +--
 drivers/usb/dwc3/dwc3-generic.c  | 19 ++------
 drivers/usb/dwc3/dwc3-generic.h  | 32 +++++++++++++
 drivers/usb/dwc3/dwc3-uniphier.c | 79 ++++++++++++++++----------------
 6 files changed, 81 insertions(+), 58 deletions(-)
 create mode 100644 drivers/usb/dwc3/dwc3-generic.h

-- 
2.25.1

[PATCH 1/5] usb: dwc3-generic: Export glue structures and functions

[Kunihiko Hayashi]

In order to allow external SoC-dependent glue drivers to use dwc3-generic
functions, push the glue structures and export the functions to a header
file.

The exported structures and functions are:

- struct dwc3_glue_data
- struct dwc3_glue_ops
- dwc3_glue_bind()
- dwc3_glue_probe()
- dwc3_glue_remove()

The SoC-dependent glue drivers can only define their own wrapper driver
and specify these functions. The drivers can also add their own compatible
strings and configure functions.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/usb/dwc3/dwc3-generic.c | 17 ++++-------------
 drivers/usb/dwc3/dwc3-generic.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 13 deletions(-)
 create mode 100644 drivers/usb/dwc3/dwc3-generic.h

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 78966718d0..1708ea14bb 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -28,11 +28,7 @@
 #include <usb/xhci.h>
 #include <asm/gpio.h>
 
-struct dwc3_glue_data {
-	struct clk_bulk		clks;
-	struct reset_ctl_bulk	resets;
-	fdt_addr_t regs;
-};
+#include "dwc3-generic.h"
 
 struct dwc3_generic_plat {
 	fdt_addr_t base;
@@ -258,11 +254,6 @@ U_BOOT_DRIVER(dwc3_generic_host) = {
 };
 #endif
 
-struct dwc3_glue_ops {
-	void (*glue_configure)(struct udevice *dev, int index,
-			       enum usb_dr_mode mode);
-};
-
 void dwc3_imx8mp_glue_configure(struct udevice *dev, int index,
 				enum usb_dr_mode mode)
 {
@@ -398,7 +389,7 @@ struct dwc3_glue_ops ti_ops = {
 	.glue_configure = dwc3_ti_glue_configure,
 };
 
-static int dwc3_glue_bind(struct udevice *parent)
+int dwc3_glue_bind(struct udevice *parent)
 {
 	ofnode node;
 	int ret;
@@ -493,7 +484,7 @@ static int dwc3_glue_clk_init(struct udevice *dev,
 	return 0;
 }
 
-static int dwc3_glue_probe(struct udevice *dev)
+int dwc3_glue_probe(struct udevice *dev)
 {
 	struct dwc3_glue_ops *ops = (struct dwc3_glue_ops *)dev_get_driver_data(dev);
 	struct dwc3_glue_data *glue = dev_get_plat(dev);
@@ -553,7 +544,7 @@ static int dwc3_glue_probe(struct udevice *dev)
 	return 0;
 }
 
-static int dwc3_glue_remove(struct udevice *dev)
+int dwc3_glue_remove(struct udevice *dev)
 {
 	struct dwc3_glue_data *glue = dev_get_plat(dev);
 
diff --git a/drivers/usb/dwc3/dwc3-generic.h b/drivers/usb/dwc3/dwc3-generic.h
new file mode 100644
index 0000000000..c7925ce4ae
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-generic.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/**
+ * dwc3-generic.h - Generic DWC3 Glue layer header
+ *
+ * Copyright (C) 2016 - 2018 Xilinx, Inc.
+ * Copyright (C) 2023 Socionext Inc.
+ */
+
+#ifndef __DRIVERS_USB_DWC3_GENERIC_H
+#define __DRIVERS_USB_DWC3_GENERIC_H
+
+#include <clk.h>
+#include <reset.h>
+#include <dwc3-uboot.h>
+
+struct dwc3_glue_data {
+	struct clk_bulk		clks;
+	struct reset_ctl_bulk	resets;
+	fdt_addr_t regs;
+};
+
+struct dwc3_glue_ops {
+	void (*glue_configure)(struct udevice *dev, int index,
+			       enum usb_dr_mode mode);
+};
+
+int dwc3_glue_bind(struct udevice *parent);
+int dwc3_glue_probe(struct udevice *dev);
+int dwc3_glue_remove(struct udevice *dev);
+
+#endif
-- 
2.25.1

[PATCH 2/5] usb: dwc3-generic: Add the size of regs property to glue structure

[Kunihiko Hayashi]

Add the size of regs property to the glue structure to correctly
specify the register region to map.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/usb/dwc3/dwc3-generic.c | 2 +-
 drivers/usb/dwc3/dwc3-generic.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 1708ea14bb..7ad26c454d 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -505,7 +505,7 @@ int dwc3_glue_probe(struct udevice *dev)
 		phy.dev = NULL;
 	}
 
-	glue->regs = dev_read_addr(dev);
+	glue->regs = dev_read_addr_size_index(dev, 0, &glue->size);
 
 	ret = dwc3_glue_clk_init(dev, glue);
 	if (ret)
diff --git a/drivers/usb/dwc3/dwc3-generic.h b/drivers/usb/dwc3/dwc3-generic.h
index c7925ce4ae..f1823a01f4 100644
--- a/drivers/usb/dwc3/dwc3-generic.h
+++ b/drivers/usb/dwc3/dwc3-generic.h
@@ -17,6 +17,7 @@ struct dwc3_glue_data {
 	struct clk_bulk		clks;
 	struct reset_ctl_bulk	resets;
 	fdt_addr_t regs;
+	fdt_size_t size;
 };
 
 struct dwc3_glue_ops {
-- 
2.25.1

[PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

The glue driver doesn't do or offer actively anything, SIMPLE_BUS is
more preferable to represent the driver.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/usb/dwc3/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index f010291d02..dadaa083e7 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -25,14 +25,14 @@ config USB_DWC3_OMAP
 
 config USB_DWC3_GENERIC
 	bool "Generic implementation of a DWC3 wrapper (aka dwc3 glue)"
-	depends on DM_USB && USB_DWC3 && MISC
+	depends on DM_USB && USB_DWC3 && (MISC || SIMPLE_BUS)
 	help
 	  Select this for Xilinx ZynqMP and similar Platforms.
 	  This wrapper supports Host and Peripheral operation modes.
 
 config SPL_USB_DWC3_GENERIC
 	bool "Generic implementation of a DWC3 wrapper (aka dwc3 glue) for the SPL"
-	depends on SPL_DM_USB && USB_DWC3 && SPL_MISC
+	depends on SPL_DM_USB && USB_DWC3 && (SPL_MISC || SPL_SIMPLE_BUS)
 	help
 	  Select this for Xilinx ZynqMP and similar Platforms.
 	  This wrapper supports Host and Peripheral operation modes.
-- 
2.25.1

[PATCH 4/5] usb: dwc3-uniphier: Use dwc3-generic instead of xhci-dwc3

[Kunihiko Hayashi]

dwc3-uniphier depends on xhci-dwc3 framework, however, it is preferable
to use dwc3-generic.

This driver calls the exported dwc3-generic functions and redefine
the SoC-dependent operations to fit dwc3-generic.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 drivers/usb/dwc3/Kconfig         |  3 +-
 drivers/usb/dwc3/dwc3-uniphier.c | 79 ++++++++++++++++----------------
 2 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index dadaa083e7..dbd14b1e90 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -55,7 +55,8 @@ config USB_DWC3_MESON_GXL
 
 config USB_DWC3_UNIPHIER
 	bool "DesignWare USB3 Host Support on UniPhier Platforms"
-	depends on ARCH_UNIPHIER && USB_XHCI_DWC3
+	depends on ARCH_UNIPHIER && USB_DWC3
+	select USB_DWC3_GENERIC
 	help
 	  Support of USB2/3 functionality in Socionext UniPhier platforms.
 	  Say 'Y' here if you have one such device.
diff --git a/drivers/usb/dwc3/dwc3-uniphier.c b/drivers/usb/dwc3/dwc3-uniphier.c
index 54b52dcd66..175a97c5e4 100644
--- a/drivers/usb/dwc3/dwc3-uniphier.c
+++ b/drivers/usb/dwc3/dwc3-uniphier.c
@@ -4,14 +4,16 @@
  *
  * Copyright (C) 2016-2017 Socionext Inc.
  *   Author: Masahiro Yamada <yamada.masahiro@socionext.com>
+ *   Author: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
  */
 
 #include <dm.h>
-#include <dm/device_compat.h>
 #include <linux/bitops.h>
-#include <linux/errno.h>
-#include <linux/io.h>
-#include <linux/sizes.h>
+#include <linux/usb/gadget.h>
+
+#include "core.h"
+#include "gadget.h"
+#include "dwc3-generic.h"
 
 #define UNIPHIER_PRO4_DWC3_RESET	0x40
 #define   UNIPHIER_PRO4_DWC3_RESET_XIOMMU	BIT(5)
@@ -27,8 +29,11 @@
 #define UNIPHIER_PXS2_DWC3_RESET	0x00
 #define   UNIPHIER_PXS2_DWC3_RESET_XLINK	BIT(15)
 
-static int uniphier_pro4_dwc3_init(void __iomem *regs)
+static void uniphier_pro4_dwc3_init(struct udevice *dev, int index,
+				    enum usb_dr_mode mode)
 {
+	struct dwc3_glue_data *glue = dev_get_plat(dev);
+	void *regs = map_physmem(glue->regs, glue->size, MAP_NOCACHE);
 	u32 tmp;
 
 	tmp = readl(regs + UNIPHIER_PRO4_DWC3_RESET);
@@ -36,11 +41,14 @@ static int uniphier_pro4_dwc3_init(void __iomem *regs)
 	tmp |= UNIPHIER_PRO4_DWC3_RESET_XIOMMU | UNIPHIER_PRO4_DWC3_RESET_XLINK;
 	writel(tmp, regs + UNIPHIER_PRO4_DWC3_RESET);
 
-	return 0;
+	unmap_physmem(regs, MAP_NOCACHE);
 }
 
-static int uniphier_pro5_dwc3_init(void __iomem *regs)
+static void uniphier_pro5_dwc3_init(struct udevice *dev, int index,
+				    enum usb_dr_mode mode)
 {
+	struct dwc3_glue_data *glue = dev_get_plat(dev);
+	void *regs = map_physmem(glue->regs, glue->size, MAP_NOCACHE);
 	u32 tmp;
 
 	tmp = readl(regs + UNIPHIER_PRO5_DWC3_RESET);
@@ -49,72 +57,65 @@ static int uniphier_pro5_dwc3_init(void __iomem *regs)
 	tmp |= UNIPHIER_PRO5_DWC3_RESET_XLINK | UNIPHIER_PRO5_DWC3_RESET_XIOMMU;
 	writel(tmp, regs + UNIPHIER_PRO5_DWC3_RESET);
 
-	return 0;
+	unmap_physmem(regs, MAP_NOCACHE);
 }
 
-static int uniphier_pxs2_dwc3_init(void __iomem *regs)
+static void uniphier_pxs2_dwc3_init(struct udevice *dev, int index,
+				    enum usb_dr_mode mode)
 {
+	struct dwc3_glue_data *glue = dev_get_plat(dev);
+	void *regs = map_physmem(glue->regs, glue->size, MAP_NOCACHE);
 	u32 tmp;
 
 	tmp = readl(regs + UNIPHIER_PXS2_DWC3_RESET);
 	tmp |= UNIPHIER_PXS2_DWC3_RESET_XLINK;
 	writel(tmp, regs + UNIPHIER_PXS2_DWC3_RESET);
 
-	return 0;
+	unmap_physmem(regs, MAP_NOCACHE);
 }
 
-static int uniphier_dwc3_probe(struct udevice *dev)
-{
-	fdt_addr_t base;
-	void __iomem *regs;
-	int (*init)(void __iomem *regs);
-	int ret;
-
-	base = dev_read_addr(dev);
-	if (base == FDT_ADDR_T_NONE)
-		return -EINVAL;
-
-	regs = ioremap(base, SZ_32K);
-	if (!regs)
-		return -ENOMEM;
-
-	init = (typeof(init))dev_get_driver_data(dev);
-	ret = init(regs);
-	if (ret)
-		dev_err(dev, "failed to init glue layer\n");
+struct dwc3_glue_ops uniphier_pro4_dwc3_ops = {
+	.glue_configure = uniphier_pro4_dwc3_init,
+};
 
-	iounmap(regs);
+struct dwc3_glue_ops uniphier_pro5_dwc3_ops = {
+	.glue_configure = uniphier_pro5_dwc3_init,
+};
 
-	return ret;
-}
+struct dwc3_glue_ops uniphier_pxs2_dwc3_ops = {
+	.glue_configure = uniphier_pxs2_dwc3_init,
+};
 
 static const struct udevice_id uniphier_dwc3_match[] = {
 	{
 		.compatible = "socionext,uniphier-pro4-dwc3",
-		.data = (ulong)uniphier_pro4_dwc3_init,
+		.data = (ulong)&uniphier_pro4_dwc3_ops,
 	},
 	{
 		.compatible = "socionext,uniphier-pro5-dwc3",
-		.data = (ulong)uniphier_pro5_dwc3_init,
+		.data = (ulong)&uniphier_pro5_dwc3_ops,
 	},
 	{
 		.compatible = "socionext,uniphier-pxs2-dwc3",
-		.data = (ulong)uniphier_pxs2_dwc3_init,
+		.data = (ulong)&uniphier_pxs2_dwc3_ops,
 	},
 	{
 		.compatible = "socionext,uniphier-ld20-dwc3",
-		.data = (ulong)uniphier_pxs2_dwc3_init,
+		.data = (ulong)&uniphier_pxs2_dwc3_ops,
 	},
 	{
 		.compatible = "socionext,uniphier-pxs3-dwc3",
-		.data = (ulong)uniphier_pxs2_dwc3_init,
+		.data = (ulong)&uniphier_pxs2_dwc3_ops,
 	},
 	{ /* sentinel */ }
 };
 
-U_BOOT_DRIVER(usb_xhci) = {
+U_BOOT_DRIVER(dwc3_uniphier_wrapper) = {
 	.name = "uniphier-dwc3",
 	.id = UCLASS_SIMPLE_BUS,
 	.of_match = uniphier_dwc3_match,
-	.probe = uniphier_dwc3_probe,
+	.bind = dwc3_glue_bind,
+	.probe = dwc3_glue_probe,
+	.remove = dwc3_glue_remove,
+	.plat_auto = sizeof(struct dwc3_glue_data),
 };
-- 
2.25.1

[PATCH 5/5] uniphier_defconfig: Disable USB_XHCI_DWC3

[Kunihiko Hayashi]

Replacing with dwc3-generic, no need USB_XHCI_DWC3 anymore.

Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
 configs/uniphier_v7_defconfig | 1 -
 configs/uniphier_v8_defconfig | 1 -
 2 files changed, 2 deletions(-)

diff --git a/configs/uniphier_v7_defconfig b/configs/uniphier_v7_defconfig
index d626968c76..03feb04b93 100644
--- a/configs/uniphier_v7_defconfig
+++ b/configs/uniphier_v7_defconfig
@@ -82,7 +82,6 @@ CONFIG_DM_SPI=y
 CONFIG_UNIPHIER_SPI=y
 CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
-CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
 CONFIG_USB_DWC3=y
diff --git a/configs/uniphier_v8_defconfig b/configs/uniphier_v8_defconfig
index 6a0e2666cf..ed58b5746e 100644
--- a/configs/uniphier_v8_defconfig
+++ b/configs/uniphier_v8_defconfig
@@ -71,7 +71,6 @@ CONFIG_SYSRESET=y
 CONFIG_SYSRESET_PSCI=y
 CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
-CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_GENERIC=y
 CONFIG_USB_DWC3=y
-- 
2.25.1

Re: [PATCH 1/5] usb: dwc3-generic: Export glue structures and functions

[Marek Vasut]

On 1/23/23 01:47, Kunihiko Hayashi wrote:
> In order to allow external SoC-dependent glue drivers to use dwc3-generic
> functions, push the glue structures and export the functions to a header
> file.
> 
> The exported structures and functions are:
> 
> - struct dwc3_glue_data
> - struct dwc3_glue_ops
> - dwc3_glue_bind()
> - dwc3_glue_probe()
> - dwc3_glue_remove()
> 
> The SoC-dependent glue drivers can only define their own wrapper driver
> and specify these functions. The drivers can also add their own compatible
> strings and configure functions.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Re: [PATCH 2/5] usb: dwc3-generic: Add the size of regs property to glue structure

[Marek Vasut]

On 1/23/23 01:47, Kunihiko Hayashi wrote:
> Add the size of regs property to the glue structure to correctly
> specify the register region to map.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Marek Vasut]

On 1/23/23 01:47, Kunihiko Hayashi wrote:
> The glue driver doesn't do or offer actively anything, SIMPLE_BUS is
> more preferable to represent the driver.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
> ---
>   drivers/usb/dwc3/Kconfig | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index f010291d02..dadaa083e7 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -25,14 +25,14 @@ config USB_DWC3_OMAP
>   
>   config USB_DWC3_GENERIC
>   	bool "Generic implementation of a DWC3 wrapper (aka dwc3 glue)"
> -	depends on DM_USB && USB_DWC3 && MISC
> +	depends on DM_USB && USB_DWC3 && (MISC || SIMPLE_BUS)

I'm afraid I don't understand why this change is needed for all variants 
of DWC3.

Is this needed for socionext dwc3 variant to handle the simple-mfd in 
e.g. arch/arm/dts/uniphier-pxs3.dtsi :

614 usb-glue@65b00000 {
615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
616              "simple-mfd";

?

Re: [PATCH 4/5] usb: dwc3-uniphier: Use dwc3-generic instead of xhci-dwc3

[Marek Vasut]

On 1/23/23 01:47, Kunihiko Hayashi wrote:
> dwc3-uniphier depends on xhci-dwc3 framework, however, it is preferable
> to use dwc3-generic.
> 
> This driver calls the exported dwc3-generic functions and redefine
> the SoC-dependent operations to fit dwc3-generic.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Re: [PATCH 5/5] uniphier_defconfig: Disable USB_XHCI_DWC3

[Marek Vasut]

On 1/23/23 01:47, Kunihiko Hayashi wrote:
> Replacing with dwc3-generic, no need USB_XHCI_DWC3 anymore.
> 
> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>

Reviewed-by: Marek Vasut <marex@denx.de>

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

Hi Marek,

Thank you for reviewing.

On 2023/01/23 10:42, Marek Vasut wrote:
> On 1/23/23 01:47, Kunihiko Hayashi wrote:
>> The glue driver doesn't do or offer actively anything, SIMPLE_BUS is
>> more preferable to represent the driver.
>>
>> Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
>> ---
>>    drivers/usb/dwc3/Kconfig | 4 ++--
>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>> index f010291d02..dadaa083e7 100644
>> --- a/drivers/usb/dwc3/Kconfig
>> +++ b/drivers/usb/dwc3/Kconfig
>> @@ -25,14 +25,14 @@ config USB_DWC3_OMAP
>>    
>>    config USB_DWC3_GENERIC
>>    	bool "Generic implementation of a DWC3 wrapper (aka dwc3 glue)"
>> -	depends on DM_USB && USB_DWC3 && MISC
>> +	depends on DM_USB && USB_DWC3 && (MISC || SIMPLE_BUS)
> 
> I'm afraid I don't understand why this change is needed for all variants
> of DWC3.

Although dwc3-generic is declared as UCLASS_NOP, the similar glue driver
dwc3-meson-glx is declared as UCLASS_SIMPLE_BUS.

> Is this needed for socionext dwc3 variant to handle the simple-mfd in
> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
> 
> 614 usb-glue@65b00000 {
> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
> 616              "simple-mfd";
> 
> ?

In case of U-Boot, the glue driver is probed by:

     /* FIXME: U-Boot own node */
     usb@65b00000 {
             compatible = "socionext,uniphier-pxs3-dwc3";

And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
Even if using "simple-mfd", this is included in drivers/core/simple-bus.c
which is declared as UCLASS_SIMPLE_BUS.

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Marek Vasut]

On 1/23/23 04:08, Kunihiko Hayashi wrote:

Hello Hayashi-san,

>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>>> index f010291d02..dadaa083e7 100644
>>> --- a/drivers/usb/dwc3/Kconfig
>>> +++ b/drivers/usb/dwc3/Kconfig
>>> @@ -25,14 +25,14 @@ config USB_DWC3_OMAP
>>>    config USB_DWC3_GENERIC
>>>        bool "Generic implementation of a DWC3 wrapper (aka dwc3 glue)"
>>> -    depends on DM_USB && USB_DWC3 && MISC
>>> +    depends on DM_USB && USB_DWC3 && (MISC || SIMPLE_BUS)
>>
>> I'm afraid I don't understand why this change is needed for all variants
>> of DWC3.
> 
> Although dwc3-generic is declared as UCLASS_NOP, the similar glue driver
> dwc3-meson-glx is declared as UCLASS_SIMPLE_BUS.

I think here it makes sense, because "amlogic,meson-axg-usb-ctrl" 
behaves like a bus with subnodes:

arch/arm/dts/meson-axg.dtsi:
  227 usb: usb@ffe09080 {
  228     compatible = "amlogic,meson-axg-usb-ctrl";
  229     reg = <0x0 0xffe09080 0x0 0x20>;
...
  244     dwc2: usb@ff400000 {
  245         compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
  246         reg = <0x0 0xff400000 0x0 0x40000>;
...
  255     };
  256
  257     dwc3: usb@ff500000 {
  258         compatible = "snps,dwc3";
  259         reg = <0x0 0xff500000 0x0 0x100000>;
...
  264     };
  265 };

arch/arm/dts/meson-gxl.dtsi
  17 usb: usb@d0078080 {
  18     compatible = "amlogic,meson-gxl-usb-ctrl";
  19     reg = <0x0 0xd0078080 0x0 0x20>;
...
  34     dwc2: usb@c9100000 {
  35         compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
  36         reg = <0x0 0xc9100000 0x0 0x40000>;
...
  45     };
  46
  47     dwc3: usb@c9000000 {
  48         compatible = "snps,dwc3";
  49         reg = <0x0 0xc9000000 0x0 0x100000>;
...
  54     };
  55 };

On the other hand, the PXS2 controller for example is not a bus:

arch/arm/dts/uniphier-pxs2.dtsi:
596 _usb0: usb@65a00000 {
597     compatible = "socionext,uniphier-dwc3", "snps,dwc3";
598     status = "disabled";
599     reg = <0x65a00000 0xcd00>;
...
610 };

>> Is this needed for socionext dwc3 variant to handle the simple-mfd in
>> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
>>
>> 614 usb-glue@65b00000 {
>> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
>> 616              "simple-mfd";
>>
>> ?
> 
> In case of U-Boot, the glue driver is probed by:
> 
>      /* FIXME: U-Boot own node */
>      usb@65b00000 {
>              compatible = "socionext,uniphier-pxs3-dwc3";
> 
> And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
> Even if using "simple-mfd", this is included in drivers/core/simple-bus.c
> which is declared as UCLASS_SIMPLE_BUS.

If I understand this correctly, node compatible with 
"socionext,uniphier-pxs3-dwc3-glue" is not used at all , right ?

The generic driver binds to node compatible with 
"socionext,uniphier-dwc3" , right ?

That means, there is nothing which would be a bus, and so the 
dwc3-uniphier.c can be switched from UCLASS_SIMPLE_BUS to UCLASS_NOP , 
is that correct ?

[...]

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

Hi Marek,

On 2023/01/23 12:37, Marek Vasut wrote:
> On 1/23/23 04:08, Kunihiko Hayashi wrote:
> 
> Hello Hayashi-san,
> 
>>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>>>> index f010291d02..dadaa083e7 100644
>>>> --- a/drivers/usb/dwc3/Kconfig
>>>> +++ b/drivers/usb/dwc3/Kconfig
>>>> @@ -25,14 +25,14 @@ config USB_DWC3_OMAP
>>>>     config USB_DWC3_GENERIC
>>>>         bool "Generic implementation of a DWC3 wrapper (aka dwc3 glue)"
>>>> -    depends on DM_USB && USB_DWC3 && MISC
>>>> +    depends on DM_USB && USB_DWC3 && (MISC || SIMPLE_BUS)
>>>
>>> I'm afraid I don't understand why this change is needed for all variants
>>> of DWC3.
>>
>> Although dwc3-generic is declared as UCLASS_NOP, the similar glue driver
>> dwc3-meson-glx is declared as UCLASS_SIMPLE_BUS.
> 
> I think here it makes sense, because "amlogic,meson-axg-usb-ctrl"
> behaves like a bus with subnodes:
> 
> arch/arm/dts/meson-axg.dtsi:
>    227 usb: usb@ffe09080 {
>    228     compatible = "amlogic,meson-axg-usb-ctrl";
>    229     reg = <0x0 0xffe09080 0x0 0x20>;
> ...
>    244     dwc2: usb@ff400000 {
>    245         compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
>    246         reg = <0x0 0xff400000 0x0 0x40000>;
> ...
>    255     };
>    256
>    257     dwc3: usb@ff500000 {
>    258         compatible = "snps,dwc3";
>    259         reg = <0x0 0xff500000 0x0 0x100000>;
> ...
>    264     };
>    265 };
> 
> arch/arm/dts/meson-gxl.dtsi
>    17 usb: usb@d0078080 {
>    18     compatible = "amlogic,meson-gxl-usb-ctrl";
>    19     reg = <0x0 0xd0078080 0x0 0x20>;
> ...
>    34     dwc2: usb@c9100000 {
>    35         compatible = "amlogic,meson-g12a-usb", "snps,dwc2";
>    36         reg = <0x0 0xc9100000 0x0 0x40000>;
> ...
>    45     };
>    46
>    47     dwc3: usb@c9000000 {
>    48         compatible = "snps,dwc3";
>    49         reg = <0x0 0xc9000000 0x0 0x100000>;
> ...
>    54     };
>    55 };
> 
> On the other hand, the PXS2 controller for example is not a bus:
> 
> arch/arm/dts/uniphier-pxs2.dtsi:
> 596 _usb0: usb@65a00000 {
> 597     compatible = "socionext,uniphier-dwc3", "snps,dwc3";
> 598     status = "disabled";
> 599     reg = <0x65a00000 0xcd00>;
> ...
> 610 };

I understand. However, this node isn't used in u-boot.
(see below for details)

>>> Is this needed for socionext dwc3 variant to handle the simple-mfd in
>>> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
>>>
>>> 614 usb-glue@65b00000 {
>>> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
>>> 616              "simple-mfd";
>>>
>>> ?
>>
>> In case of U-Boot, the glue driver is probed by:
>>
>>       /* FIXME: U-Boot own node */
>>       usb@65b00000 {
>>               compatible = "socionext,uniphier-pxs3-dwc3";
>>
>> And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
>> Even if using "simple-mfd", this is included in drivers/core/simple-bus.c
>> which is declared as UCLASS_SIMPLE_BUS.
> 
> If I understand this correctly, node compatible with
> "socionext,uniphier-pxs3-dwc3-glue" is not used at all , right ?

Yes.
Original uniphier devicetree has the following usb nodes.

     usb@65a00000 {
         compatible = "snps,dwc3";
     };
     usb-glue@65b00000 {
         compatible = "socionext,uniphier-pxs3-dwc3-glue", "simple-mfd";
     };

However, U-Boot dwc3-generic needs to put dwc3 node under the glue node.
Due to this restriction, there is another usb node dedicated to u-boot.

     /* FIXME: U-Boot own node */
     usb@65b00000 { /* glue */
         compatible = "socionext,uniphier-pxs3-dwc3";

         dwc3@65a00000 {
             compatible = "snps,dwc3";
         };
     };

So instead of "socionext,uniphier-pxs3-dwc3-glue", the glue driver
uses "socionext,uniphier-pxs3-dwc3" in U-Boot.

> The generic driver binds to node compatible with
> "socionext,uniphier-dwc3" , right ?

No, the generic driver binds "socionext,uniphier-pxs3-dwc3".

> That means, there is nothing which would be a bus, and so the
> dwc3-uniphier.c can be switched from UCLASS_SIMPLE_BUS to UCLASS_NOP ,
> is that correct ?

There is still the issue of different usb node between Original and u-boot,
however, the glue driver can be switched to UCLASS_NOP.

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Marek Vasut]

On 1/23/23 06:01, Kunihiko Hayashi wrote:
> Hi Marek,

Hello Hayashi-san,

[...]

>> On the other hand, the PXS2 controller for example is not a bus:
>>
>> arch/arm/dts/uniphier-pxs2.dtsi:
>> 596 _usb0: usb@65a00000 {
>> 597     compatible = "socionext,uniphier-dwc3", "snps,dwc3";
>> 598     status = "disabled";
>> 599     reg = <0x65a00000 0xcd00>;
>> ...
>> 610 };
> 
> I understand. However, this node isn't used in u-boot.
> (see below for details)
> 
>>>> Is this needed for socionext dwc3 variant to handle the simple-mfd in
>>>> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
>>>>
>>>> 614 usb-glue@65b00000 {
>>>> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
>>>> 616              "simple-mfd";
>>>>
>>>> ?
>>>
>>> In case of U-Boot, the glue driver is probed by:
>>>
>>>       /* FIXME: U-Boot own node */
>>>       usb@65b00000 {
>>>               compatible = "socionext,uniphier-pxs3-dwc3";
>>>
>>> And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
>>> Even if using "simple-mfd", this is included in 
>>> drivers/core/simple-bus.c
>>> which is declared as UCLASS_SIMPLE_BUS.
>>
>> If I understand this correctly, node compatible with
>> "socionext,uniphier-pxs3-dwc3-glue" is not used at all , right ?
> 
> Yes.
> Original uniphier devicetree has the following usb nodes.
> 
>      usb@65a00000 {
>          compatible = "snps,dwc3";
>      };
>      usb-glue@65b00000 {
>          compatible = "socionext,uniphier-pxs3-dwc3-glue", "simple-mfd";
>      };
> 
> However, U-Boot dwc3-generic needs to put dwc3 node under the glue node.

Have a look at arch/arm/dts/imx8mq.dtsi which does not use any glue 
node, the snps,dwc3 compatible node is directly placed on the soc@0 bus:

1417
1418         usb_dwc3_0: usb@38100000 {
1419             compatible = "fsl,imx8mq-dwc3", "snps,dwc3";
1420             reg = <0x38100000 0x10000>;
1421             clocks = <&clk IMX8MQ_CLK_USB1_CTRL_ROOT>,

> Due to this restriction, there is another usb node dedicated to u-boot.
> 
>      /* FIXME: U-Boot own node */
>      usb@65b00000 { /* glue */
>          compatible = "socionext,uniphier-pxs3-dwc3";
> 
>          dwc3@65a00000 {
>              compatible = "snps,dwc3";
>          };
>      };
> 
> So instead of "socionext,uniphier-pxs3-dwc3-glue", the glue driver
> uses "socionext,uniphier-pxs3-dwc3" in U-Boot.

Can we use the same method as imx8mq.dtsi uses, to avoid this special 
node workaround ?

>> The generic driver binds to node compatible with
>> "socionext,uniphier-dwc3" , right ?
> 
> No, the generic driver binds "socionext,uniphier-pxs3-dwc3".
> 
>> That means, there is nothing which would be a bus, and so the
>> dwc3-uniphier.c can be switched from UCLASS_SIMPLE_BUS to UCLASS_NOP ,
>> is that correct ?
> 
> There is still the issue of different usb node between Original and u-boot,
> however, the glue driver can be switched to UCLASS_NOP.

Understood, thank you for the clarification.

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

Hi Marek,

On 2023/01/24 0:49, Marek Vasut wrote:
> On 1/23/23 06:01, Kunihiko Hayashi wrote:
>> Hi Marek,
> 
> Hello Hayashi-san,
> 
> [...]
> 
>>> On the other hand, the PXS2 controller for example is not a bus:
>>>
>>> arch/arm/dts/uniphier-pxs2.dtsi:
>>> 596 _usb0: usb@65a00000 {
>>> 597     compatible = "socionext,uniphier-dwc3", "snps,dwc3";
>>> 598     status = "disabled";
>>> 599     reg = <0x65a00000 0xcd00>;
>>> ...
>>> 610 };
>>
>> I understand. However, this node isn't used in u-boot.
>> (see below for details)
>>
>>>>> Is this needed for socionext dwc3 variant to handle the simple-mfd in
>>>>> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
>>>>>
>>>>> 614 usb-glue@65b00000 {
>>>>> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
>>>>> 616              "simple-mfd";
>>>>>
>>>>> ?
>>>>
>>>> In case of U-Boot, the glue driver is probed by:
>>>>
>>>>        /* FIXME: U-Boot own node */
>>>>        usb@65b00000 {
>>>>                compatible = "socionext,uniphier-pxs3-dwc3";
>>>>
>>>> And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
>>>> Even if using "simple-mfd", this is included in
>>>> drivers/core/simple-bus.c
>>>> which is declared as UCLASS_SIMPLE_BUS.
>>>
>>> If I understand this correctly, node compatible with
>>> "socionext,uniphier-pxs3-dwc3-glue" is not used at all , right ?
>>
>> Yes.
>> Original uniphier devicetree has the following usb nodes.
>>
>>       usb@65a00000 {
>>           compatible = "snps,dwc3";
>>       };
>>       usb-glue@65b00000 {
>>           compatible = "socionext,uniphier-pxs3-dwc3-glue", "simple-mfd";
>>       };
>>
>> However, U-Boot dwc3-generic needs to put dwc3 node under the glue node.
> 
> Have a look at arch/arm/dts/imx8mq.dtsi which does not use any glue
> node, the snps,dwc3 compatible node is directly placed on the soc@0 bus:
> 
> 1417
> 1418         usb_dwc3_0: usb@38100000 {
> 1419             compatible = "fsl,imx8mq-dwc3", "snps,dwc3";
> 1420             reg = <0x38100000 0x10000>;
> 1421             clocks = <&clk IMX8MQ_CLK_USB1_CTRL_ROOT>,
> 
>> Due to this restriction, there is another usb node dedicated to u-boot.
>>
>>       /* FIXME: U-Boot own node */
>>       usb@65b00000 { /* glue */
>>           compatible = "socionext,uniphier-pxs3-dwc3";
>>
>>           dwc3@65a00000 {
>>               compatible = "snps,dwc3";
>>           };
>>       };
>>
>> So instead of "socionext,uniphier-pxs3-dwc3-glue", the glue driver
>> uses "socionext,uniphier-pxs3-dwc3" in U-Boot.
> 
> Can we use the same method as imx8mq.dtsi uses, to avoid this special
> node workaround ?

Umm, it's curious. It seems imx8mq doesn't have any glue registers and defines
dwc3 core registers directly.

On the other hand, "fsl,imx8mq-dwc3" is included in dwc3-generic.c, though,
dwc3_glue_bind() calls the driver that the child node shows.

     ofnode_for_each_subnode(node, dev_ofnode(parent)) {
         ...
         device_bind_driver_to_node(parent, driver, name,
                                    node, &dev);
         ...
     }

Maybe the child node expects the driver that "snps,dwc3" indicates.

If we use the same method as imx8mq.dtsi, I think there is no way to probe
the driver for "snps,dwc3" in dwc3-generic.c.

BTW, xchi-dwc3.c can handle "snps,dwc3", but it seems this doesn't use
usb/dwc3/ functions (so I'm confusing it).

>>> The generic driver binds to node compatible with
>>> "socionext,uniphier-dwc3" , right ?
>>
>> No, the generic driver binds "socionext,uniphier-pxs3-dwc3".
>>
>>> That means, there is nothing which would be a bus, and so the
>>> dwc3-uniphier.c can be switched from UCLASS_SIMPLE_BUS to UCLASS_NOP ,
>>> is that correct ?
>>
>> There is still the issue of different usb node between Original and
>> u-boot,
>> however, the glue driver can be switched to UCLASS_NOP.
> 
> Understood, thank you for the clarification.

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Marek Vasut]

On 1/24/23 03:53, Kunihiko Hayashi wrote:
> Hi Marek,

Hello Hayashi-san,

> On 2023/01/24 0:49, Marek Vasut wrote:
>> On 1/23/23 06:01, Kunihiko Hayashi wrote:
>>> Hi Marek,
>>
>> Hello Hayashi-san,
>>
>> [...]
>>
>>>> On the other hand, the PXS2 controller for example is not a bus:
>>>>
>>>> arch/arm/dts/uniphier-pxs2.dtsi:
>>>> 596 _usb0: usb@65a00000 {
>>>> 597     compatible = "socionext,uniphier-dwc3", "snps,dwc3";
>>>> 598     status = "disabled";
>>>> 599     reg = <0x65a00000 0xcd00>;
>>>> ...
>>>> 610 };
>>>
>>> I understand. However, this node isn't used in u-boot.
>>> (see below for details)
>>>
>>>>>> Is this needed for socionext dwc3 variant to handle the simple-mfd in
>>>>>> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
>>>>>>
>>>>>> 614 usb-glue@65b00000 {
>>>>>> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
>>>>>> 616              "simple-mfd";
>>>>>>
>>>>>> ?
>>>>>
>>>>> In case of U-Boot, the glue driver is probed by:
>>>>>
>>>>>        /* FIXME: U-Boot own node */
>>>>>        usb@65b00000 {
>>>>>                compatible = "socionext,uniphier-pxs3-dwc3";
>>>>>
>>>>> And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
>>>>> Even if using "simple-mfd", this is included in
>>>>> drivers/core/simple-bus.c
>>>>> which is declared as UCLASS_SIMPLE_BUS.
>>>>
>>>> If I understand this correctly, node compatible with
>>>> "socionext,uniphier-pxs3-dwc3-glue" is not used at all , right ?
>>>
>>> Yes.
>>> Original uniphier devicetree has the following usb nodes.
>>>
>>>       usb@65a00000 {
>>>           compatible = "snps,dwc3";
>>>       };
>>>       usb-glue@65b00000 {
>>>           compatible = "socionext,uniphier-pxs3-dwc3-glue", 
>>> "simple-mfd";
>>>       };
>>>
>>> However, U-Boot dwc3-generic needs to put dwc3 node under the glue node.
>>
>> Have a look at arch/arm/dts/imx8mq.dtsi which does not use any glue
>> node, the snps,dwc3 compatible node is directly placed on the soc@0 bus:
>>
>> 1417
>> 1418         usb_dwc3_0: usb@38100000 {
>> 1419             compatible = "fsl,imx8mq-dwc3", "snps,dwc3";
>> 1420             reg = <0x38100000 0x10000>;
>> 1421             clocks = <&clk IMX8MQ_CLK_USB1_CTRL_ROOT>,
>>
>>> Due to this restriction, there is another usb node dedicated to u-boot.
>>>
>>>       /* FIXME: U-Boot own node */
>>>       usb@65b00000 { /* glue */
>>>           compatible = "socionext,uniphier-pxs3-dwc3";
>>>
>>>           dwc3@65a00000 {
>>>               compatible = "snps,dwc3";
>>>           };
>>>       };
>>>
>>> So instead of "socionext,uniphier-pxs3-dwc3-glue", the glue driver
>>> uses "socionext,uniphier-pxs3-dwc3" in U-Boot.
>>
>> Can we use the same method as imx8mq.dtsi uses, to avoid this special
>> node workaround ?
> 
> Umm, it's curious. It seems imx8mq doesn't have any glue registers and 
> defines
> dwc3 core registers directly.
> 
> On the other hand, "fsl,imx8mq-dwc3" is included in dwc3-generic.c, though,
> dwc3_glue_bind() calls the driver that the child node shows.
> 
>      ofnode_for_each_subnode(node, dev_ofnode(parent)) {
>          ...
>          device_bind_driver_to_node(parent, driver, name,
>                                     node, &dev);
>          ...
>      }
> 
> Maybe the child node expects the driver that "snps,dwc3" indicates.
> 
> If we use the same method as imx8mq.dtsi, I think there is no way to probe
> the driver for "snps,dwc3" in dwc3-generic.c.
> 
> BTW, xchi-dwc3.c can handle "snps,dwc3", but it seems this doesn't use
> usb/dwc3/ functions (so I'm confusing it).

Let's try the following, please have a look at this change:

https://source.denx.de/u-boot/custodians/u-boot-usb/-/commit/028ad9536e501986f4e19d27f462f81a9ea70bad

The change is difficult to read, use 'git show -w' to ignore space 
changes, that might help.

The idea is that the dwc3-generic.c (or dwc3-uniphier.c , placement does 
not really matter) binds to "socionext,uniphier-pxs3-dwc3-glue" 
compatible first.

Then, the dwc3_glue_ops is extended with a new callback 
glue_get_ctrl_dev, which returns the pointer to controller DT node (the 
node compatible with "socionext,uniphier-dwc3"). This is used in 
dwc3_glue_bind(), which either uses the current implementation with a 
loop over all subnodes and tries to find the controller subnode, OR, 
calls the glue_get_ctrl_dev callback, obtains controller device node 
pointer that way, and runs the inner loop of dwc3_glue_bind() (now 
dwc3_glue_bind_common()) only on that pointer.

In either case, the dwc3-generic driver is bound to the controller. You 
might need to re-use this trick in dwc3_glue_probe() too.

This should allow you to get rid of the custom DT node too.

Does this work ?

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

Hi Marek,

On 2023/01/25 10:38, Marek Vasut wrote:
> On 1/24/23 03:53, Kunihiko Hayashi wrote:
>> Hi Marek,
> 
> Hello Hayashi-san,
> 
>> On 2023/01/24 0:49, Marek Vasut wrote:
>>> On 1/23/23 06:01, Kunihiko Hayashi wrote:
>>>> Hi Marek,
>>>
>>> Hello Hayashi-san,
>>>
>>> [...]
>>>
>>>>> On the other hand, the PXS2 controller for example is not a bus:
>>>>>
>>>>> arch/arm/dts/uniphier-pxs2.dtsi:
>>>>> 596 _usb0: usb@65a00000 {
>>>>> 597     compatible = "socionext,uniphier-dwc3", "snps,dwc3";
>>>>> 598     status = "disabled";
>>>>> 599     reg = <0x65a00000 0xcd00>;
>>>>> ...
>>>>> 610 };
>>>>
>>>> I understand. However, this node isn't used in u-boot.
>>>> (see below for details)
>>>>
>>>>>>> Is this needed for socionext dwc3 variant to handle the simple-mfd in
>>>>>>> e.g. arch/arm/dts/uniphier-pxs3.dtsi :
>>>>>>>
>>>>>>> 614 usb-glue@65b00000 {
>>>>>>> 615     compatible = "socionext,uniphier-pxs3-dwc3-glue",
>>>>>>> 616              "simple-mfd";
>>>>>>>
>>>>>>> ?
>>>>>>
>>>>>> In case of U-Boot, the glue driver is probed by:
>>>>>>
>>>>>>         /* FIXME: U-Boot own node */
>>>>>>         usb@65b00000 {
>>>>>>                 compatible = "socionext,uniphier-pxs3-dwc3";
>>>>>>
>>>>>> And dwc3-uniphier is also declared as UCLASS_SIMPLE_BUS.
>>>>>> Even if using "simple-mfd", this is included in
>>>>>> drivers/core/simple-bus.c
>>>>>> which is declared as UCLASS_SIMPLE_BUS.
>>>>>
>>>>> If I understand this correctly, node compatible with
>>>>> "socionext,uniphier-pxs3-dwc3-glue" is not used at all , right ?
>>>>
>>>> Yes.
>>>> Original uniphier devicetree has the following usb nodes.
>>>>
>>>>        usb@65a00000 {
>>>>            compatible = "snps,dwc3";
>>>>        };
>>>>        usb-glue@65b00000 {
>>>>            compatible = "socionext,uniphier-pxs3-dwc3-glue",
>>>> "simple-mfd";
>>>>        };
>>>>
>>>> However, U-Boot dwc3-generic needs to put dwc3 node under the glue node.
>>>
>>> Have a look at arch/arm/dts/imx8mq.dtsi which does not use any glue
>>> node, the snps,dwc3 compatible node is directly placed on the soc@0 bus:
>>>
>>> 1417
>>> 1418         usb_dwc3_0: usb@38100000 {
>>> 1419             compatible = "fsl,imx8mq-dwc3", "snps,dwc3";
>>> 1420             reg = <0x38100000 0x10000>;
>>> 1421             clocks = <&clk IMX8MQ_CLK_USB1_CTRL_ROOT>,
>>>
>>>> Due to this restriction, there is another usb node dedicated to u-boot.
>>>>
>>>>        /* FIXME: U-Boot own node */
>>>>        usb@65b00000 { /* glue */
>>>>            compatible = "socionext,uniphier-pxs3-dwc3";
>>>>
>>>>            dwc3@65a00000 {
>>>>                compatible = "snps,dwc3";
>>>>            };
>>>>        };
>>>>
>>>> So instead of "socionext,uniphier-pxs3-dwc3-glue", the glue driver
>>>> uses "socionext,uniphier-pxs3-dwc3" in U-Boot.
>>>
>>> Can we use the same method as imx8mq.dtsi uses, to avoid this special
>>> node workaround ?
>>
>> Umm, it's curious. It seems imx8mq doesn't have any glue registers and
>> defines
>> dwc3 core registers directly.
>>
>> On the other hand, "fsl,imx8mq-dwc3" is included in dwc3-generic.c,
>> though,
>> dwc3_glue_bind() calls the driver that the child node shows.
>>
>>       ofnode_for_each_subnode(node, dev_ofnode(parent)) {
>>           ...
>>           device_bind_driver_to_node(parent, driver, name,
>>                                      node, &dev);
>>           ...
>>       }
>>
>> Maybe the child node expects the driver that "snps,dwc3" indicates.
>>
>> If we use the same method as imx8mq.dtsi, I think there is no way to probe
>> the driver for "snps,dwc3" in dwc3-generic.c.
>>
>> BTW, xchi-dwc3.c can handle "snps,dwc3", but it seems this doesn't use
>> usb/dwc3/ functions (so I'm confusing it).
> 
> Let's try the following, please have a look at this change:
> 
> https://source.denx.de/u-boot/custodians/u-boot-usb/-/commit/028ad9536e501986f4e19d27f462f81a9ea70bad
> 
> The change is difficult to read, use 'git show -w' to ignore space
> changes, that might help.

Thanks a lot for your work!

> The idea is that the dwc3-generic.c (or dwc3-uniphier.c , placement does
> not really matter) binds to "socionext,uniphier-pxs3-dwc3-glue"
> compatible first.
> 
> Then, the dwc3_glue_ops is extended with a new callback
> glue_get_ctrl_dev, which returns the pointer to controller DT node (the
> node compatible with "socionext,uniphier-dwc3"). This is used in
> dwc3_glue_bind(), which either uses the current implementation with a
> loop over all subnodes and tries to find the controller subnode, OR,
> calls the glue_get_ctrl_dev callback, obtains controller device node
> pointer that way, and runs the inner loop of dwc3_glue_bind() (now
> dwc3_glue_bind_common()) only on that pointer.

I understand your patch adds trying to call "glue_get_ctrl_dev" to get
controller device node before finding subnode,

> In either case, the dwc3-generic driver is bound to the controller. You
> might need to re-use this trick in dwc3_glue_probe() too.
> 
> This should allow you to get rid of the custom DT node too.
> 
> Does this work ?

I'll try this soon.

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Marek Vasut]

On 1/25/23 09:40, Kunihiko Hayashi wrote:
> Hi Marek,

Hello Hayashi-san,

[...]

>> The idea is that the dwc3-generic.c (or dwc3-uniphier.c , placement does
>> not really matter) binds to "socionext,uniphier-pxs3-dwc3-glue"
>> compatible first.
>>
>> Then, the dwc3_glue_ops is extended with a new callback
>> glue_get_ctrl_dev, which returns the pointer to controller DT node (the
>> node compatible with "socionext,uniphier-dwc3"). This is used in
>> dwc3_glue_bind(), which either uses the current implementation with a
>> loop over all subnodes and tries to find the controller subnode, OR,
>> calls the glue_get_ctrl_dev callback, obtains controller device node
>> pointer that way, and runs the inner loop of dwc3_glue_bind() (now
>> dwc3_glue_bind_common()) only on that pointer.
> 
> I understand your patch adds trying to call "glue_get_ctrl_dev" to get
> controller device node before finding subnode,

Yes. Feel free to adjust the patch so it fits uniphier better of course.

>> In either case, the dwc3-generic driver is bound to the controller. You
>> might need to re-use this trick in dwc3_glue_probe() too.
>>
>> This should allow you to get rid of the custom DT node too.
>>
>> Does this work ?
> 
> I'll try this soon.

Thank you !

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

Hi Marek,

Sorry for late reply.
I was stuck in some pitfalls.

On 2023/01/25 22:03, Marek Vasut wrote:
> On 1/25/23 09:40, Kunihiko Hayashi wrote:
>> Hi Marek,
> 
> Hello Hayashi-san,
> 
> [...]
> 
>>> The idea is that the dwc3-generic.c (or dwc3-uniphier.c , placement does
>>> not really matter) binds to "socionext,uniphier-pxs3-dwc3-glue"
>>> compatible first.
>>>
>>> Then, the dwc3_glue_ops is extended with a new callback
>>> glue_get_ctrl_dev, which returns the pointer to controller DT node (the
>>> node compatible with "socionext,uniphier-dwc3"). This is used in
>>> dwc3_glue_bind(), which either uses the current implementation with a
>>> loop over all subnodes and tries to find the controller subnode, OR,
>>> calls the glue_get_ctrl_dev callback, obtains controller device node
>>> pointer that way, and runs the inner loop of dwc3_glue_bind() (now
>>> dwc3_glue_bind_common()) only on that pointer.
>>
>> I understand your patch adds trying to call "glue_get_ctrl_dev" to get
>> controller device node before finding subnode,
> 
> Yes. Feel free to adjust the patch so it fits uniphier better of course.
> 
>>> In either case, the dwc3-generic driver is bound to the controller. You
>>> might need to re-use this trick in dwc3_glue_probe() too.
>>>
>>> This should allow you to get rid of the custom DT node too.
>>>
>>> Does this work ?
>>
>> I'll try this soon.
> 
> Thank you !

I've successfully enabled the regular usb node using your suggested
patch [1] and your earlier submitted patch [2].

[1] 
https://source.denx.de/u-boot/custodians/u-boot-usb/-/commit/028ad9536e501986f4e19d27f462f81a9ea70bad
   - Change 2nd argument type from "udevice *" to "ofnode"
     because glue_get_ctrl_dev() can give only "ofnode" structure from
     the device node.
   - Move soc-dependent stuffs to dwc3-uniphier.c.

[2] https://patchwork.ozlabs.org/project/uboot/patch/20221215223822.137739-1-marex@denx.de/
   - correspond to "for top level generic node with no subnode"

However, the uniphier usb3 glue node has more child nodes (not for
the controller), so I need to add more patches to enable more clocks
that the child nodes have.

I should send new patches as v2 series, though, how do you treat
your patches [1] and [2]?

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Marek Vasut]

On 1/30/23 06:52, Kunihiko Hayashi wrote:
> Hi Marek,

Hello Hayashi-san,

> Sorry for late reply.
> I was stuck in some pitfalls.

No worries, the MW closed today, but rc2 should be still OK to land 
these patches.

[...]

> I've successfully enabled the regular usb node using your suggested
> patch [1] and your earlier submitted patch [2].
> 
> [1] 
> https://source.denx.de/u-boot/custodians/u-boot-usb/-/commit/028ad9536e501986f4e19d27f462f81a9ea70bad
>    - Change 2nd argument type from "udevice *" to "ofnode"
>      because glue_get_ctrl_dev() can give only "ofnode" structure from
>      the device node.
>    - Move soc-dependent stuffs to dwc3-uniphier.c.

Please integrate this patch into your series, feel free to change it as 
needed too.

> [2] 
> https://patchwork.ozlabs.org/project/uboot/patch/20221215223822.137739-1-marex@denx.de/
>    - correspond to "for top level generic node with no subnode"

Could you provide AB/RB on this one if this looks OK? Then I would apply 
it to usb tree .

> However, the uniphier usb3 glue node has more child nodes (not for
> the controller), so I need to add more patches to enable more clocks
> that the child nodes have.
> 
> I should send new patches as v2 series, though, how do you treat
> your patches [1] and [2]?

Yes please, send V2. Regarding [1] and [2], see above.

Thank you

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Kunihiko Hayashi]

Hi Marek,

On 2023/01/31 7:50, Marek Vasut wrote:
> On 1/30/23 06:52, Kunihiko Hayashi wrote:
>> Hi Marek,
> 
> Hello Hayashi-san,
> 
>> Sorry for late reply.
>> I was stuck in some pitfalls.
> 
> No worries, the MW closed today, but rc2 should be still OK to land
> these patches.

Thank you for caring.

> [...]
> 
>> I've successfully enabled the regular usb node using your suggested
>> patch [1] and your earlier submitted patch [2].
>>
>> [1]
>> https://source.denx.de/u-boot/custodians/u-boot-usb/-/commit/028ad9536e501986f4e19d27f462f81a9ea70bad
>>     - Change 2nd argument type from "udevice *" to "ofnode"
>>       because glue_get_ctrl_dev() can give only "ofnode" structure from
>>       the device node.
>>     - Move soc-dependent stuffs to dwc3-uniphier.c.
> 
> Please integrate this patch into your series, feel free to change it as
> needed too.

OK, I'll make the commit log.
You're the original author of this patch, what about your tag?

>> [2]
>> https://patchwork.ozlabs.org/project/uboot/patch/20221215223822.137739-1-marex@denx.de/
>>     - correspond to "for top level generic node with no subnode"
> 
> Could you provide AB/RB on this one if this looks OK? Then I would apply
> it to usb tree .

This looks good to me. The dwc3-uniphier uses this for the reference clock in the
top level generic node. I'll provide my tag.

>> However, the uniphier usb3 glue node has more child nodes (not for
>> the controller), so I need to add more patches to enable more clocks
>> that the child nodes have.
>>
>> I should send new patches as v2 series, though, how do you treat
>> your patches [1] and [2]?
> 
> Yes please, send V2. Regarding [1] and [2], see above.

OK, I'll clean up and make the series soon.

Thank you,

---
Best Regards
Kunihiko Hayashi

Re: [PATCH 3/5] usb: dwc3-generic: Add dependency on SIMPLE_BUS

[Marek Vasut]

On 1/31/23 01:27, Kunihiko Hayashi wrote:
> Hi Marek,

Hello Hayashi-san,

> On 2023/01/31 7:50, Marek Vasut wrote:
>> On 1/30/23 06:52, Kunihiko Hayashi wrote:
>>> Hi Marek,
>>
>> Hello Hayashi-san,
>>
>>> Sorry for late reply.
>>> I was stuck in some pitfalls.
>>
>> No worries, the MW closed today, but rc2 should be still OK to land
>> these patches.
> 
> Thank you for caring.
> 
>> [...]
>>
>>> I've successfully enabled the regular usb node using your suggested
>>> patch [1] and your earlier submitted patch [2].
>>>
>>> [1]
>>> https://source.denx.de/u-boot/custodians/u-boot-usb/-/commit/028ad9536e501986f4e19d27f462f81a9ea70bad
>>>     - Change 2nd argument type from "udevice *" to "ofnode"
>>>       because glue_get_ctrl_dev() can give only "ofnode" structure from
>>>       the device node.
>>>     - Move soc-dependent stuffs to dwc3-uniphier.c.
>>
>> Please integrate this patch into your series, feel free to change it as
>> needed too.
> 
> OK, I'll make the commit log.
> You're the original author of this patch, what about your tag?

Since the patch needs heavy changes anyway, maybe some 'Suggested-by: 
Marek' would be best, or no tag at all ?

>>> [2]
>>> https://patchwork.ozlabs.org/project/uboot/patch/20221215223822.137739-1-marex@denx.de/
>>>     - correspond to "for top level generic node with no subnode"
>>
>> Could you provide AB/RB on this one if this looks OK? Then I would apply
>> it to usb tree .
> 
> This looks good to me. The dwc3-uniphier uses this for the reference 
> clock in the
> top level generic node. I'll provide my tag.

Thank you

>>> However, the uniphier usb3 glue node has more child nodes (not for
>>> the controller), so I need to add more patches to enable more clocks
>>> that the child nodes have.
>>>
>>> I should send new patches as v2 series, though, how do you treat
>>> your patches [1] and [2]?
>>
>> Yes please, send V2. Regarding [1] and [2], see above.
> 
> OK, I'll clean up and make the series soon.

Thank you