[PATCH v6 0/3] usb: musb: sunxi: add F1C100s MUSB support

[PATCH v6 0/3] usb: musb: sunxi: add F1C100s MUSB support

[Andre Przywara]

Hi,

this is a split off from the combined USB/DT Allwinner F1C100s support
series[1], with just the (MUSB) USB patches.
There were no code changes in v5 or v6, those are just rebases and
resends.
============================

This patchset introduces support for the F1C100s USB controller, which
just consists of a MUSB controller variant and some USB PHY. The USB PHY
support bits are already in -next.
Patch 1 adds the compatible string to the binding, patch 2 introduces
that compatible string to the driver, and lets it pick the appropriate
flags.
As this came up in reviews, patch 3 cleans up the SoC specific
configuration bits in the driver, to make future additions easier.

Please have a look and apply, if possible.

Cheers,
Andre.

[1] https://lore.kernel.org/linux-arm-kernel/20230111015332.172021-1-andre.przywara@arm.com/

Changelog v5 ... v6:
- just keep MUSB driver and binding patches
- add Jernej's Reviewed-by:
- Rebase on top of gregkh/usb/usb-next

Changelog v4 ... v5:
- Rebase on top to v6.2-rc1

Changelog v3 ... v4:
- Dropped the PHY patches, they go via a different tree and need a
  different base
- rebased on top of linux-sunxi/sunxi/for-next (provides H616 USB)
- musb DT binding: use enum
- musb cleanup: use musb_hdrc_config config pointer directly
- musb cleanup: use const where possible
- drop partitions from Popstick DTS file
- clarify Popstick has a USB type-A *plug*
- add tags

Changelog v2 ... v3:
- remove redundant "Device Tree Bindings" suffix in DT binding doc title
- add BSD license to binding doc file (as per checkpatch)
- fix some commit message title prefixes
- use proper plural spelling for usb0_id_det-gpios
- popstick.dts: Reorder otg_sram node reference alphabetically
- popstick.dts: Add regulator- prefix to 3.3V regulator node name
- popstick.dts: Fix status, compatible and reg property order
- popstick.dts: Drop unneeded mmc0 and spi0 aliases
- add patch to clean up sunxi MUSB driver
- add Acks and Reviewed-by's

Changelog v1 ... v2:
- USB PHY binding: clarify the relation with other phy-sun4i-usb bindings
- Add Popstick binding and .dts patches


Andre Przywara (1):
  usb: musb: sunxi: Introduce config struct

Icenowy Zheng (2):
  dt-bindings: usb: sunxi-musb: add F1C100s MUSB compatible string
  usb: musb: sunxi: add support for the F1C100s MUSB controller

 .../usb/allwinner,sun4i-a10-musb.yaml         | 10 +-
 drivers/usb/musb/sunxi.c                      | 99 +++++++++++++------
 2 files changed, 76 insertions(+), 33 deletions(-)

-- 
2.25.1

[PATCH v6 1/3] dt-bindings: usb: sunxi-musb: add F1C100s MUSB compatible string

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

Allwinner F1C100s has a hybrid MUSB controller between the A10 one and
the A33 one.

Add a compatible string for it.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Samuel Holland <samuel@sholland.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 .../bindings/usb/allwinner,sun4i-a10-musb.yaml         | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
index 8992eff6ce387..f972ce976e860 100644
--- a/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
+++ b/Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml
@@ -13,10 +13,12 @@ maintainers:
 properties:
   compatible:
     oneOf:
-      - const: allwinner,sun4i-a10-musb
-      - const: allwinner,sun6i-a31-musb
-      - const: allwinner,sun8i-a33-musb
-      - const: allwinner,sun8i-h3-musb
+      - enum:
+          - allwinner,sun4i-a10-musb
+          - allwinner,sun6i-a31-musb
+          - allwinner,sun8i-a33-musb
+          - allwinner,sun8i-h3-musb
+          - allwinner,suniv-f1c100s-musb
       - items:
           - enum:
               - allwinner,sun8i-a83t-musb
-- 
2.25.1

[PATCH v6 2/3] usb: musb: sunxi: add support for the F1C100s MUSB controller

[Andre Przywara]

From: Icenowy Zheng <uwu@icenowy.me>

The suniv SoC has a MUSB controller like the one in A33, but with a SRAM
region to be claimed.

Add support for it.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/usb/musb/sunxi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 7f9a999cd5ff1..4b368d16a73ad 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -722,14 +722,17 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 	INIT_WORK(&glue->work, sunxi_musb_work);
 	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
 
-	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb"))
+	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb") ||
+	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
 		set_bit(SUNXI_MUSB_FL_HAS_SRAM, &glue->flags);
+	}
 
 	if (of_device_is_compatible(np, "allwinner,sun6i-a31-musb"))
 		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
 
 	if (of_device_is_compatible(np, "allwinner,sun8i-a33-musb") ||
-	    of_device_is_compatible(np, "allwinner,sun8i-h3-musb")) {
+	    of_device_is_compatible(np, "allwinner,sun8i-h3-musb") ||
+	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
 		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
 		set_bit(SUNXI_MUSB_FL_NO_CONFIGDATA, &glue->flags);
 	}
