[PATCH 0/7] usb: fotg210: Various improvements

[PATCH 0/7] usb: fotg210: Various improvements

[Linus Walleij]

This is some gradual improvements to the FOTG210 dual-mode
USB host/gadget driver.

- Create stricter device tree bindings fixing some mistakes.
  We have at least FOTG200 and FOTG210 out there. A third
  variant named FUSB220 is probably related but has a separate
  driver in the vendor code trees.

- Start to break out common code for remapping of memory and
  clock handling to begin with, move this to the core file.

- Check that the block is in the right role before
  proceeding, don't abort registration but print a warning
  if it's not.

- Assing some default speed and the OF node when the UDC
  starts. This is based on the similar pattern in the DW
  controllers.

- Implement a proper VBUS session handler calling down to
  the hardware VBUS control handler in the Gemini variant.
  This makes everything work on my hardware.

To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: linux-usb@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Fabian Vogt <fabian@ritter-vogt.de>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
Linus Walleij (7):
      dt-bindings: usb: Correct and extend FOTG210 schema
      usb: fotg210: List different variants
      usb: fotg210: Acquire memory resource in core
      usb: fotg210: Move clock handling to core
      usb: fotg210: Check role register in core
      usb: fotg210-udc: Assign of_node and speed on start
      usb: fotg210-udc: Implement VBUS session

 .../devicetree/bindings/usb/faraday,fotg210.yaml   |  7 +-
 drivers/usb/fotg210/fotg210-core.c                 | 81 +++++++++++++++++++---
 drivers/usb/fotg210/fotg210-hcd.c                  | 48 ++-----------
 drivers/usb/fotg210/fotg210-hcd.h                  |  1 +
 drivers/usb/fotg210/fotg210-udc.c                  | 68 ++++++++----------
 drivers/usb/fotg210/fotg210-udc.h                  |  2 +-
 drivers/usb/fotg210/fotg210.h                      | 27 ++++++--
 7 files changed, 136 insertions(+), 98 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230103-gemini-fotg210-usb-95a76798706a

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH 1/7] dt-bindings: usb: Correct and extend FOTG210 schema

[Linus Walleij]

It turns out that this IP block exists in at least two
incarnations: FOTG200 and FOTG210. The one in the Gemini
is FOTG200, so add the variants and rectify the binding
for Gemini.

This affects things such as the placement of certain
registers.

It remains to be seen how similar this block is to the
third USB block from Faraday, FUSB220.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 Documentation/devicetree/bindings/usb/faraday,fotg210.yaml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml b/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml
index 84b3b69256b1..12d4fc742f20 100644
--- a/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml
+++ b/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml
@@ -5,7 +5,7 @@
 $id: http://devicetree.org/schemas/usb/faraday,fotg210.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Faraday Technology FOTG210 HS OTG USB 2.0 controller
+title: Faraday Technology FOTG200 series HS OTG USB 2.0 controller Bindings
 
 maintainers:
   - Linus Walleij <linus.walleij@linaro.org>
@@ -17,10 +17,11 @@ allOf:
 properties:
   compatible:
     oneOf:
+      - const: faraday,fotg200
       - const: faraday,fotg210
       - items:
           - const: cortina,gemini-usb
-          - const: faraday,fotg210
+          - const: faraday,fotg200
 
   reg:
     maxItems: 1
