diff mbox series

[v3,11/11] usb: musb: sunxi: Introduce config struct

Message ID 20221106154826.6687-12-andre.przywara@arm.com
State Changes Requested
Headers show
Series ARM: suniv: USB and PopStick board support | expand

Commit Message

Andre Przywara Nov. 6, 2022, 3:48 p.m. UTC 
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>
---
 drivers/usb/musb/sunxi.c | 101 +++++++++++++++++++++++++++++----------
 1 file changed, 75 insertions(+), 26 deletions(-)

Comments

Jernej Škrabec Nov. 7, 2022, 5:56 p.m. UTC | #1 
Hi Andre!

Dne nedelja, 06. november 2022 ob 16:48:26 CET je Andre Przywara napisal(a):
> 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>
> ---
>  drivers/usb/musb/sunxi.c | 101 +++++++++++++++++++++++++++++----------
>  1 file changed, 75 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
> index 4b368d16a73a..266f8baf5af0 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 {
> +	int nr_endpoints;
> +	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;
> 
> @@ -625,7 +633,7 @@ static const struct musb_platform_ops sunxi_musb_ops = {
> #define SUNXI_MUSB_MAX_EP_NUM	6
>  #define SUNXI_MUSB_RAM_BITS	11
> 
> -static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
> +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),
> @@ -641,7 +649,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,18 +660,18 @@ 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,
>  	.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 struct musb_hdrc_config sunxi_musb_hdrc_config_4eps = {

While at it, you can mark above struct as const. 5eps struct is already marked 
as const.

> +	.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,
> @@ -677,6 +685,7 @@ static int sunxi_musb_probe(struct platform_device
> *pdev) 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 +722,35 @@ 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;
> +
> +	switch (cfg->nr_endpoints) {
> +	case 4:
> +		pdata.config = &sunxi_musb_hdrc_config_4eps;
> +		break;
> +	case 5:
> +		pdata.config = &sunxi_musb_hdrc_config_5eps;
> +		break;
> +	default:
> +		dev_err(&pdev->dev, "Only 4 or 5 endpoints 
supported\n");
> +		return -EINVAL;
> +	}

Overall nice cleanup! Only thing I would rather see different is to use pointer 
to struct musb_fifo_cfg directly in config struct. That way you avoid above 
switch case.

Best regards,
Jernej

> 
>  	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 +828,46 @@ static int sunxi_musb_remove(struct platform_device
> *pdev) return 0;
>  }
> 
> +static const struct sunxi_musb_cfg sun4i_a10_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.has_sram = true,
> +};
> +
> +static const struct sunxi_musb_cfg sun6i_a31_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.has_reset = true,
> +};
> +
> +static const struct sunxi_musb_cfg sun8i_a33_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.has_reset = true,
> +	.no_configdata = true,
> +};
> +
> +static const struct sunxi_musb_cfg sun8i_h3_musb_cfg = {
> +	.nr_endpoints = 4,
> +	.has_reset = true,
> +	.no_configdata = true,
> +};
> +
> +static const struct sunxi_musb_cfg suniv_f1c100s_musb_cfg = {
> +	.nr_endpoints = 5,
> +	.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.35.5
diff mbox series

Patch

diff --git a/drivers/usb/musb/sunxi.c b/drivers/usb/musb/sunxi.c
index 4b368d16a73a..266f8baf5af0 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 {
+	int nr_endpoints;
+	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;
 
@@ -625,7 +633,7 @@  static const struct musb_platform_ops sunxi_musb_ops = {
 #define SUNXI_MUSB_MAX_EP_NUM	6
 #define SUNXI_MUSB_RAM_BITS	11
 
-static struct musb_fifo_cfg sunxi_musb_mode_cfg[] = {
+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),
@@ -641,7 +649,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,18 +660,18 @@  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,
 	.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 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,
@@ -677,6 +685,7 @@  static int sunxi_musb_probe(struct platform_device *pdev)
 	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 +722,35 @@  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;
+
+	switch (cfg->nr_endpoints) {
+	case 4:
+		pdata.config = &sunxi_musb_hdrc_config_4eps;
+		break;
+	case 5:
+		pdata.config = &sunxi_musb_hdrc_config_5eps;
+		break;
+	default:
+		dev_err(&pdev->dev, "Only 4 or 5 endpoints supported\n");
+		return -EINVAL;
+	}
 
 	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 +828,46 @@  static int sunxi_musb_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct sunxi_musb_cfg sun4i_a10_musb_cfg = {
+	.nr_endpoints = 5,
+	.has_sram = true,
+};
+
+static const struct sunxi_musb_cfg sun6i_a31_musb_cfg = {
+	.nr_endpoints = 5,
+	.has_reset = true,
+};
+
+static const struct sunxi_musb_cfg sun8i_a33_musb_cfg = {
+	.nr_endpoints = 5,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
+static const struct sunxi_musb_cfg sun8i_h3_musb_cfg = {
+	.nr_endpoints = 4,
+	.has_reset = true,
+	.no_configdata = true,
+};
+
+static const struct sunxi_musb_cfg suniv_f1c100s_musb_cfg = {
+	.nr_endpoints = 5,
+	.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);