diff mbox series

[07/16] reset: rzg2l-usbphy-ctrl: Get reset control array

Message ID 20240822152801.602318-8-claudiu.beznea.uj@bp.renesas.com (mailing list archive)
State Not Applicable, archived
Headers show
Series Add initial USB support for the Renesas RZ/G3S SoC | expand

Commit Message

Claudiu Aug. 22, 2024, 3:27 p.m. UTC 
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>
---
 drivers/reset/reset-rzg2l-usbphy-ctrl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Biju Das Aug. 23, 2024, 7:25 a.m. UTC | #1 
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
Claudiu Aug. 23, 2024, 8:05 a.m. UTC | #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
>
Biju Das Aug. 23, 2024, 8:17 a.m. UTC | #3 
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
> >
Geert Uytterhoeven Oct. 8, 2024, 2:46 p.m. UTC | #4 
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 mbox series

Patch

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");