@@ -66,7 +67,7 @@ examples:
     #include <dt-bindings/clock/cortina,gemini-clock.h>
     #include <dt-bindings/reset/cortina,gemini-reset.h>
     usb0: usb@68000000 {
-        compatible = "cortina,gemini-usb", "faraday,fotg210";
+        compatible = "cortina,gemini-usb", "faraday,fotg200";
         reg = <0x68000000 0x1000>;
         interrupts = <10 IRQ_TYPE_LEVEL_HIGH>;
         resets = <&syscon GEMINI_RESET_USB0>;

-- 
2.38.1

Re: [PATCH 1/7] dt-bindings: usb: Correct and extend FOTG210 schema

[Krzysztof Kozlowski]

On 04/01/2023 00:35, Linus Walleij wrote:
> It turns out that this IP block exists in at least two
> incarnations: FOTG200 and FOTG210. The one in the Gemini
> is FOTG200, so add the variants and rectify the binding
> for Gemini.
> 
> This affects things such as the placement of certain
> registers.
> 
> It remains to be seen how similar this block is to the
> third USB block from Faraday, FUSB220.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  Documentation/devicetree/bindings/usb/faraday,fotg210.yaml | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml b/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml
> index 84b3b69256b1..12d4fc742f20 100644
> --- a/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml
> +++ b/Documentation/devicetree/bindings/usb/faraday,fotg210.yaml
> @@ -5,7 +5,7 @@
>  $id: http://devicetree.org/schemas/usb/faraday,fotg210.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Faraday Technology FOTG210 HS OTG USB 2.0 controller
> +title: Faraday Technology FOTG200 series HS OTG USB 2.0 controller Bindings

That's not correct change, probably due to rebasing. Change the name
only, do not add "Bindings".


Best regards,
Krzysztof

[PATCH 2/7] usb: fotg210: List different variants

[Linus Walleij]

There are at least two variants of the FOTG: FOTG200 and
FOTG210. Handle them in this driver and let's add
more quirks as we go along.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/usb/fotg210/fotg210-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index 8a54edf921ac..cef12827e797 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -127,7 +127,9 @@ static int fotg210_remove(struct platform_device *pdev)
 
 #ifdef CONFIG_OF
 static const struct of_device_id fotg210_of_match[] = {
+	{ .compatible = "faraday,fotg200" },
 	{ .compatible = "faraday,fotg210" },
+	/* TODO: can we also handle FUSB220? */
 	{},
 };
 MODULE_DEVICE_TABLE(of, fotg210_of_match);

-- 
2.38.1

[PATCH 3/7] usb: fotg210: Acquire memory resource in core

[Linus Walleij]

The subdrivers are obtaining and mapping the memory resource
separately. Create a common state container for the shared
resources and start populating this by acquiring the IO
memory resource and remap it and pass this to the subdrivers
for host and peripheral.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/usb/fotg210/fotg210-core.c | 29 ++++++++++++++++++++++-------
 drivers/usb/fotg210/fotg210-hcd.c  | 15 +++++----------
 drivers/usb/fotg210/fotg210-hcd.h  |  1 +
 drivers/usb/fotg210/fotg210-udc.c  | 16 +++-------------
 drivers/usb/fotg210/fotg210-udc.h  |  1 +
 drivers/usb/fotg210/fotg210.h      | 24 ++++++++++++++++++++----
 6 files changed, 52 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index cef12827e797..4593120c02de 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -33,9 +33,10 @@
 #define GEMINI_MISC_USB0_MINI_B		BIT(29)
 #define GEMINI_MISC_USB1_MINI_B		BIT(30)
 
-static int fotg210_gemini_init(struct device *dev, struct resource *res,
+static int fotg210_gemini_init(struct fotg210 *fotg, struct resource *res,
 			       enum usb_dr_mode mode)
 {
+	struct device *dev = fotg->dev;
 	struct device_node *np = dev->of_node;
 	struct regmap *map;
 	bool wakeup;
@@ -47,6 +48,7 @@ static int fotg210_gemini_init(struct device *dev, struct resource *res,
 		dev_err(dev, "no syscon\n");
 		return PTR_ERR(map);
 	}
+	fotg->map = map;
 	wakeup = of_property_read_bool(np, "wakeup-source");
 
 	/*
@@ -55,6 +57,7 @@ static int fotg210_gemini_init(struct device *dev, struct resource *res,
 	 */
 	mask = 0;
 	if (res->start == 0x69000000) {
+		fotg->port = GEMINI_PORT_1;
 		mask = GEMINI_MISC_USB1_VBUS_ON | GEMINI_MISC_USB1_MINI_B |
 			GEMINI_MISC_USB1_WAKEUP;
 		if (mode == USB_DR_MODE_HOST)
@@ -64,6 +67,7 @@ static int fotg210_gemini_init(struct device *dev, struct resource *res,
 		if (wakeup)
 			val |= GEMINI_MISC_USB1_WAKEUP;
 	} else {
+		fotg->port = GEMINI_PORT_0;
 		mask = GEMINI_MISC_USB0_VBUS_ON | GEMINI_MISC_USB0_MINI_B |
 			GEMINI_MISC_USB0_WAKEUP;
 		if (mode == USB_DR_MODE_HOST)
@@ -89,23 +93,34 @@ static int fotg210_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	enum usb_dr_mode mode;
+	struct fotg210 *fotg;
 	int ret;
 
+	fotg = devm_kzalloc(dev, sizeof(*fotg), GFP_KERNEL);
+	if (!fotg)
+		return -ENOMEM;
+	fotg->dev = dev;
+
+	fotg->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!fotg->res)
+		return -ENODEV;
+
+	fotg->base = devm_ioremap_resource(dev, fotg->res);
+	if (!fotg->base)
+		return -ENOMEM;
+
 	mode = usb_get_dr_mode(dev);
 
 	if (of_device_is_compatible(dev->of_node, "cortina,gemini-usb")) {
-		struct resource *res;
-
-		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-		ret = fotg210_gemini_init(dev, res, mode);
+		ret = fotg210_gemini_init(fotg, fotg->res, mode);
 		if (ret)
 			return ret;
 	}
 
 	if (mode == USB_DR_MODE_PERIPHERAL)
-		ret = fotg210_udc_probe(pdev);
+		ret = fotg210_udc_probe(pdev, fotg);
 	else
-		ret = fotg210_hcd_probe(pdev);
+		ret = fotg210_hcd_probe(pdev, fotg);
 
 	return ret;
 }
diff --git a/drivers/usb/fotg210/fotg210-hcd.c b/drivers/usb/fotg210/fotg210-hcd.c
index 51ac93a2eb98..15ba5b1618e1 100644
--- a/drivers/usb/fotg210/fotg210-hcd.c
+++ b/drivers/usb/fotg210/fotg210-hcd.c
@@ -5557,11 +5557,10 @@ static void fotg210_init(struct fotg210_hcd *fotg210)
  * then invokes the start() method for the HCD associated with it
  * through the hotplug entry's driver_data.
  */
-int fotg210_hcd_probe(struct platform_device *pdev)
+int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
 {
 	struct device *dev = &pdev->dev;
 	struct usb_hcd *hcd;
-	struct resource *res;
 	int irq;
 	int retval;
 	struct fotg210_hcd *fotg210;
@@ -5585,18 +5584,14 @@ int fotg210_hcd_probe(struct platform_device *pdev)
 
 	hcd->has_tt = 1;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	hcd->regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(hcd->regs)) {
-		retval = PTR_ERR(hcd->regs);
-		goto failed_put_hcd;
-	}
+	hcd->regs = fotg->base;
 
-	hcd->rsrc_start = res->start;
-	hcd->rsrc_len = resource_size(res);
+	hcd->rsrc_start = fotg->res->start;
+	hcd->rsrc_len = resource_size(fotg->res);
 
 	fotg210 = hcd_to_fotg210(hcd);
 
+	fotg210->fotg = fotg;
 	fotg210->caps = hcd->regs;
 
 	/* It's OK not to supply this clock */
diff --git a/drivers/usb/fotg210/fotg210-hcd.h b/drivers/usb/fotg210/fotg210-hcd.h
index 0781442b7a24..13c9342982ee 100644
--- a/drivers/usb/fotg210/fotg210-hcd.h
+++ b/drivers/usb/fotg210/fotg210-hcd.h
@@ -182,6 +182,7 @@ struct fotg210_hcd {			/* one per controller */
 #	define INCR(x) do {} while (0)
 #endif
 
+	struct fotg210		*fotg;		/* Overarching FOTG210 device */
 	/* silicon clock */
 	struct clk		*pclk;
 };
diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c
index 66e1b7ee3346..034193592a36 100644
--- a/drivers/usb/fotg210/fotg210-udc.c
+++ b/drivers/usb/fotg210/fotg210-udc.c
@@ -1142,21 +1142,14 @@ int fotg210_udc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-int fotg210_udc_probe(struct platform_device *pdev)
+int fotg210_udc_probe(struct platform_device *pdev, struct fotg210 *fotg)
 {
-	struct resource *res;
 	struct fotg210_udc *fotg210 = NULL;
 	struct device *dev = &pdev->dev;
 	int irq;
 	int ret = 0;
 	int i;
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		pr_err("platform_get_resource error.\n");
-		return -ENODEV;
-	}
-
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		pr_err("could not get irq\n");
@@ -1169,6 +1162,7 @@ int fotg210_udc_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	fotg210->dev = dev;
+	fotg210->fotg = fotg;
 
 	/* It's OK not to supply this clock */
 	fotg210->pclk = devm_clk_get(dev, "PCLK");
@@ -1207,11 +1201,7 @@ int fotg210_udc_probe(struct platform_device *pdev)
 			goto err_alloc;
 	}
 
-	fotg210->reg = ioremap(res->start, resource_size(res));
-	if (fotg210->reg == NULL) {
-		dev_err(dev, "ioremap error\n");
-		goto err_alloc;
-	}
+	fotg210->reg = fotg->base;
 
 	spin_lock_init(&fotg210->lock);
 
diff --git a/drivers/usb/fotg210/fotg210-udc.h b/drivers/usb/fotg210/fotg210-udc.h
index fadb57ca8d78..20335a38a410 100644
--- a/drivers/usb/fotg210/fotg210-udc.h
+++ b/drivers/usb/fotg210/fotg210-udc.h
@@ -236,6 +236,7 @@ struct fotg210_udc {
 	unsigned long		irq_trigger;
 
 	struct device			*dev;
+	struct fotg210			*fotg;
 	struct usb_phy			*phy;
 	struct usb_gadget		gadget;
 	struct usb_gadget_driver	*driver;
diff --git a/drivers/usb/fotg210/fotg210.h b/drivers/usb/fotg210/fotg210.h
index ef79d8323d89..50436cc16538 100644
--- a/drivers/usb/fotg210/fotg210.h
+++ b/drivers/usb/fotg210/fotg210.h
@@ -2,13 +2,28 @@
 #ifndef __FOTG210_H
 #define __FOTG210_H
 
+enum gemini_port {
+	GEMINI_PORT_NONE = 0,
+	GEMINI_PORT_0,
+	GEMINI_PORT_1,
+};
+
+struct fotg210 {
+	struct device *dev;
+	struct resource *res;
+	void __iomem *base;
+	struct regmap *map;
+	enum gemini_port port;
+};
+
 #ifdef CONFIG_USB_FOTG210_HCD
-int fotg210_hcd_probe(struct platform_device *pdev);
+int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg);
 int fotg210_hcd_remove(struct platform_device *pdev);
 int fotg210_hcd_init(void);
 void fotg210_hcd_cleanup(void);
 #else
-static inline int fotg210_hcd_probe(struct platform_device *pdev)
+static inline int fotg210_hcd_probe(struct platform_device *pdev,
+				    struct fotg210 *fotg)
 {
 	return 0;
 }
@@ -26,10 +41,11 @@ static inline void fotg210_hcd_cleanup(void)
 #endif
 
 #ifdef CONFIG_USB_FOTG210_UDC
-int fotg210_udc_probe(struct platform_device *pdev);
+int fotg210_udc_probe(struct platform_device *pdev, struct fotg210 *fotg);
 int fotg210_udc_remove(struct platform_device *pdev);
 #else
-static inline int fotg210_udc_probe(struct platform_device *pdev)
+static inline int fotg210_udc_probe(struct platform_device *pdev,
+				    struct fotg210 *fotg)
 {
 	return 0;
 }

-- 
2.38.1

[PATCH 4/7] usb: fotg210: Move clock handling to core

[Linus Walleij]

Grab the optional silicon block clock, prepare and enable it in
the core before proceeding to prepare the host or peripheral
driver. This saves duplicate code and also uses the simple
devm_clk_get_optional_enabled() to do everything we really
want to do.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/usb/fotg210/fotg210-core.c |  5 +++++
 drivers/usb/fotg210/fotg210-hcd.c  | 33 ++-------------------------------
 drivers/usb/fotg210/fotg210-udc.c  | 30 +++---------------------------
 drivers/usb/fotg210/fotg210-udc.h  |  1 -
 drivers/usb/fotg210/fotg210.h      |  1 +
 5 files changed, 11 insertions(+), 59 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index 4593120c02de..58d84747fb30 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -6,6 +6,7 @@
  * driver.
  */
 #include <linux/bitops.h>
+#include <linux/clk.h>
 #include <linux/device.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
@@ -109,6 +110,10 @@ static int fotg210_probe(struct platform_device *pdev)
 	if (!fotg->base)
 		return -ENOMEM;
 
+	fotg->pclk = devm_clk_get_optional_enabled(dev, "PCLK");
+	if (IS_ERR(fotg->pclk))
+		return PTR_ERR(fotg->pclk);
+
 	mode = usb_get_dr_mode(dev);
 
 	if (of_device_is_compatible(dev->of_node, "cortina,gemini-usb")) {
diff --git a/drivers/usb/fotg210/fotg210-hcd.c b/drivers/usb/fotg210/fotg210-hcd.c
index 15ba5b1618e1..7bd1e8f3080d 100644
--- a/drivers/usb/fotg210/fotg210-hcd.c
+++ b/drivers/usb/fotg210/fotg210-hcd.c
@@ -33,7 +33,6 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
-#include <linux/clk.h>
 
 #include <asm/byteorder.h>
 #include <asm/irq.h>
@@ -5594,44 +5593,22 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
 	fotg210->fotg = fotg;
 	fotg210->caps = hcd->regs;
 
-	/* It's OK not to supply this clock */
-	fotg210->pclk = clk_get(dev, "PCLK");
-	if (!IS_ERR(fotg210->pclk)) {
-		retval = clk_prepare_enable(fotg210->pclk);
-		if (retval) {
-			dev_err(dev, "failed to enable PCLK\n");
-			goto failed_put_hcd;
-		}
-	} else if (PTR_ERR(fotg210->pclk) == -EPROBE_DEFER) {
-		/*
-		 * Percolate deferrals, for anything else,
-		 * just live without the clocking.
-		 */
-		retval = PTR_ERR(fotg210->pclk);
-		goto failed_dis_clk;
-	}
-
 	retval = fotg210_setup(hcd);
 	if (retval)
-		goto failed_dis_clk;
+		goto failed_put_hcd;
 
 	fotg210_init(fotg210);
 
 	retval = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (retval) {
 		dev_err(dev, "failed to add hcd with err %d\n", retval);
-		goto failed_dis_clk;
+		goto failed_put_hcd;
 	}
 	device_wakeup_enable(hcd->self.controller);
 	platform_set_drvdata(pdev, hcd);
 
 	return retval;
 
-failed_dis_clk:
-	if (!IS_ERR(fotg210->pclk)) {
-		clk_disable_unprepare(fotg210->pclk);
-		clk_put(fotg210->pclk);
-	}
 failed_put_hcd:
 	usb_put_hcd(hcd);
 fail_create_hcd:
@@ -5647,12 +5624,6 @@ int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg)
 int fotg210_hcd_remove(struct platform_device *pdev)
 {
 	struct usb_hcd *hcd = platform_get_drvdata(pdev);
-	struct fotg210_hcd *fotg210 = hcd_to_fotg210(hcd);
-
-	if (!IS_ERR(fotg210->pclk)) {
-		clk_disable_unprepare(fotg210->pclk);
-		clk_put(fotg210->pclk);
-	}
 
 	usb_remove_hcd(hcd);
 	usb_put_hcd(hcd);
diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c
index 034193592a36..6a4b94d26951 100644
--- a/drivers/usb/fotg210/fotg210-udc.c
+++ b/drivers/usb/fotg210/fotg210-udc.c
@@ -15,7 +15,6 @@
 #include <linux/platform_device.h>
 #include <linux/usb/ch9.h>
 #include <linux/usb/gadget.h>
-#include <linux/clk.h>
 #include <linux/usb/otg.h>
 #include <linux/usb/phy.h>
 
@@ -1134,9 +1133,6 @@ int fotg210_udc_remove(struct platform_device *pdev)
 	for (i = 0; i < FOTG210_MAX_NUM_EP; i++)
 		kfree(fotg210->ep[i]);
 
-	if (!IS_ERR(fotg210->pclk))
-		clk_disable_unprepare(fotg210->pclk);
-
 	kfree(fotg210);
 
 	return 0;
@@ -1164,34 +1160,17 @@ int fotg210_udc_probe(struct platform_device *pdev, struct fotg210 *fotg)
 	fotg210->dev = dev;
 	fotg210->fotg = fotg;
 
-	/* It's OK not to supply this clock */
-	fotg210->pclk = devm_clk_get(dev, "PCLK");
-	if (!IS_ERR(fotg210->pclk)) {
-		ret = clk_prepare_enable(fotg210->pclk);
-		if (ret) {
-			dev_err(dev, "failed to enable PCLK\n");
-			goto err;
-		}
-	} else if (PTR_ERR(fotg210->pclk) == -EPROBE_DEFER) {
-		/*
-		 * Percolate deferrals, for anything else,
-		 * just live without the clocking.
-		 */
-		ret = -EPROBE_DEFER;
-		goto err;
-	}
-
 	fotg210->phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
 	if (IS_ERR(fotg210->phy)) {
 		ret = PTR_ERR(fotg210->phy);
 		if (ret == -EPROBE_DEFER)
-			goto err_pclk;
+			goto err_free;
 		dev_info(dev, "no PHY found\n");
 		fotg210->phy = NULL;
 	} else {
 		ret = usb_phy_init(fotg210->phy);
 		if (ret)
-			goto err_pclk;
+			goto err_free;
 		dev_info(dev, "found and initialized PHY\n");
 	}
 
@@ -1286,11 +1265,8 @@ int fotg210_udc_probe(struct platform_device *pdev, struct fotg210 *fotg)
 err_alloc:
 	for (i = 0; i < FOTG210_MAX_NUM_EP; i++)
 		kfree(fotg210->ep[i]);
-err_pclk:
-	if (!IS_ERR(fotg210->pclk))
-		clk_disable_unprepare(fotg210->pclk);
 
-err:
+err_free:
 	kfree(fotg210);
 	return ret;
 }
diff --git a/drivers/usb/fotg210/fotg210-udc.h b/drivers/usb/fotg210/fotg210-udc.h
index 20335a38a410..22b72caf498c 100644
--- a/drivers/usb/fotg210/fotg210-udc.h
+++ b/drivers/usb/fotg210/fotg210-udc.h
@@ -231,7 +231,6 @@ struct fotg210_ep {
 struct fotg210_udc {
 	spinlock_t		lock; /* protect the struct */
 	void __iomem		*reg;
-	struct clk		*pclk;
 
 	unsigned long		irq_trigger;
 
diff --git a/drivers/usb/fotg210/fotg210.h b/drivers/usb/fotg210/fotg210.h
index 50436cc16538..4d0d4ae1a957 100644
--- a/drivers/usb/fotg210/fotg210.h
+++ b/drivers/usb/fotg210/fotg210.h
@@ -12,6 +12,7 @@ struct fotg210 {
 	struct device *dev;
 	struct resource *res;
 	void __iomem *base;
+	struct clk *pclk;
 	struct regmap *map;
 	enum gemini_port port;
 };

-- 
2.38.1

[PATCH 5/7] usb: fotg210: Check role register in core

[Linus Walleij]

Read the role register and check that we are in host/peripheral
mode and issue warnings if we're not in the right role when
probing respective driver.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/usb/fotg210/fotg210-core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index 58d84747fb30..013a1d0112fc 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -18,6 +18,11 @@
 
 #include "fotg210.h"
 
+/* Role Register 0x80 */
+#define FOTG210_RR			0x80
+#define FOTG210_RR_ID			BIT(21) /* 1 = B-device, 0 = A-device */
+#define FOTG210_RR_CROLE		BIT(20) /* 1 = device, 0 = host */
+
 /*
  * Gemini-specific initialization function, only executed on the
  * Gemini SoC using the global misc control register.
@@ -95,6 +100,7 @@ static int fotg210_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	enum usb_dr_mode mode;
 	struct fotg210 *fotg;
+	u32 val;
 	int ret;
 
 	fotg = devm_kzalloc(dev, sizeof(*fotg), GFP_KERNEL);
@@ -122,10 +128,16 @@ static int fotg210_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	if (mode == USB_DR_MODE_PERIPHERAL)
+	val = readl(fotg->base + FOTG210_RR);
+	if (mode == USB_DR_MODE_PERIPHERAL) {
+		if (!(val & FOTG210_RR_CROLE))
+			dev_err(dev, "block not in device role\n");
 		ret = fotg210_udc_probe(pdev, fotg);
-	else
+	} else {
+		if (val & FOTG210_RR_CROLE)
+			dev_err(dev, "block not in host role\n");
 		ret = fotg210_hcd_probe(pdev, fotg);
+	}
 
 	return ret;
 }

-- 
2.38.1

[PATCH 6/7] usb: fotg210-udc: Assign of_node and speed on start

[Linus Walleij]

Follow the example set by other drivers to assign of_node
and speed to the driver when binding, also print bound
info akin to other UDC drivers.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/usb/fotg210/fotg210-udc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c
index 6a4b94d26951..4099e7453112 100644
--- a/drivers/usb/fotg210/fotg210-udc.c
+++ b/drivers/usb/fotg210/fotg210-udc.c
@@ -1015,6 +1015,10 @@ static int fotg210_udc_start(struct usb_gadget *g,
 	/* hook up the driver */
 	driver->driver.bus = NULL;
 	fotg210->driver = driver;
+	fotg210->gadget.dev.of_node = fotg210->dev->of_node;
+	fotg210->gadget.speed = USB_SPEED_UNKNOWN;
+
+	dev_info(fotg210->dev, "bound driver %s\n", driver->driver.name);
 
 	if (!IS_ERR_OR_NULL(fotg210->phy)) {
 		ret = otg_set_peripheral(fotg210->phy->otg,
@@ -1071,6 +1075,7 @@ static int fotg210_udc_stop(struct usb_gadget *g)
 
 	fotg210_init(fotg210);
 	fotg210->driver = NULL;
+	fotg210->gadget.speed = USB_SPEED_UNKNOWN;
 
 	spin_unlock_irqrestore(&fotg210->lock, flags);
 

-- 
2.38.1

[PATCH 7/7] usb: fotg210-udc: Implement VBUS session

[Linus Walleij]

Implement VBUS session handling for FOTG210. This is
mainly used by the UDC driver which needs to call down to
the FOTG210 core and enable/disable VBUS, as this needs to be
handled outside of the HCD and UDC drivers, by platform
specific glue code.

The Gemini has a special bit in a system register to turn
VBUS on and off so we implement this in the FOTG210 core.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/usb/fotg210/fotg210-core.c | 29 +++++++++++++++++++++++++++++
 drivers/usb/fotg210/fotg210-udc.c  | 17 +++++++++++++++++
 drivers/usb/fotg210/fotg210.h      |  2 ++
 3 files changed, 48 insertions(+)

diff --git a/drivers/usb/fotg210/fotg210-core.c b/drivers/usb/fotg210/fotg210-core.c
index 013a1d0112fc..70d2ff5a2682 100644
--- a/drivers/usb/fotg210/fotg210-core.c
+++ b/drivers/usb/fotg210/fotg210-core.c
@@ -95,6 +95,35 @@ static int fotg210_gemini_init(struct fotg210 *fotg, struct resource *res,
 	return 0;
 }
 
+/**
+ * fotg210_vbus() - Called by gadget driver to enable/disable VBUS
+ * @enable: true to enable VBUS, false to disable VBUS
+ */
+void fotg210_vbus(struct fotg210 *fotg, bool enable)
+{
+	u32 mask;
+	u32 val;
+	int ret;
+
+	switch (fotg->port) {
+	case GEMINI_PORT_0:
+		mask = GEMINI_MISC_USB0_VBUS_ON;
+		val = enable ? GEMINI_MISC_USB0_VBUS_ON : 0;
+		break;
+	case GEMINI_PORT_1:
+		mask = GEMINI_MISC_USB1_VBUS_ON;
+		val = enable ? GEMINI_MISC_USB1_VBUS_ON : 0;
+		break;
+	default:
+		return;
+	}
+	ret = regmap_update_bits(fotg->map, GEMINI_GLOBAL_MISC_CTRL, mask, val);
+	if (ret)
+		dev_err(fotg->dev, "failed to %s VBUS\n",
+			enable ? "enable" : "disable");
+	dev_info(fotg->dev, "%s: %s VBUS\n", __func__, enable ? "enable" : "disable");
+}
+
 static int fotg210_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
diff --git a/drivers/usb/fotg210/fotg210-udc.c b/drivers/usb/fotg210/fotg210-udc.c
index 4099e7453112..754429553f61 100644
--- a/drivers/usb/fotg210/fotg210-udc.c
+++ b/drivers/usb/fotg210/fotg210-udc.c
@@ -1082,9 +1082,26 @@ static int fotg210_udc_stop(struct usb_gadget *g)
 	return 0;
 }
 
+/**
+ * fotg210_vbus_session - Called by external transceiver to enable/disable udc
+ * @_gadget: usb gadget
+ * @is_active: 0 if should disable UDC VBUS, 1 if should enable
+ *
+ * Returns 0
+ */
+static int fotg210_vbus_session(struct usb_gadget *g, int is_active)
+{
+	struct fotg210_udc *fotg210 = gadget_to_fotg210(g);
+
+	/* Call down to core integration layer to drive or disable VBUS */
+	fotg210_vbus(fotg210->fotg, is_active);
+	return 0;
+}
+
 static const struct usb_gadget_ops fotg210_gadget_ops = {
 	.udc_start		= fotg210_udc_start,
 	.udc_stop		= fotg210_udc_stop,
+	.vbus_session		= fotg210_vbus_session,
 };
 
 /**
diff --git a/drivers/usb/fotg210/fotg210.h b/drivers/usb/fotg210/fotg210.h
index 4d0d4ae1a957..c44c0afe2956 100644
--- a/drivers/usb/fotg210/fotg210.h
+++ b/drivers/usb/fotg210/fotg210.h
@@ -17,6 +17,8 @@ struct fotg210 {
 	enum gemini_port port;
 };
 
+void fotg210_vbus(struct fotg210 *fotg, bool enable);
+
 #ifdef CONFIG_USB_FOTG210_HCD
 int fotg210_hcd_probe(struct platform_device *pdev, struct fotg210 *fotg);
 int fotg210_hcd_remove(struct platform_device *pdev);

-- 
2.38.1

Re: [PATCH 0/7] usb: fotg210: Various improvements

[Linus Walleij]

On Wed, Jan 4, 2023 at 12:35 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> This is some gradual improvements to the FOTG210 dual-mode
> USB host/gadget driver.

Been two weeks, something I should change or are these good to
go?

Yours,
Linus Walleij

Re: [PATCH 0/7] usb: fotg210: Various improvements

[Linus Walleij]

On Wed, Jan 18, 2023 at 8:03 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jan 4, 2023 at 12:35 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > This is some gradual improvements to the FOTG210 dual-mode
> > USB host/gadget driver.
>
> Been two weeks, something I should change or are these good to
> go?

Ah no wait I see it, a small change needed on the DT binding.
I'll respin.

Yours,
Linus Walleij

Re: [PATCH 0/7] usb: fotg210: Various improvements

[Greg Kroah-Hartman]

On Wed, Jan 18, 2023 at 08:03:59AM +0100, Linus Walleij wrote:
> On Wed, Jan 4, 2023 at 12:35 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> > This is some gradual improvements to the FOTG210 dual-mode
> > USB host/gadget driver.
> 
> Been two weeks, something I should change or are these good to
> go?

I dropped the series from my review queue due to this comment:
	https://lore.kernel.org/r/9bbd5343-30bc-1146-3296-2c3a43b9a91b@linaro.org

so I was expecting a new series, is that not the case?

thanks,

greg k-h

Re: [PATCH 0/7] usb: fotg210: Various improvements

[Linus Walleij]

On Wed, Jan 18, 2023 at 8:10 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Jan 18, 2023 at 08:03:59AM +0100, Linus Walleij wrote:
> > On Wed, Jan 4, 2023 at 12:35 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> > > This is some gradual improvements to the FOTG210 dual-mode
> > > USB host/gadget driver.
> >
> > Been two weeks, something I should change or are these good to
> > go?
>
> I dropped the series from my review queue due to this comment:
>         https://lore.kernel.org/r/9bbd5343-30bc-1146-3296-2c3a43b9a91b@linaro.org
>
> so I was expecting a new series, is that not the case?

You're right, I've dropped a single word and resent!

Yours,
Linus Walleij