| Message ID | 20240822152801.602318-8-claudiu.beznea.uj@bp.renesas.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add initial USB support for the Renesas RZ/G3S SoC | expand |
Hi Claudiu, > -----Original Message----- > From: Claudiu <claudiu.beznea@tuxon.dev> > Sent: Thursday, August 22, 2024 4:28 PM > Subject: [PATCH 07/16] reset: rzg2l-usbphy-ctrl: Get reset control array > > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Before accessing the USB area of the RZ/G3S SoC the PWRRDY bit of the SYS_USB_PWRRDY register need to > be cleared. When USB area is not used the PWRRDY bit of the SYS_USB_PWRRDY register need to be set. > This register is in the SYSC controller address space and the assert/de-assert of the signal handled > by SYSC_USB_PWRRDY was implemented as a reset signal. > > The USB modules available on the RZ/G3S SoC that need this bit set are: > - USB ch0 (supporting host and peripheral mode) > - USB ch2 (supporting host mode) > - USBPHY control > > As the USBPHY control is the root device for all the other USB channels (USB ch0, USB ch1) add support > to set the PWRRDY for the USB area when initializing the USBPHY control. As this is done though reset > signals get the reset array in the USBPHY control driver. > Comment applicable, if the USB PWRRDY signal is modelled as reset signal. There is no user for this patch. The first user is RZ/G3S and is there is no support yet. I think you should merge this patch with next one so that there is a user(RZ/G3S) for this patch. Cheers, Biju > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > drivers/reset/reset-rzg2l-usbphy-ctrl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/reset/reset-rzg2l-usbphy-ctrl.c b/drivers/reset/reset-rzg2l-usbphy-ctrl.c > index 1cd157f4f03b..8b64c12f3bec 100644 > --- a/drivers/reset/reset-rzg2l-usbphy-ctrl.c > +++ b/drivers/reset/reset-rzg2l-usbphy-ctrl.c > @@ -132,7 +132,7 @@ static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev) > if (IS_ERR(regmap)) > return PTR_ERR(regmap); > > - priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > + priv->rstc = devm_reset_control_array_get_exclusive(&pdev->dev); > if (IS_ERR(priv->rstc)) > return dev_err_probe(dev, PTR_ERR(priv->rstc), > "failed to get reset\n"); > -- > 2.39.2
Hi, Biju, On 23.08.2024 10:25, Biju Das wrote: > Hi Claudiu, > >> -----Original Message----- >> From: Claudiu <claudiu.beznea@tuxon.dev> >> Sent: Thursday, August 22, 2024 4:28 PM >> Subject: [PATCH 07/16] reset: rzg2l-usbphy-ctrl: Get reset control array >> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> Before accessing the USB area of the RZ/G3S SoC the PWRRDY bit of the SYS_USB_PWRRDY register need to >> be cleared. When USB area is not used the PWRRDY bit of the SYS_USB_PWRRDY register need to be set. >> This register is in the SYSC controller address space and the assert/de-assert of the signal handled >> by SYSC_USB_PWRRDY was implemented as a reset signal. >> >> The USB modules available on the RZ/G3S SoC that need this bit set are: >> - USB ch0 (supporting host and peripheral mode) >> - USB ch2 (supporting host mode) >> - USBPHY control >> >> As the USBPHY control is the root device for all the other USB channels (USB ch0, USB ch1) add support >> to set the PWRRDY for the USB area when initializing the USBPHY control. As this is done though reset >> signals get the reset array in the USBPHY control driver. >> > > Comment applicable, if the USB PWRRDY signal is modelled as reset signal. > > There is no user for this patch. The first user is RZ/G3S and is there is no support yet. > I think you should merge this patch with next one so that there is a user(RZ/G3S) > for this patch. I have nothing against to squash it. I was seeing it differently: - this patch prepares the ground for the addition of RZ/G3S - the next patch just enables the support for RZ/G3S It looks to me more modular like this, patches are simpler, easier for review and each issue is described differently in patch description. I can do it either ways. Thank you, Claudiu Beznea > Cheers, > Biju > >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> --- >> drivers/reset/reset-rzg2l-usbphy-ctrl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/reset/reset-rzg2l-usbphy-ctrl.c b/drivers/reset/reset-rzg2l-usbphy-ctrl.c >> index 1cd157f4f03b..8b64c12f3bec 100644 >> --- a/drivers/reset/reset-rzg2l-usbphy-ctrl.c >> +++ b/drivers/reset/reset-rzg2l-usbphy-ctrl.c >> @@ -132,7 +132,7 @@ static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev) >> if (IS_ERR(regmap)) >> return PTR_ERR(regmap); >> >> - priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); >> + priv->rstc = devm_reset_control_array_get_exclusive(&pdev->dev); >> if (IS_ERR(priv->rstc)) >> return dev_err_probe(dev, PTR_ERR(priv->rstc), >> "failed to get reset\n"); >> -- >> 2.39.2 >
Hi Claudiu, > -----Original Message----- > From: claudiu beznea <claudiu.beznea@tuxon.dev> > Sent: Friday, August 23, 2024 9:05 AM > Subject: Re: [PATCH 07/16] reset: rzg2l-usbphy-ctrl: Get reset control array > > Hi, Biju, > > On 23.08.2024 10:25, Biju Das wrote: > > Hi Claudiu, > > > >> -----Original Message----- > >> From: Claudiu <claudiu.beznea@tuxon.dev> > >> Sent: Thursday, August 22, 2024 4:28 PM > >> Subject: [PATCH 07/16] reset: rzg2l-usbphy-ctrl: Get reset control > >> array > >> > >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> > >> Before accessing the USB area of the RZ/G3S SoC the PWRRDY bit of the > >> SYS_USB_PWRRDY register need to be cleared. When USB area is not used the PWRRDY bit of the > SYS_USB_PWRRDY register need to be set. > >> This register is in the SYSC controller address space and the > >> assert/de-assert of the signal handled by SYSC_USB_PWRRDY was implemented as a reset signal. > >> > >> The USB modules available on the RZ/G3S SoC that need this bit set are: > >> - USB ch0 (supporting host and peripheral mode) > >> - USB ch2 (supporting host mode) > >> - USBPHY control > >> > >> As the USBPHY control is the root device for all the other USB > >> channels (USB ch0, USB ch1) add support to set the PWRRDY for the USB > >> area when initializing the USBPHY control. As this is done though reset signals get the reset array > in the USBPHY control driver. > >> > > > > Comment applicable, if the USB PWRRDY signal is modelled as reset signal. > > > > There is no user for this patch. The first user is RZ/G3S and is there is no support yet. > > I think you should merge this patch with next one so that there is a > > user(RZ/G3S) for this patch. > > I have nothing against to squash it. I was seeing it differently: > - this patch prepares the ground for the addition of RZ/G3S > - the next patch just enables the support for RZ/G3S > > It looks to me more modular like this, patches are simpler, easier for review and each issue is > described differently in patch description. > > I can do it either ways. If it is complicated restructuring for supporting new SoC then it make sense to split into a number of patches. This is straight forward. You need a device quirk to support RZ/G3S. So this kind of changes can go in a single patch, as on second patch you are just filling device entry. Previously I got feedback from mainline to add a patch, if there is a user. Cheers, Biju > >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > >> --- > >> drivers/reset/reset-rzg2l-usbphy-ctrl.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/reset/reset-rzg2l-usbphy-ctrl.c > >> b/drivers/reset/reset-rzg2l-usbphy-ctrl.c > >> index 1cd157f4f03b..8b64c12f3bec 100644 > >> --- a/drivers/reset/reset-rzg2l-usbphy-ctrl.c > >> +++ b/drivers/reset/reset-rzg2l-usbphy-ctrl.c > >> @@ -132,7 +132,7 @@ static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev) > >> if (IS_ERR(regmap)) > >> return PTR_ERR(regmap); > >> > >> - priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > >> + priv->rstc = devm_reset_control_array_get_exclusive(&pdev->dev); > >> if (IS_ERR(priv->rstc)) > >> return dev_err_probe(dev, PTR_ERR(priv->rstc), > >> "failed to get reset\n"); > >> -- > >> 2.39.2 > >
On Thu, Aug 22, 2024 at 5:28 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Before accessing the USB area of the RZ/G3S SoC the PWRRDY bit of the > SYS_USB_PWRRDY register need to be cleared. When USB area is not used the > PWRRDY bit of the SYS_USB_PWRRDY register need to be set. This register is > in the SYSC controller address space and the assert/de-assert of the > signal handled by SYSC_USB_PWRRDY was implemented as a reset signal. > > The USB modules available on the RZ/G3S SoC that need this bit set are: > - USB ch0 (supporting host and peripheral mode) > - USB ch2 (supporting host mode) > - USBPHY control > > As the USBPHY control is the root device for all the other USB channels > (USB ch0, USB ch1) add support to set the PWRRDY for the USB area when > initializing the USBPHY control. As this is done though reset signals > get the reset array in the USBPHY control driver. > > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
diff --git a/drivers/reset/reset-rzg2l-usbphy-ctrl.c b/drivers/reset/reset-rzg2l-usbphy-ctrl.c index 1cd157f4f03b..8b64c12f3bec 100644 --- a/drivers/reset/reset-rzg2l-usbphy-ctrl.c +++ b/drivers/reset/reset-rzg2l-usbphy-ctrl.c @@ -132,7 +132,7 @@ static int rzg2l_usbphy_ctrl_probe(struct platform_device *pdev) if (IS_ERR(regmap)) return PTR_ERR(regmap); - priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); + priv->rstc = devm_reset_control_array_get_exclusive(&pdev->dev); if (IS_ERR(priv->rstc)) return dev_err_probe(dev, PTR_ERR(priv->rstc), "failed to get reset\n");