@@ -815,6 +818,7 @@ static const struct of_device_id sunxi_musb_match[] = {
 	{ .compatible = "allwinner,sun6i-a31-musb", },
 	{ .compatible = "allwinner,sun8i-a33-musb", },
 	{ .compatible = "allwinner,sun8i-h3-musb", },
+	{ .compatible = "allwinner,suniv-f1c100s-musb", },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_musb_match);
-- 
2.25.1

[PATCH v6 3/3] usb: musb: sunxi: Introduce config struct

[Andre Przywara]

Currently the probe routine explicitly compares the compatible string of
the device node to figure out which features and quirks a certain
Allwinner MUSB model requires. This gets harder to maintain for new
SoCs.

Add a struct sunxi_musb_cfg that names the features and quirks
explicitly, and create instances of this struct for every type of MUSB
device we support. Then bind this to the compatible strings via the OF
data feature.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/usb/musb/sunxi.c | 103 ++++++++++++++++++++++++++-------------
 1 file changed, 70 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 4b368d16a73ad..9b622cd9b2bd5 100644
--- a/drivers/usb/musb/sunxi.c
+++ b/drivers/usb/musb/sunxi.c
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/phy/phy-sun4i-usb.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
@@ -67,6 +68,13 @@
 #define SUNXI_MUSB_FL_NO_CONFIGDATA		7
 #define SUNXI_MUSB_FL_PHY_MODE_PEND		8
 
+struct sunxi_musb_cfg {
+	const struct musb_hdrc_config *hdrc_config;
+	bool has_sram;
+	bool has_reset;
+	bool no_configdata;
+};
+
 /* Our read/write methods need access and do not get passed in a musb ref :| */
 static struct musb *sunxi_musb;
 
@@ -621,11 +629,10 @@ static const struct musb_platform_ops sunxi_musb_ops = {
 	.post_root_reset_end = sunxi_musb_post_root_reset_end,
 };
 
-/* Allwinner OTG supports up to 5 endpoints */
-#define SUNXI_MUSB_MAX_EP_NUM	6
 #define SUNXI_MUSB_RAM_BITS	11
 
-static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
+/* Allwinner OTG supports up to 5 endpoints */
+static struct musb_fifo_cfg sunxi_musb_mode_cfg_5eps[] = {
 	MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
 	MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
 	MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
@@ -639,9 +646,7 @@ static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
 };
 
 /* H3/V3s OTG supports only 4 endpoints */
