diff mbox series

[2/4] usb: renesas_usbhs: Update usbhs pipe configuration for RZ/G2L family

Message ID 20240308180919.6603-3-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series Fix USB pipe configuration for RZ/G2L | expand

Commit Message

Biju Das March 8, 2024, 6:09 p.m. UTC 
From: Huy Nguyen <huy.nguyen.wh@renesas.com>

The RZ/G2L family SoCs has 10 PIPE buffers compared to 16 pipe
buffers on RZ/A2M. Update the pipe configuration for RZ/G2L family
SoCs. For the backward compatibility SoC specific compatible is used
and will remove the same after few kernel releases.

Signed-off-by: Huy Nguyen <huy.nguyen.wh@renesas.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/usb/renesas_usbhs/common.c | 31 ++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven March 8, 2024, 7:39 p.m. UTC | #1 
Hi Biju,

On Fri, Mar 8, 2024 at 7:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> From: Huy Nguyen <huy.nguyen.wh@renesas.com>
>
> The RZ/G2L family SoCs has 10 PIPE buffers compared to 16 pipe
> buffers on RZ/A2M. Update the pipe configuration for RZ/G2L family
> SoCs. For the backward compatibility SoC specific compatible is used
> and will remove the same after few kernel releases.
>
> Signed-off-by: Huy Nguyen <huy.nguyen.wh@renesas.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/usb/renesas_usbhs/common.c
> +++ b/drivers/usb/renesas_usbhs/common.c
> @@ -397,6 +397,20 @@ static struct renesas_usbhs_driver_pipe_config usbhsc_new_pipe[] = {
>         RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0xd8, true),
>  };
>
> +/* commonly used on RZ/G2L family */
> +static struct renesas_usbhs_driver_pipe_config usbhsc_rzg2l_pipe[] = {
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_CONTROL, 64, 0x00, false),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_ISOC, 1024, 0x08, true),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_ISOC, 1024, 0x28, true),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x48, true),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x58, true),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x68, true),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x04, false),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x05, false),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x06, false),
> +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x07, false),
> +};
> +
>  /*
>   *             power control
>   */
> @@ -581,6 +595,10 @@ static const struct of_device_id usbhs_of_match[] = {
>                 .compatible = "renesas,rza2-usbhs",
>                 .data = &usbhs_rza2_plat_info,
>         },
> +       {
> +               .compatible = "renesas,rzg2l-usbhs",
> +               .data = &usbhs_rza2_plat_info,

Is usbhs_rza2_plat_info correct for RZ/G2L?

> +       },
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, usbhs_of_match);
> @@ -645,8 +663,17 @@ static int usbhs_probe(struct platform_device *pdev)
>
>         /* set default param if platform doesn't have */
>         if (usbhs_get_dparam(priv, has_new_pipe_configs)) {
> -               priv->dparam.pipe_configs = usbhsc_new_pipe;
> -               priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
> +               /* for backward compatibility check soc specific compatible */
> +               if (of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g043") ||
> +                   of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g044") ||
> +                   of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g054") ||

Please no of_device_is_compatible() checks in a driver's .probe()
method. Just add entries to usbhs_of_match[] instead.

> +                   of_device_is_compatible(pdev->dev.of_node, "renesas,rzg2l-usbhs")) {

Ah, that's where you really handle RZ/G2L.
Please use renesas_usbhs_platform_info instead.

> +                       priv->dparam.pipe_configs = usbhsc_rzg2l_pipe;
> +                       priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_rzg2l_pipe);
> +               } else {
> +                       priv->dparam.pipe_configs = usbhsc_new_pipe;
> +                       priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
> +               }
>         } else if (!priv->dparam.pipe_configs) {
>                 priv->dparam.pipe_configs = usbhsc_default_pipe;
>                 priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_default_pipe);

Gr{oetje,eeting}s,

                        Geert
