Hi Geert,
Thanks for the feedback.
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Friday, March 8, 2024 7:40 PM
> To: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxxxxxx> wrote:
> > From: Huy Nguyen <huy.nguyen.wh@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> 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);