-#define SUNXI_MUSB_MAX_EP_NUM_H3	5
-
-static struct musb_fifo_cfg sunxi_musb_mode_cfg_h3[] = {
+static struct musb_fifo_cfg sunxi_musb_mode_cfg_4eps[] = {
 	MUSB_EP_FIFO_SINGLE(1, FIFO_TX, 512),
 	MUSB_EP_FIFO_SINGLE(1, FIFO_RX, 512),
 	MUSB_EP_FIFO_SINGLE(2, FIFO_TX, 512),
@@ -652,31 +657,33 @@ static struct musb_fifo_cfg sunxi_musb_mode_cfg_h3[] = {
 	MUSB_EP_FIFO_SINGLE(4, FIFO_RX, 512),
 };
 
-static const struct musb_hdrc_config sunxi_musb_hdrc_config = {
-	.fifo_cfg       = sunxi_musb_mode_cfg,
-	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg),
+static const struct musb_hdrc_config sunxi_musb_hdrc_config_5eps = {
+	.fifo_cfg       = sunxi_musb_mode_cfg_5eps,
+	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_5eps),
 	.multipoint	= true,
 	.dyn_fifo	= true,
-	.num_eps	= SUNXI_MUSB_MAX_EP_NUM,
+	/* Two FIFOs per endpoint, plus ep_0. */
+	.num_eps	= (ARRAY_SIZE(sunxi_musb_mode_cfg_5eps) / 2) + 1,
 	.ram_bits	= SUNXI_MUSB_RAM_BITS,
 };
 
-static struct musb_hdrc_config sunxi_musb_hdrc_config_h3 = {
-	.fifo_cfg       = sunxi_musb_mode_cfg_h3,
-	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_h3),
+static const struct musb_hdrc_config sunxi_musb_hdrc_config_4eps = {
+	.fifo_cfg       = sunxi_musb_mode_cfg_4eps,
+	.fifo_cfg_size  = ARRAY_SIZE(sunxi_musb_mode_cfg_4eps),
 	.multipoint	= true,
 	.dyn_fifo	= true,
-	.num_eps	= SUNXI_MUSB_MAX_EP_NUM_H3,
+	/* Two FIFOs per endpoint, plus ep_0. */
+	.num_eps	= (ARRAY_SIZE(sunxi_musb_mode_cfg_4eps) / 2) + 1,
 	.ram_bits	= SUNXI_MUSB_RAM_BITS,
 };
 
-
 static int sunxi_musb_probe(struct platform_device *pdev)
 {
 	struct musb_hdrc_platform_data	pdata;
 	struct platform_device_info	pinfo;
 	struct sunxi_glue		*glue;
 	struct device_node		*np = pdev->dev.of_node;
+	const struct sunxi_musb_cfg	*cfg;
 	int ret;
 
 	if (!np) {
@@ -713,29 +720,25 @@ static int sunxi_musb_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 	pdata.platform_ops	= &sunxi_musb_ops;
-	if (!of_device_is_compatible(np, "allwinner,sun8i-h3-musb"))
-		pdata.config = &sunxi_musb_hdrc_config;
-	else
-		pdata.config = &sunxi_musb_hdrc_config_h3;
+
+	cfg = of_device_get_match_data(&pdev->dev);
+	if (!cfg)
+		return -EINVAL;
+
+	pdata.config = cfg->hdrc_config;
 
 	glue->dev = &pdev->dev;
 	INIT_WORK(&glue->work, sunxi_musb_work);
 	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
 
-	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb") ||
-	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
+	if (cfg->has_sram)
 		set_bit(SUNXI_MUSB_FL_HAS_SRAM, &glue->flags);
-	}
 
-	if (of_device_is_compatible(np, "allwinner,sun6i-a31-musb"))
+	if (cfg->has_reset)
 		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
 
-	if (of_device_is_compatible(np, "allwinner,sun8i-a33-musb") ||
-	    of_device_is_compatible(np, "allwinner,sun8i-h3-musb") ||
-	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
-		set_bit(SUNXI_MUSB_FL_HAS_RESET, &glue->flags);
+	if (cfg->no_configdata)
 		set_bit(SUNXI_MUSB_FL_NO_CONFIGDATA, &glue->flags);
-	}
 
 	glue->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(glue->clk)) {
@@ -813,12 +816,46 @@ static int sunxi_musb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct sunxi_musb_cfg sun4i_a10_musb_cfg = {
+	.hdrc_config = &sunxi_musb_hdrc_config_5eps,
+	.has_sram = true,
+};
+
+static const struct sunxi_musb_cfg sun6i_a31_musb_cfg = {
+	.hdrc_config = &sunxi_musb_hdrc_config_5eps,
+	.has_reset = true,
+};
+
+static const struct sunxi_musb_cfg sun8i_a33_musb_cfg = {
+	.hdrc_config = &sunxi_musb_hdrc_config_5eps,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
+static const struct sunxi_musb_cfg sun8i_h3_musb_cfg = {
+	.hdrc_config = &sunxi_musb_hdrc_config_4eps,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
+static const struct sunxi_musb_cfg suniv_f1c100s_musb_cfg = {
+	.hdrc_config = &sunxi_musb_hdrc_config_5eps,
+	.has_sram = true,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
 static const struct of_device_id sunxi_musb_match[] = {
-	{ .compatible = "allwinner,sun4i-a10-musb", },
-	{ .compatible = "allwinner,sun6i-a31-musb", },
-	{ .compatible = "allwinner,sun8i-a33-musb", },
-	{ .compatible = "allwinner,sun8i-h3-musb", },
-	{ .compatible = "allwinner,suniv-f1c100s-musb", },
+	{ .compatible = "allwinner,sun4i-a10-musb",
+	  .data = &sun4i_a10_musb_cfg, },
+	{ .compatible = "allwinner,sun6i-a31-musb",
+	  .data = &sun6i_a31_musb_cfg, },
+	{ .compatible = "allwinner,sun8i-a33-musb",
+	  .data = &sun8i_a33_musb_cfg, },
+	{ .compatible = "allwinner,sun8i-h3-musb",
+	  .data = &sun8i_h3_musb_cfg, },
+	{ .compatible = "allwinner,suniv-f1c100s-musb",
+	  .data = &suniv_f1c100s_musb_cfg, },
 	{}
 };
 MODULE_DEVICE_TABLE(of, sunxi_musb_match);
-- 
2.25.1

Re: [PATCH v6 2/3] usb: musb: sunxi: add support for the F1C100s MUSB controller

[Sergey Shtylyov]

On 2/1/23 1:53 PM, Andre Przywara wrote:

> From: Icenowy Zheng <uwu@icenowy.me>
> 
> The suniv SoC has a MUSB controller like the one in A33, but with a SRAM
> region to be claimed.
> 
> Add support for it.
> 
> Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
> Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  drivers/usb/musb/sunxi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 7f9a999cd5ff1..4b368d16a73ad 100644
> --- a/drivers/usb/musb/sunxi.c
> +++ b/drivers/usb/musb/sunxi.c
> @@ -722,14 +722,17 @@ static int sunxi_musb_probe(struct platform_device *pdev)
>  	INIT_WORK(&glue->work, sunxi_musb_work);
>  	glue->host_nb.notifier_call = sunxi_musb_host_notifier;
>  
> -	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb"))
> +	if (of_device_is_compatible(np, "allwinner,sun4i-a10-musb") ||
> +	    of_device_is_compatible(np, "allwinner,suniv-f1c100s-musb")) {
>  		set_bit(SUNXI_MUSB_FL_HAS_SRAM, &glue->flags);
> +	}

  Why {} around single statement?


[...]

MBR, Sergey