Biju Das March 9, 2024, 10:20 a.m. UTC | #2 
Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Friday, March 8, 2024 7:40 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Subject: Re: [PATCH 2/4] usb: renesas_usbhs: Update usbhs pipe configuration for RZ/G2L family
> 
> Hi Biju,
> 
> On Fri, Mar 8, 2024 at 7:09 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > From: Huy Nguyen <huy.nguyen.wh@renesas.com>
> >
> > The RZ/G2L family SoCs has 10 PIPE buffers compared to 16 pipe buffers
> > on RZ/A2M. Update the pipe configuration for RZ/G2L family SoCs. For
> > the backward compatibility SoC specific compatible is used and will
> > remove the same after few kernel releases.
> >
> > Signed-off-by: Huy Nguyen <huy.nguyen.wh@renesas.com>
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/usb/renesas_usbhs/common.c
> > +++ b/drivers/usb/renesas_usbhs/common.c
> > @@ -397,6 +397,20 @@ static struct renesas_usbhs_driver_pipe_config usbhsc_new_pipe[] = {
> >         RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0xd8, true),
> > };
> >
> > +/* commonly used on RZ/G2L family */
> > +static struct renesas_usbhs_driver_pipe_config usbhsc_rzg2l_pipe[] = {
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_CONTROL, 64, 0x00, false),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_ISOC, 1024, 0x08, true),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_ISOC, 1024, 0x28, true),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x48, true),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x58, true),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x68, true),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x04, false),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x05, false),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x06, false),
> > +       RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x07, false), };
> > +
> >  /*
> >   *             power control
> >   */
> > @@ -581,6 +595,10 @@ static const struct of_device_id usbhs_of_match[] = {
> >                 .compatible = "renesas,rza2-usbhs",
> >                 .data = &usbhs_rza2_plat_info,
> >         },
> > +       {
> > +               .compatible = "renesas,rzg2l-usbhs",
> > +               .data = &usbhs_rza2_plat_info,
> 
> Is usbhs_rza2_plat_info correct for RZ/G2L?

Ok, Will use usbhs_rzg2l_plat_info, filling pipe_configs and pipe_size.

> 
> > +       },
> >         { },
> >  };
> >  MODULE_DEVICE_TABLE(of, usbhs_of_match); @@ -645,8 +663,17 @@ static
> > int usbhs_probe(struct platform_device *pdev)
> >
> >         /* set default param if platform doesn't have */
> >         if (usbhs_get_dparam(priv, has_new_pipe_configs)) {
> > -               priv->dparam.pipe_configs = usbhsc_new_pipe;
> > -               priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
> > +               /* for backward compatibility check soc specific compatible */
> > +               if (of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g043") ||
> > +                   of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g044") ||
> > +                   of_device_is_compatible(pdev->dev.of_node,
> > + "renesas,usbhs-r9a07g054") ||
> 
> Please no of_device_is_compatible() checks in a driver's .probe() method. Just add entries to
> usbhs_of_match[] instead.

Ok.
> 
> > +                   of_device_is_compatible(pdev->dev.of_node,
> > + "renesas,rzg2l-usbhs")) {
> 
> Ah, that's where you really handle RZ/G2L.
> Please use renesas_usbhs_platform_info instead.

Agreed, will move the table to rza2.c and use info to fill
pipe_configs and pipe_size.

Cheers,
Biju

> 
> > +                       priv->dparam.pipe_configs = usbhsc_rzg2l_pipe;
> > +                       priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_rzg2l_pipe);
> > +               } else {
> > +                       priv->dparam.pipe_configs = usbhsc_new_pipe;
> > +                       priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
> > +               }
> >         } else if (!priv->dparam.pipe_configs) {
> >                 priv->dparam.pipe_configs = usbhsc_default_pipe;
> >                 priv->dparam.pipe_size =
> > ARRAY_SIZE(usbhsc_default_pipe);
diff mbox series

Patch

diff --git a/drivers/usb/renesas_usbhs/common.c b/drivers/usb/renesas_usbhs/common.c
index dd1c17542439..030ec36deb64 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -397,6 +397,20 @@  static struct renesas_usbhs_driver_pipe_config usbhsc_new_pipe[] = {
 	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0xd8, true),
 };
 
+/* commonly used on RZ/G2L family */
+static struct renesas_usbhs_driver_pipe_config usbhsc_rzg2l_pipe[] = {
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_CONTROL, 64, 0x00, false),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_ISOC, 1024, 0x08, true),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_ISOC, 1024, 0x28, true),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x48, true),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x58, true),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_BULK, 512, 0x68, true),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x04, false),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x05, false),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x06, false),
+	RENESAS_USBHS_PIPE(USB_ENDPOINT_XFER_INT, 64, 0x07, false),
+};
+
 /*
  *		power control
  */
@@ -581,6 +595,10 @@  static const struct of_device_id usbhs_of_match[] = {
 		.compatible = "renesas,rza2-usbhs",
 		.data = &usbhs_rza2_plat_info,
 	},
+	{
+		.compatible = "renesas,rzg2l-usbhs",
+		.data = &usbhs_rza2_plat_info,
+	},
 	{ },
 };
 MODULE_DEVICE_TABLE(of, usbhs_of_match);
@@ -645,8 +663,17 @@  static int usbhs_probe(struct platform_device *pdev)
 
 	/* set default param if platform doesn't have */
 	if (usbhs_get_dparam(priv, has_new_pipe_configs)) {
-		priv->dparam.pipe_configs = usbhsc_new_pipe;
-		priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
+		/* for backward compatibility check soc specific compatible */
+		if (of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g043") ||
+		    of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g044") ||
+		    of_device_is_compatible(pdev->dev.of_node, "renesas,usbhs-r9a07g054") ||
+		    of_device_is_compatible(pdev->dev.of_node, "renesas,rzg2l-usbhs")) {
+			priv->dparam.pipe_configs = usbhsc_rzg2l_pipe;
+			priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_rzg2l_pipe);
+		} else {
+			priv->dparam.pipe_configs = usbhsc_new_pipe;
+			priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_new_pipe);
+		}
 	} else if (!priv->dparam.pipe_configs) {
 		priv->dparam.pipe_configs = usbhsc_default_pipe;
 		priv->dparam.pipe_size = ARRAY_SIZE(usbhsc_default_pipe);