mbox series

[00/16] Add initial USB support for the Renesas RZ/G3S SoC

Message ID 20240822152801.602318-1-claudiu.beznea.uj@bp.renesas.com
Headers show
Series Add initial USB support for the Renesas RZ/G3S SoC | expand

Message

Claudiu Beznea Aug. 22, 2024, 3:27 p.m. UTC
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Hi,

Series adds initial USB support for the Renesas RZ/G3S SoC.

Series is split as follows:

- patch 01/16		- add clock reset and power domain support for USB
- patch 02-04/16	- add reset control support for a USB signal
			  that need to be controlled before/after
			  the power to USB area is turned on/off.
			  
			  Philipp, Ulf, Geert, all,
			  
			  I detailed my approach for this in patch
			  04/16, please have a look and let me know
			  your input.
			  
			  Thank you!

- patch 05/16		- moves SoC identification to SYSC driver
- patch 06-08/16	- updates USB PHY control driver for USB
			  support
- patch 09/16		- update documentation for USBHS
- patch 10-12/16	- updates the USB PHY driver for USB support
- patch 13-15/16	- updates the device tree with USB support
- patch 16/16		- enables the reset control driver

Thank you,
Claudiu Beznea

Claudiu Beznea (16):
  clk: renesas: r9a08g045: Add clocks, resets and power domains for USB
  dt-bindings: soc: renesas: renesas,rzg2l-sysc: Add #reset-cells for
    RZ/G3S
  dt-bindings: reset: renesas,r9a08g045-sysc: Add reset IDs for RZ/G3S
    SYSC reset
  soc: renesas: Add SYSC driver for Renesas RZ/G3S
  soc: renesas: sysc: Move RZ/G3S SoC detection on SYSC driver
  dt-bindings: reset: renesas,rzg2l-usbphy-ctrl: Document RZ/G3S SoC
  reset: rzg2l-usbphy-ctrl: Get reset control array
  reset: rzg2l-usbphy-ctrl: Add support for RZ/G3S
  dt-bindings: usb: renesas,usbhs: Document RZ/G3S SoC
  phy: renesas: rcar-gen3-usb2: Add support to initialize the bus
  dt-bindings: phy: renesas,usb2-phy: Document RZ/G3S phy bindings
  phy: renesas: rcar-gen3-usb2: Add support for the RZ/G3S SoC
  arm64: dts: renesas: Add #reset-cells to system controller node
  arm64: dts: renesas: r9a08g045: Add USB support
  arm64: dts: renesas: rzg3s-smarc: Enable USB support
  arm64: defconfig: Enable RZ/G3S SYSC reset driver

 .../bindings/phy/renesas,usb2-phy.yaml        |   4 +-
 .../reset/renesas,rzg2l-usbphy-ctrl.yaml      |  35 +++-
 .../soc/renesas/renesas,rzg2l-sysc.yaml       |  16 ++
 .../bindings/usb/renesas,usbhs.yaml           |   2 +
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi    | 120 +++++++++++++
 arch/arm64/boot/dts/renesas/rzg3s-smarc.dtsi  |  61 +++++++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/clk/renesas/r9a08g045-cpg.c           |  17 ++
 drivers/phy/renesas/phy-rcar-gen3-usb2.c      |  60 ++++++-
 drivers/reset/Kconfig                         |   7 +
 drivers/reset/Makefile                        |   1 +
 drivers/reset/reset-rzg2l-usbphy-ctrl.c       |   3 +-
 drivers/reset/reset-rzg3s-sysc.c              | 140 ++++++++++++++++
 drivers/soc/renesas/Makefile                  |   1 +
 drivers/soc/renesas/renesas-soc.c             |  12 --
 drivers/soc/renesas/rzg3s-sysc.c              | 158 ++++++++++++++++++
 .../reset/renesas,r9a08g045-sysc.h            |  10 ++
 include/linux/soc/renesas/rzg3s-sysc-reset.h  |  24 +++
 18 files changed, 648 insertions(+), 24 deletions(-)
 create mode 100644 drivers/reset/reset-rzg3s-sysc.c
 create mode 100644 drivers/soc/renesas/rzg3s-sysc.c
 create mode 100644 include/dt-bindings/reset/renesas,r9a08g045-sysc.h
 create mode 100644 include/linux/soc/renesas/rzg3s-sysc-reset.h

Comments

Conor Dooley Aug. 22, 2024, 4:45 p.m. UTC | #1
On Thu, Aug 22, 2024 at 06:27:48PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Add reset IDs for the Renesas RZ/G3S SYSC reset controller driver.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Conor Dooley Aug. 22, 2024, 4:46 p.m. UTC | #2
On Thu, Aug 22, 2024 at 06:27:54PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The USBHS IP block on RZ/G3S SoC is identitcal to the one found on the
> RZ/G2L device. Document the RZ/G3S USBHS IP block.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Ulf Hansson Aug. 29, 2024, 3:26 p.m. UTC | #3
On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> Hi,
>
> Series adds initial USB support for the Renesas RZ/G3S SoC.
>
> Series is split as follows:
>
> - patch 01/16           - add clock reset and power domain support for USB
> - patch 02-04/16        - add reset control support for a USB signal
>                           that need to be controlled before/after
>                           the power to USB area is turned on/off.
>
>                           Philipp, Ulf, Geert, all,
>
>                           I detailed my approach for this in patch
>                           04/16, please have a look and let me know
>                           your input.

I have looked briefly. Your suggested approach may work, but I have a
few thoughts, see below.

If I understand correctly, it is the consumer driver for the device
that is attached to the USB power domain that becomes responsible for
asserting/de-asserting this new signal. Right?

In this regard, please note that the consumer driver doesn't really
know when the power domain really gets powered-on/off. Calling
pm_runtime_get|put*() is dealing with the reference counting. For
example, a call to pm_runtime_get*() just makes sure that the PM
domain gets-or-remains powered-on. Could this be a problem from the
reset-signal point of view?

[...]

Kind regards
Uffe
Vinod Koul Aug. 30, 2024, 8:10 a.m. UTC | #4
On Thu, 22 Aug 2024 18:27:45 +0300, Claudiu wrote:
> Series adds initial USB support for the Renesas RZ/G3S SoC.
> 
> Series is split as follows:
> 
> - patch 01/16		- add clock reset and power domain support for USB
> - patch 02-04/16	- add reset control support for a USB signal
> 			  that need to be controlled before/after
> 			  the power to USB area is turned on/off.
> 
> [...]

Applied, thanks!

[10/16] phy: renesas: rcar-gen3-usb2: Add support to initialize the bus
        commit: 4eae16375357a2a7e8501be5469532f7636064b3
[11/16] dt-bindings: phy: renesas,usb2-phy: Document RZ/G3S phy bindings
        commit: f3c8498551146dfb014be0d85d3a7df98be16aa2
[12/16] phy: renesas: rcar-gen3-usb2: Add support for the RZ/G3S SoC
        commit: 3c2ea12a625dbf5a864f4920235fa1c739d06e7d

Best regards,
Claudiu Beznea Aug. 30, 2024, 8:22 a.m. UTC | #5
Hi, Ulf,

On 29.08.2024 18:26, Ulf Hansson wrote:
> On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Hi,
>>
>> Series adds initial USB support for the Renesas RZ/G3S SoC.
>>
>> Series is split as follows:
>>
>> - patch 01/16           - add clock reset and power domain support for USB
>> - patch 02-04/16        - add reset control support for a USB signal
>>                           that need to be controlled before/after
>>                           the power to USB area is turned on/off.
>>
>>                           Philipp, Ulf, Geert, all,
>>
>>                           I detailed my approach for this in patch
>>                           04/16, please have a look and let me know
>>                           your input.
> 
> I have looked briefly. Your suggested approach may work, but I have a
> few thoughts, see below.
> 
> If I understand correctly, it is the consumer driver for the device
> that is attached to the USB power domain that becomes responsible for
> asserting/de-asserting this new signal. Right?

Right!

> 
> In this regard, please note that the consumer driver doesn't really
> know when the power domain really gets powered-on/off. Calling
> pm_runtime_get|put*() is dealing with the reference counting. For
> example, a call to pm_runtime_get*() just makes sure that the PM
> domain gets-or-remains powered-on. Could this be a problem from the
> reset-signal point of view?

It should be safe. From the HW manual I understand the hardware block is
something like the following:


                  USB area
         +-------------------------+
         |                         |
         | PHY --->USB controller  |
SYSC --> |  ^                      |
         |  |                      |
         | PHY reset               |
         +-------------------------+

Where:
- SYSC is the system controller that controls the new signal for which
  I'm requesting opinions in this series
- PHY reset: is the block controlling the PHYs
- PHY: is the block controlling the USB PHYs
- USB controller: is the USB controller

Currently, I passed the SYSC signal handling to the PHY reset driver; w/o
PHY reset the rest of the USB logic cannot work (neither PHY block nor USB
controller).

Currently, the PHY reset driver call pm_runtime_resume_and_get() in probe
and pm_runtime_put() in remove. The struct reset_control_ops::{assert,
deassert} only set specific bits in registers (no pm_runtime* calls).

The PHY driver is taking its PHY reset in probe and release it in remove().
With this approach the newly introduced SYSC signal will be
de-asserted/asserted only in the PHY reset probe/remove (either if it is
handled though PM domain or reset control signal).

If the SYSC signal would be passed to all the blocks in the USB area (and
it would be handled though PM domains) it should be no problem either,
AFAICT, because of reference counting the pm_runtime_get|put*() is taking
care of. As the PHY reset is the root node the in the devices node tree for
USB the reference counting should work, too (I may miss something though,
please correct me if I'm wrong).

If the SYSC signal would be handled though a reset control driver (as
proposed in this series) and we want to pass this reference to all the
blocks in the USB area then we can request the reset signal as shared and,
AFAIK, this is also reference counted. The devices node tree should help
with the order, too, if I'm not wrong.

Thank you for looking at this,
Claudiu Beznea

> 
> [...]
> 
> Kind regards
> Uffe
Ulf Hansson Aug. 30, 2024, 10:14 a.m. UTC | #6
On Fri, 30 Aug 2024 at 10:22, claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>
> Hi, Ulf,
>
> On 29.08.2024 18:26, Ulf Hansson wrote:
> > On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> Hi,
> >>
> >> Series adds initial USB support for the Renesas RZ/G3S SoC.
> >>
> >> Series is split as follows:
> >>
> >> - patch 01/16           - add clock reset and power domain support for USB
> >> - patch 02-04/16        - add reset control support for a USB signal
> >>                           that need to be controlled before/after
> >>                           the power to USB area is turned on/off.
> >>
> >>                           Philipp, Ulf, Geert, all,
> >>
> >>                           I detailed my approach for this in patch
> >>                           04/16, please have a look and let me know
> >>                           your input.
> >
> > I have looked briefly. Your suggested approach may work, but I have a
> > few thoughts, see below.
> >
> > If I understand correctly, it is the consumer driver for the device
> > that is attached to the USB power domain that becomes responsible for
> > asserting/de-asserting this new signal. Right?
>
> Right!
>
> >
> > In this regard, please note that the consumer driver doesn't really
> > know when the power domain really gets powered-on/off. Calling
> > pm_runtime_get|put*() is dealing with the reference counting. For
> > example, a call to pm_runtime_get*() just makes sure that the PM
> > domain gets-or-remains powered-on. Could this be a problem from the
> > reset-signal point of view?
>
> It should be safe. From the HW manual I understand the hardware block is
> something like the following:
>
>
>                   USB area
>          +-------------------------+
>          |                         |
>          | PHY --->USB controller  |
> SYSC --> |  ^                      |
>          |  |                      |
>          | PHY reset               |
>          +-------------------------+
>
> Where:
> - SYSC is the system controller that controls the new signal for which
>   I'm requesting opinions in this series
> - PHY reset: is the block controlling the PHYs
> - PHY: is the block controlling the USB PHYs
> - USB controller: is the USB controller
>
> Currently, I passed the SYSC signal handling to the PHY reset driver; w/o
> PHY reset the rest of the USB logic cannot work (neither PHY block nor USB
> controller).
>
> Currently, the PHY reset driver call pm_runtime_resume_and_get() in probe
> and pm_runtime_put() in remove. The struct reset_control_ops::{assert,
> deassert} only set specific bits in registers (no pm_runtime* calls).

Thanks for clarifying!

For my understanding, in what register range do these bits belong? Is
it the USB logic or in the PM domain logic, or something else.

>
> The PHY driver is taking its PHY reset in probe and release it in remove().
> With this approach the newly introduced SYSC signal will be
> de-asserted/asserted only in the PHY reset probe/remove (either if it is
> handled though PM domain or reset control signal).
>
> If the SYSC signal would be passed to all the blocks in the USB area (and
> it would be handled though PM domains) it should be no problem either,
> AFAICT, because of reference counting the pm_runtime_get|put*() is taking
> care of. As the PHY reset is the root node the in the devices node tree for
> USB the reference counting should work, too (I may miss something though,
> please correct me if I'm wrong).
>
> If the SYSC signal would be handled though a reset control driver (as
> proposed in this series) and we want to pass this reference to all the
> blocks in the USB area then we can request the reset signal as shared and,
> AFAIK, this is also reference counted. The devices node tree should help
> with the order, too, if I'm not wrong.

Reference counting a reset signal sounds a bit weird to me, but I
guess it can work. :-)

To sum up from my side;

As long as it's fine that we may end up asserting/de-asserting the
reset-signal, without actually knowing if the PM domain is getting
turn-on/off, then using a reset-control like what you propose seems
okay to me.

If not, there are two other options that can be considered I think.
*) Using the genpd on/off notifiers, to really allow the consumer
driver of the reset-control to know when the PM domain gets turned
on/off.
**) Move the entire reset handling into the PM domain provider, as it
obviously knows when the domain is getting turned on/off.

Thanks again for your explanations!

Kind regards
Uffe
Claudiu Beznea Aug. 30, 2024, 11:32 a.m. UTC | #7
On 30.08.2024 13:14, Ulf Hansson wrote:
> On Fri, 30 Aug 2024 at 10:22, claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>>
>> Hi, Ulf,
>>
>> On 29.08.2024 18:26, Ulf Hansson wrote:
>>> On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>>
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> Hi,
>>>>
>>>> Series adds initial USB support for the Renesas RZ/G3S SoC.
>>>>
>>>> Series is split as follows:
>>>>
>>>> - patch 01/16           - add clock reset and power domain support for USB
>>>> - patch 02-04/16        - add reset control support for a USB signal
>>>>                           that need to be controlled before/after
>>>>                           the power to USB area is turned on/off.
>>>>
>>>>                           Philipp, Ulf, Geert, all,
>>>>
>>>>                           I detailed my approach for this in patch
>>>>                           04/16, please have a look and let me know
>>>>                           your input.
>>>
>>> I have looked briefly. Your suggested approach may work, but I have a
>>> few thoughts, see below.
>>>
>>> If I understand correctly, it is the consumer driver for the device
>>> that is attached to the USB power domain that becomes responsible for
>>> asserting/de-asserting this new signal. Right?
>>
>> Right!
>>
>>>
>>> In this regard, please note that the consumer driver doesn't really
>>> know when the power domain really gets powered-on/off. Calling
>>> pm_runtime_get|put*() is dealing with the reference counting. For
>>> example, a call to pm_runtime_get*() just makes sure that the PM
>>> domain gets-or-remains powered-on. Could this be a problem from the
>>> reset-signal point of view?
>>
>> It should be safe. From the HW manual I understand the hardware block is
>> something like the following:
>>
>>
>>                   USB area
>>          +-------------------------+
>>          |                         |
>>          | PHY --->USB controller  |
>> SYSC --> |  ^                      |
>>          |  |                      |
>>          | PHY reset               |
>>          +-------------------------+
>>
>> Where:
>> - SYSC is the system controller that controls the new signal for which
>>   I'm requesting opinions in this series
>> - PHY reset: is the block controlling the PHYs
>> - PHY: is the block controlling the USB PHYs
>> - USB controller: is the USB controller
>>
>> Currently, I passed the SYSC signal handling to the PHY reset driver; w/o
>> PHY reset the rest of the USB logic cannot work (neither PHY block nor USB
>> controller).
>>
>> Currently, the PHY reset driver call pm_runtime_resume_and_get() in probe
>> and pm_runtime_put() in remove. The struct reset_control_ops::{assert,
>> deassert} only set specific bits in registers (no pm_runtime* calls).
> 
> Thanks for clarifying!
> 
> For my understanding, in what register range do these bits belong? Is
> it the USB logic or in the PM domain logic, or something else.

The PHY reset block is an individual hardware block with its own address
space, clocks and reset signals.

The PHY block may reside in the same address space with USB controller but
it can provide PHY support for an external USB controller, something like:

+--------------------------+
|  PHY ---> USB controller |
|   |                      |
+---|----------------------+
   \/
+---------------+
| USB controller|
+---------------+

Because of this the PHY block is modeled in Linux as a standalone driver.

And SYSC is an individual HW block with its own address space. This is
where it resides the control of the signal for which I'm asking for directions.

> 
>>
>> The PHY driver is taking its PHY reset in probe and release it in remove().
>> With this approach the newly introduced SYSC signal will be
>> de-asserted/asserted only in the PHY reset probe/remove (either if it is
>> handled though PM domain or reset control signal).
>>
>> If the SYSC signal would be passed to all the blocks in the USB area (and
>> it would be handled though PM domains) it should be no problem either,
>> AFAICT, because of reference counting the pm_runtime_get|put*() is taking
>> care of. As the PHY reset is the root node the in the devices node tree for
>> USB the reference counting should work, too (I may miss something though,
>> please correct me if I'm wrong).
>>
>> If the SYSC signal would be handled though a reset control driver (as
>> proposed in this series) and we want to pass this reference to all the
>> blocks in the USB area then we can request the reset signal as shared and,
>> AFAIK, this is also reference counted. The devices node tree should help
>> with the order, too, if I'm not wrong.
> 
> Reference counting a reset signal sounds a bit weird to me, but I
> guess it can work. :-)
> 
> To sum up from my side;
> 
> As long as it's fine that we may end up asserting/de-asserting the
> reset-signal, without actually knowing if the PM domain is getting
> turn-on/off, 

With my understanding of it, that should not happen, at least not with the
current implementation of the drivers involved in this.

> then using a reset-control like what you propose seems
> okay to me.

I would prefer this option, too.

> 
> If not, there are two other options that can be considered I think.
> *) Using the genpd on/off notifiers, to really allow the consumer
> driver of the reset-control to know when the PM domain gets turned
> on/off.
> **) Move the entire reset handling into the PM domain provider, as it
> obviously knows when the domain is getting turned on/off.

This option is what I've explored, tested on my side.

I explored it in 2 ways:

1/ SYSC modeled as an individual PM domain provider (this is more
   appropriate to how HW manual described the hardware) with this the PHY
   reset DT node would have to get 2 PM domains handlers (one for the
   current PM domain provider and the other one for SYSC):

+               phyrst: usbphy-ctrl@11e00000 {
+                       compatible = "renesas,r9a08g045-usbphy-ctrl";
+                       reg = <0 0x11e00000 0 0x10000>;
+                       clocks = <&cpg CPG_MOD R9A08G045_USB_PCLK>;
+                       resets = <&cpg R9A08G045_USB_PRESETN>;
+                       power-domain-names = "cpg", "sysc";
+                       power-domains = <&cpg R9A08G045_PD_USB_PHY>, <&sysc
R9A08G045_SYSC_PD_USB>;
+                       #reset-cells = <1>;
+                       status = "disabled";
+
+                       usb0_vbus_otg: regulator-vbus {
+                               regulator-name = "vbus";
+                       };
+               };
+

and the PHY reset driver will get bulky with powering on/off both of these,
at least with my current implementation, something like (and the following
code is in probe()):

+       if (priv->set_power) {
+               priv->cpg_genpd_dev = dev_pm_domain_attach_by_name(dev, "cpg");
+               if (IS_ERR(priv->cpg_genpd_dev)) {
+                       dev_err_probe(dev, error, "Failed to attach CPG PM
domain!");
+                       error = PTR_ERR(priv->cpg_genpd_dev);
+                       goto err_pm_runtime_put;
+               }
+
+               priv->sysc_genpd_dev = dev_pm_domain_attach_by_name(dev,
"sysc");
+               if (IS_ERR(priv->sysc_genpd_dev)) {
+                       dev_err_probe(dev, error, "Failed to attach sysc PM
domain!");
+                       error = PTR_ERR(priv->sysc_genpd_dev);
+                       goto err_genpd_cpg_detach;
+               }
+
+               priv->cpg_genpd_dl = device_link_add(dev, priv->cpg_genpd_dev,
+                                                    DL_FLAG_PM_RUNTIME |
+                                                    DL_FLAG_STATELESS);
+               if (!priv->cpg_genpd_dl) {
+                       dev_err_probe(dev, -ENOMEM, "Failed to add CPG
genpd device link!");
+                       goto err_genpd_sysc_detach;
+               }
+
+               priv->sysc_genpd_dl = device_link_add(dev,
priv->sysc_genpd_dev,
+                                                     DL_FLAG_PM_RUNTIME |
+                                                     DL_FLAG_STATELESS);
+               if (!priv->sysc_genpd_dl) {
+                       dev_err_probe(dev, -ENOMEM, "Failed to add sysc
genpd device link!");
+                       goto err_genpd_cpg_dl_del;
+               }
+
+
+               error = pm_runtime_resume_and_get(priv->cpg_genpd_dev);
+               if (error) {
+                       dev_err_probe(dev, error, "Failed to runtime resume
cpg PM domain!");
+                       goto err_genpd_sysc_dl_del;
+               }
+
+               error = pm_runtime_resume_and_get(priv->sysc_genpd_dev);
+               if (error) {
+                       dev_err_probe(dev, error, "Failed to runtime resume
sysc PM domain!");
+                       goto err_genpd_cpg_off;
+               }
+       }
+

2/ SYSC being a PM domain provider parent of the CPG (current PM domain
   provider). With this the phy reset node is like proposed in this series
   (powered by CPG PM domain):

+               phyrst: usbphy-ctrl@11e00000 {
+                       compatible = "renesas,r9a08g045-usbphy-ctrl",
+                                    "renesas,rzg2l-usbphy-ctrl";
+                       reg = <0 0x11e00000 0 0x10000>;
+                       clocks = <&cpg CPG_MOD R9A08G045_USB_PCLK>;
+                       resets = <&cpg R9A08G045_USB_PRESETN>;
+                       power-domains = <&cpg R9A08G045_PD_USB_PHY>;
+                       #reset-cells = <1>;
+                       status = "disabled";
+
+                       usb0_vbus_otg: regulator-vbus {
+                               regulator-name = "vbus";
+                       };
+               };

And the USB SYSC PM domain is parent for all USB PM domains provided by CPG
(3 in this case). With this there should be some glue code b/w CPG (code in
drivers/clk/renesas/{rzg2l-cpg.c, r9a08g045-cpg.c}) and SYSC drivers (I
have something ugly locally, haven't tried to detach CPG code from SYSC
code at the moment).


> 
> Thanks again for your explanations!

Thank you, also, for looking into this,
Claudiu Beznea

> 
> Kind regards
> Uffe
Biju Das Aug. 31, 2024, 5:13 a.m. UTC | #8
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Friday, August 30, 2024 9:23 AM
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> Hi, Ulf,
> 
> On 29.08.2024 18:26, Ulf Hansson wrote:
> > On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> Hi,
> >>
> >> Series adds initial USB support for the Renesas RZ/G3S SoC.
> >>
> >> Series is split as follows:
> >>
> >> - patch 01/16           - add clock reset and power domain support for USB
> >> - patch 02-04/16        - add reset control support for a USB signal
> >>                           that need to be controlled before/after
> >>                           the power to USB area is turned on/off.
> >>
> >>                           Philipp, Ulf, Geert, all,
> >>
> >>                           I detailed my approach for this in patch
> >>                           04/16, please have a look and let me know
> >>                           your input.
> >
> > I have looked briefly. Your suggested approach may work, but I have a
> > few thoughts, see below.
> >
> > If I understand correctly, it is the consumer driver for the device
> > that is attached to the USB power domain that becomes responsible for
> > asserting/de-asserting this new signal. Right?
> 
> Right!
> 
> >
> > In this regard, please note that the consumer driver doesn't really
> > know when the power domain really gets powered-on/off. Calling
> > pm_runtime_get|put*() is dealing with the reference counting. For
> > example, a call to pm_runtime_get*() just makes sure that the PM
> > domain gets-or-remains powered-on. Could this be a problem from the
> > reset-signal point of view?
> 
> It should be safe. From the HW manual I understand the hardware block is something like the following:
> 
> 
>                   USB area
>          +-------------------------+
>          |                         |
>          | PHY --->USB controller  |
> SYSC --> |  ^                      |
>          |  |                      |
>          | PHY reset               |
>          +-------------------------+

How USB PWRRDY signal is connected to USB? 

USB block consists of PHY control, PHY, USB HOST and USB OTG Controller IPs.

Is it connected to top level block or connected to each IP's for turning off the USB region power?

? Or Just PHY (HW manual mentions for AWO, the USB PWRRDY signal->USB PHY PWRRDY signal control)? 

If the USBPWRRDY signal is connected across modules with this reset signal approach
then you may need to update bindings [1] with that reset signal

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240822152801.602318-12-claudiu.beznea.uj@bp.renesas.com/
 

Cheers,
Biju

> 
> Where:
> - SYSC is the system controller that controls the new signal for which
>   I'm requesting opinions in this series
> - PHY reset: is the block controlling the PHYs
> - PHY: is the block controlling the USB PHYs
> - USB controller: is the USB controller
> 
> Currently, I passed the SYSC signal handling to the PHY reset driver; w/o PHY reset the rest of the
> USB logic cannot work (neither PHY block nor USB controller).
> 
> Currently, the PHY reset driver call pm_runtime_resume_and_get() in probe and pm_runtime_put() in
> remove. The struct reset_control_ops::{assert, deassert} only set specific bits in registers (no
> pm_runtime* calls).
> 
> The PHY driver is taking its PHY reset in probe and release it in remove().
> With this approach the newly introduced SYSC signal will be de-asserted/asserted only in the PHY reset
> probe/remove (either if it is handled though PM domain or reset control signal).
> 
> If the SYSC signal would be passed to all the blocks in the USB area (and it would be handled though
> PM domains) it should be no problem either, AFAICT, because of reference counting the
> pm_runtime_get|put*() is taking care of. As the PHY reset is the root node the in the devices node
> tree for USB the reference counting should work, too (I may miss something though, please correct me
> if I'm wrong).
> 
> If the SYSC signal would be handled though a reset control driver (as proposed in this series) and we
> want to pass this reference to all the blocks in the USB area then we can request the reset signal as
> shared and, AFAIK, this is also reference counted. The devices node tree should help with the order,
> too, if I'm not wrong.
> 
> Thank you for looking at this,
> Claudiu Beznea
> 
> >
> > [...]
> >
> > Kind regards
> > Uffe
Ulf Hansson Aug. 31, 2024, 10:32 a.m. UTC | #9
[...]

> >
> > If not, there are two other options that can be considered I think.
> > *) Using the genpd on/off notifiers, to really allow the consumer
> > driver of the reset-control to know when the PM domain gets turned
> > on/off.
> > **) Move the entire reset handling into the PM domain provider, as it
> > obviously knows when the domain is getting turned on/off.
>
> This option is what I've explored, tested on my side.
>
> I explored it in 2 ways:
>
> 1/ SYSC modeled as an individual PM domain provider (this is more
>    appropriate to how HW manual described the hardware) with this the PHY
>    reset DT node would have to get 2 PM domains handlers (one for the
>    current PM domain provider and the other one for SYSC):
>
> +               phyrst: usbphy-ctrl@11e00000 {
> +                       compatible = "renesas,r9a08g045-usbphy-ctrl";
> +                       reg = <0 0x11e00000 0 0x10000>;
> +                       clocks = <&cpg CPG_MOD R9A08G045_USB_PCLK>;
> +                       resets = <&cpg R9A08G045_USB_PRESETN>;
> +                       power-domain-names = "cpg", "sysc";
> +                       power-domains = <&cpg R9A08G045_PD_USB_PHY>, <&sysc
> R9A08G045_SYSC_PD_USB>;
> +                       #reset-cells = <1>;
> +                       status = "disabled";
> +
> +                       usb0_vbus_otg: regulator-vbus {
> +                               regulator-name = "vbus";
> +                       };
> +               };
> +

According to what you have described earlier/above, modelling the SYSC
as a PM domain provider seems like a better description of the HW to
me. Although, as I said earlier, if you prefer the reset approach, I
would not object to that.

>
> and the PHY reset driver will get bulky with powering on/off both of these,
> at least with my current implementation, something like (and the following
> code is in probe()):
>
> +       if (priv->set_power) {
> +               priv->cpg_genpd_dev = dev_pm_domain_attach_by_name(dev, "cpg");
> +               if (IS_ERR(priv->cpg_genpd_dev)) {
> +                       dev_err_probe(dev, error, "Failed to attach CPG PM
> domain!");
> +                       error = PTR_ERR(priv->cpg_genpd_dev);
> +                       goto err_pm_runtime_put;
> +               }
> +
> +               priv->sysc_genpd_dev = dev_pm_domain_attach_by_name(dev,
> "sysc");
> +               if (IS_ERR(priv->sysc_genpd_dev)) {
> +                       dev_err_probe(dev, error, "Failed to attach sysc PM
> domain!");
> +                       error = PTR_ERR(priv->sysc_genpd_dev);
> +                       goto err_genpd_cpg_detach;
> +               }
> +
> +               priv->cpg_genpd_dl = device_link_add(dev, priv->cpg_genpd_dev,
> +                                                    DL_FLAG_PM_RUNTIME |
> +                                                    DL_FLAG_STATELESS);
> +               if (!priv->cpg_genpd_dl) {
> +                       dev_err_probe(dev, -ENOMEM, "Failed to add CPG
> genpd device link!");
> +                       goto err_genpd_sysc_detach;
> +               }
> +
> +               priv->sysc_genpd_dl = device_link_add(dev,
> priv->sysc_genpd_dev,
> +                                                     DL_FLAG_PM_RUNTIME |
> +                                                     DL_FLAG_STATELESS);
> +               if (!priv->sysc_genpd_dl) {
> +                       dev_err_probe(dev, -ENOMEM, "Failed to add sysc
> genpd device link!");
> +                       goto err_genpd_cpg_dl_del;
> +               }
> +
> +
> +               error = pm_runtime_resume_and_get(priv->cpg_genpd_dev);
> +               if (error) {
> +                       dev_err_probe(dev, error, "Failed to runtime resume
> cpg PM domain!");
> +                       goto err_genpd_sysc_dl_del;
> +               }
> +
> +               error = pm_runtime_resume_and_get(priv->sysc_genpd_dev);
> +               if (error) {
> +                       dev_err_probe(dev, error, "Failed to runtime resume
> sysc PM domain!");
> +                       goto err_genpd_cpg_off;
> +               }
> +       }

Indeed, the code above looks bulky.

Fortunately, we now have dev|devm_pm_domain_attach_list(), which
replaces all of the code above.

[...]

Kind regards
Uffe
Biju Das Sept. 2, 2024, 7:54 a.m. UTC | #10
Hi Claudiu,

> -----Original Message-----
> From: Biju Das
> Sent: Saturday, August 31, 2024 6:14 AM
> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> Hi Claudiu,
> 
> > -----Original Message-----
> > From: claudiu beznea <claudiu.beznea@tuxon.dev>
> > Sent: Friday, August 30, 2024 9:23 AM
> > Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> > RZ/G3S SoC
> >
> > Hi, Ulf,
> >
> > On 29.08.2024 18:26, Ulf Hansson wrote:
> > > On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> > >>
> > >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > >>
> > >> Hi,
> > >>
> > >> Series adds initial USB support for the Renesas RZ/G3S SoC.
> > >>
> > >> Series is split as follows:
> > >>
> > >> - patch 01/16           - add clock reset and power domain support for USB
> > >> - patch 02-04/16        - add reset control support for a USB signal
> > >>                           that need to be controlled before/after
> > >>                           the power to USB area is turned on/off.
> > >>
> > >>                           Philipp, Ulf, Geert, all,
> > >>
> > >>                           I detailed my approach for this in patch
> > >>                           04/16, please have a look and let me know
> > >>                           your input.
> > >
> > > I have looked briefly. Your suggested approach may work, but I have
> > > a few thoughts, see below.
> > >
> > > If I understand correctly, it is the consumer driver for the device
> > > that is attached to the USB power domain that becomes responsible
> > > for asserting/de-asserting this new signal. Right?
> >
> > Right!
> >
> > >
> > > In this regard, please note that the consumer driver doesn't really
> > > know when the power domain really gets powered-on/off. Calling
> > > pm_runtime_get|put*() is dealing with the reference counting. For
> > > example, a call to pm_runtime_get*() just makes sure that the PM
> > > domain gets-or-remains powered-on. Could this be a problem from the
> > > reset-signal point of view?
> >
> > It should be safe. From the HW manual I understand the hardware block is something like the
> following:
> >
> >
> >                   USB area
> >          +-------------------------+
> >          |                         |
> >          | PHY --->USB controller  |
> > SYSC --> |  ^                      |
> >          |  |                      |
> >          | PHY reset               |
> >          +-------------------------+
> 
> How USB PWRRDY signal is connected to USB?
> 
> USB block consists of PHY control, PHY, USB HOST and USB OTG Controller IPs.
> 
> Is it connected to top level block or connected to each IP's for turning off the USB region power?
> 
> ? Or Just PHY (HW manual mentions for AWO, the USB PWRRDY signal->USB PHY PWRRDY signal control)?

As per the update from HW team,

"SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to ALL_ON).

Refer to step 8,9 in Table 41.10 Example Transition Flow Outline from ALL_ON Mode to AWO Mode.
Refer to step 9,10 in Table 41.11 Example Transition Flow Outline from AWO Mode to ALL_ON Mode.

When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break."

Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in linux? 

Cheers,
Biju
Claudiu Beznea Sept. 2, 2024, 8:28 a.m. UTC | #11
On 31.08.2024 08:13, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>> Sent: Friday, August 30, 2024 9:23 AM
>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>> Hi, Ulf,
>>
>> On 29.08.2024 18:26, Ulf Hansson wrote:
>>> On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>>
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> Hi,
>>>>
>>>> Series adds initial USB support for the Renesas RZ/G3S SoC.
>>>>
>>>> Series is split as follows:
>>>>
>>>> - patch 01/16           - add clock reset and power domain support for USB
>>>> - patch 02-04/16        - add reset control support for a USB signal
>>>>                           that need to be controlled before/after
>>>>                           the power to USB area is turned on/off.
>>>>
>>>>                           Philipp, Ulf, Geert, all,
>>>>
>>>>                           I detailed my approach for this in patch
>>>>                           04/16, please have a look and let me know
>>>>                           your input.
>>>
>>> I have looked briefly. Your suggested approach may work, but I have a
>>> few thoughts, see below.
>>>
>>> If I understand correctly, it is the consumer driver for the device
>>> that is attached to the USB power domain that becomes responsible for
>>> asserting/de-asserting this new signal. Right?
>>
>> Right!
>>
>>>
>>> In this regard, please note that the consumer driver doesn't really
>>> know when the power domain really gets powered-on/off. Calling
>>> pm_runtime_get|put*() is dealing with the reference counting. For
>>> example, a call to pm_runtime_get*() just makes sure that the PM
>>> domain gets-or-remains powered-on. Could this be a problem from the
>>> reset-signal point of view?
>>
>> It should be safe. From the HW manual I understand the hardware block is something like the following:
>>
>>
>>                   USB area
>>          +-------------------------+
>>          |                         |
>>          | PHY --->USB controller  |
>> SYSC --> |  ^                      |
>>          |  |                      |
>>          | PHY reset               |
>>          +-------------------------+
> 
> How USB PWRRDY signal is connected to USB? 

HW manual mentions this in the chapter describing the SYS_USB_PWRRDY register:

Controls PWRRDY terminal of USB

0: PWRRDY

1: PWRRDY down

When turning off the *USB region* power, set this bit to 1.

When turning on the *USB region* power, set this bit to 0

By USB region I get the that it is related to the SoC area where resides
all the USB IPs. I cannot tell more than what is in the hardware manual.


> 
> USB block consists of PHY control, PHY, USB HOST and USB OTG Controller IPs.
> 
> Is it connected to top level block or connected to each IP's for turning off the USB region power?

I cannot tell more than it is in the hardware manual.

> 
> ? Or Just PHY (HW manual mentions for AWO, the USB PWRRDY signal->USB PHY PWRRDY signal control)?
> 
> If the USBPWRRDY signal is connected across modules with this reset signal approach
> then you may need to update bindings [1] with that reset signal
> 
> [1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240822152801.602318-12-claudiu.beznea.uj@bp.renesas.com/

If that is true, then this signal may need to be routed to the PHY for
better hardware description.

>  
> 
> Cheers,
> Biju
> 
>>
>> Where:
>> - SYSC is the system controller that controls the new signal for which
>>   I'm requesting opinions in this series
>> - PHY reset: is the block controlling the PHYs
>> - PHY: is the block controlling the USB PHYs
>> - USB controller: is the USB controller
>>
>> Currently, I passed the SYSC signal handling to the PHY reset driver; w/o PHY reset the rest of the
>> USB logic cannot work (neither PHY block nor USB controller).
>>
>> Currently, the PHY reset driver call pm_runtime_resume_and_get() in probe and pm_runtime_put() in
>> remove. The struct reset_control_ops::{assert, deassert} only set specific bits in registers (no
>> pm_runtime* calls).
>>
>> The PHY driver is taking its PHY reset in probe and release it in remove().
>> With this approach the newly introduced SYSC signal will be de-asserted/asserted only in the PHY reset
>> probe/remove (either if it is handled though PM domain or reset control signal).
>>
>> If the SYSC signal would be passed to all the blocks in the USB area (and it would be handled though
>> PM domains) it should be no problem either, AFAICT, because of reference counting the
>> pm_runtime_get|put*() is taking care of. As the PHY reset is the root node the in the devices node
>> tree for USB the reference counting should work, too (I may miss something though, please correct me
>> if I'm wrong).
>>
>> If the SYSC signal would be handled though a reset control driver (as proposed in this series) and we
>> want to pass this reference to all the blocks in the USB area then we can request the reset signal as
>> shared and, AFAIK, this is also reference counted. The devices node tree should help with the order,
>> too, if I'm not wrong.
>>
>> Thank you for looking at this,
>> Claudiu Beznea
>>
>>>
>>> [...]
>>>
>>> Kind regards
>>> Uffe
Claudiu Beznea Sept. 2, 2024, 8:47 a.m. UTC | #12
Hi, Biju,

On 02.09.2024 10:54, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Biju Das
>> Sent: Saturday, August 31, 2024 6:14 AM
>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>> Hi Claudiu,
>>
>>> -----Original Message-----
>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>> Sent: Friday, August 30, 2024 9:23 AM
>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>> RZ/G3S SoC
>>>
>>> Hi, Ulf,
>>>
>>> On 29.08.2024 18:26, Ulf Hansson wrote:
>>>> On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>>>
>>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>
>>>>> Hi,
>>>>>
>>>>> Series adds initial USB support for the Renesas RZ/G3S SoC.
>>>>>
>>>>> Series is split as follows:
>>>>>
>>>>> - patch 01/16           - add clock reset and power domain support for USB
>>>>> - patch 02-04/16        - add reset control support for a USB signal
>>>>>                           that need to be controlled before/after
>>>>>                           the power to USB area is turned on/off.
>>>>>
>>>>>                           Philipp, Ulf, Geert, all,
>>>>>
>>>>>                           I detailed my approach for this in patch
>>>>>                           04/16, please have a look and let me know
>>>>>                           your input.
>>>>
>>>> I have looked briefly. Your suggested approach may work, but I have
>>>> a few thoughts, see below.
>>>>
>>>> If I understand correctly, it is the consumer driver for the device
>>>> that is attached to the USB power domain that becomes responsible
>>>> for asserting/de-asserting this new signal. Right?
>>>
>>> Right!
>>>
>>>>
>>>> In this regard, please note that the consumer driver doesn't really
>>>> know when the power domain really gets powered-on/off. Calling
>>>> pm_runtime_get|put*() is dealing with the reference counting. For
>>>> example, a call to pm_runtime_get*() just makes sure that the PM
>>>> domain gets-or-remains powered-on. Could this be a problem from the
>>>> reset-signal point of view?
>>>
>>> It should be safe. From the HW manual I understand the hardware block is something like the
>> following:
>>>
>>>
>>>                   USB area
>>>          +-------------------------+
>>>          |                         |
>>>          | PHY --->USB controller  |
>>> SYSC --> |  ^                      |
>>>          |  |                      |
>>>          | PHY reset               |
>>>          +-------------------------+
>>
>> How USB PWRRDY signal is connected to USB?
>>
>> USB block consists of PHY control, PHY, USB HOST and USB OTG Controller IPs.
>>
>> Is it connected to top level block or connected to each IP's for turning off the USB region power?
>>
>> ? Or Just PHY (HW manual mentions for AWO, the USB PWRRDY signal->USB PHY PWRRDY signal control)?
> 
> As per the update from HW team,
> 
> "SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to ALL_ON).
> 
> Refer to step 8,9 in Table 41.10 Example Transition Flow Outline from ALL_ON Mode to AWO Mode.
> Refer to step 9,10 in Table 41.11 Example Transition Flow Outline from AWO Mode to ALL_ON Mode.

All this is not new information.
Biju Das Sept. 2, 2024, 8:53 a.m. UTC | #13
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Monday, September 2, 2024 9:47 AM
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> Hi, Biju,
> 
> On 02.09.2024 10:54, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Biju Das
> >> Sent: Saturday, August 31, 2024 6:14 AM
> >> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
> >> RZ/G3S SoC
> >>
> >> Hi Claudiu,
> >>
> >>> -----Original Message-----
> >>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>> Sent: Friday, August 30, 2024 9:23 AM
> >>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >>> RZ/G3S SoC
> >>>
> >>> Hi, Ulf,
> >>>
> >>> On 29.08.2024 18:26, Ulf Hansson wrote:
> >>>> On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >>>>>
> >>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> Series adds initial USB support for the Renesas RZ/G3S SoC.
> >>>>>
> >>>>> Series is split as follows:
> >>>>>
> >>>>> - patch 01/16           - add clock reset and power domain support for USB
> >>>>> - patch 02-04/16        - add reset control support for a USB signal
> >>>>>                           that need to be controlled before/after
> >>>>>                           the power to USB area is turned on/off.
> >>>>>
> >>>>>                           Philipp, Ulf, Geert, all,
> >>>>>
> >>>>>                           I detailed my approach for this in patch
> >>>>>                           04/16, please have a look and let me know
> >>>>>                           your input.
> >>>>
> >>>> I have looked briefly. Your suggested approach may work, but I have
> >>>> a few thoughts, see below.
> >>>>
> >>>> If I understand correctly, it is the consumer driver for the device
> >>>> that is attached to the USB power domain that becomes responsible
> >>>> for asserting/de-asserting this new signal. Right?
> >>>
> >>> Right!
> >>>
> >>>>
> >>>> In this regard, please note that the consumer driver doesn't really
> >>>> know when the power domain really gets powered-on/off. Calling
> >>>> pm_runtime_get|put*() is dealing with the reference counting. For
> >>>> example, a call to pm_runtime_get*() just makes sure that the PM
> >>>> domain gets-or-remains powered-on. Could this be a problem from the
> >>>> reset-signal point of view?
> >>>
> >>> It should be safe. From the HW manual I understand the hardware
> >>> block is something like the
> >> following:
> >>>
> >>>
> >>>                   USB area
> >>>          +-------------------------+
> >>>          |                         |
> >>>          | PHY --->USB controller  |
> >>> SYSC --> |  ^                      |
> >>>          |  |                      |
> >>>          | PHY reset               |
> >>>          +-------------------------+
> >>
> >> How USB PWRRDY signal is connected to USB?
> >>
> >> USB block consists of PHY control, PHY, USB HOST and USB OTG Controller IPs.
> >>
> >> Is it connected to top level block or connected to each IP's for turning off the USB region power?
> >>
> >> ? Or Just PHY (HW manual mentions for AWO, the USB PWRRDY signal->USB PHY PWRRDY signal control)?
> >
> > As per the update from HW team,
> >
> > "SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to
> ALL_ON).
> >
> > Refer to step 8,9 in Table 41.10 Example Transition Flow Outline from ALL_ON Mode to AWO Mode.
> > Refer to step 9,10 in Table 41.11 Example Transition Flow Outline from AWO Mode to ALL_ON Mode.
> 
> All this is not new information.
> 
> From experiments, we need to control these signals also when booting as intermediary booting
> application may control and leave it in improper state. W/o having SYSC signals configured properly
> there is no chance for USB to work (it should be the same for PCIe but I haven't explored it yet).
> 
> >
> > When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break."
> 
> From experiments, I know this, as this is the reason the SYSC USB PWRRDY has been implemented in Linux
> and proposed in this series.

You mean you call reset assert for USB PWRRDY signal for system transition (AWO Mode to ALL_ON Mode)??


 
> >
> > Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in linux?
> 
> As you know, the RZ/G3S USB PM code is already prepared. This is also configuring these signals when
> going to suspend/exiting from resume. W/o configuring properly these signals the USB is not working
> after a suspend/resume cycle.

One option is to handle SYSC USB PWRRDY signal in TF-A, if you plan to handle system transitions there??

Cheers,
Biju
Claudiu Beznea Sept. 2, 2024, 9:14 a.m. UTC | #14
On 02.09.2024 11:53, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>> Sent: Monday, September 2, 2024 9:47 AM
>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>> Hi, Biju,
>>
>> On 02.09.2024 10:54, Biju Das wrote:
>>> Hi Claudiu,
>>>
>>>> -----Original Message-----
>>>> From: Biju Das
>>>> Sent: Saturday, August 31, 2024 6:14 AM
>>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
>>>> RZ/G3S SoC
>>>>
>>>> Hi Claudiu,
>>>>
>>>>> -----Original Message-----
>>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>>> Sent: Friday, August 30, 2024 9:23 AM
>>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>>> RZ/G3S SoC
>>>>>
>>>>> Hi, Ulf,
>>>>>
>>>>> On 29.08.2024 18:26, Ulf Hansson wrote:
>>>>>> On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>>>>>
>>>>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Series adds initial USB support for the Renesas RZ/G3S SoC.
>>>>>>>
>>>>>>> Series is split as follows:
>>>>>>>
>>>>>>> - patch 01/16           - add clock reset and power domain support for USB
>>>>>>> - patch 02-04/16        - add reset control support for a USB signal
>>>>>>>                           that need to be controlled before/after
>>>>>>>                           the power to USB area is turned on/off.
>>>>>>>
>>>>>>>                           Philipp, Ulf, Geert, all,
>>>>>>>
>>>>>>>                           I detailed my approach for this in patch
>>>>>>>                           04/16, please have a look and let me know
>>>>>>>                           your input.
>>>>>>
>>>>>> I have looked briefly. Your suggested approach may work, but I have
>>>>>> a few thoughts, see below.
>>>>>>
>>>>>> If I understand correctly, it is the consumer driver for the device
>>>>>> that is attached to the USB power domain that becomes responsible
>>>>>> for asserting/de-asserting this new signal. Right?
>>>>>
>>>>> Right!
>>>>>
>>>>>>
>>>>>> In this regard, please note that the consumer driver doesn't really
>>>>>> know when the power domain really gets powered-on/off. Calling
>>>>>> pm_runtime_get|put*() is dealing with the reference counting. For
>>>>>> example, a call to pm_runtime_get*() just makes sure that the PM
>>>>>> domain gets-or-remains powered-on. Could this be a problem from the
>>>>>> reset-signal point of view?
>>>>>
>>>>> It should be safe. From the HW manual I understand the hardware
>>>>> block is something like the
>>>> following:
>>>>>
>>>>>
>>>>>                   USB area
>>>>>          +-------------------------+
>>>>>          |                         |
>>>>>          | PHY --->USB controller  |
>>>>> SYSC --> |  ^                      |
>>>>>          |  |                      |
>>>>>          | PHY reset               |
>>>>>          +-------------------------+
>>>>
>>>> How USB PWRRDY signal is connected to USB?
>>>>
>>>> USB block consists of PHY control, PHY, USB HOST and USB OTG Controller IPs.
>>>>
>>>> Is it connected to top level block or connected to each IP's for turning off the USB region power?
>>>>
>>>> ? Or Just PHY (HW manual mentions for AWO, the USB PWRRDY signal->USB PHY PWRRDY signal control)?
>>>
>>> As per the update from HW team,
>>>
>>> "SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to
>> ALL_ON).
>>>
>>> Refer to step 8,9 in Table 41.10 Example Transition Flow Outline from ALL_ON Mode to AWO Mode.
>>> Refer to step 9,10 in Table 41.11 Example Transition Flow Outline from AWO Mode to ALL_ON Mode.
>>
>> All this is not new information.
>>
>> From experiments, we need to control these signals also when booting as intermediary booting
>> application may control and leave it in improper state. W/o having SYSC signals configured properly
>> there is no chance for USB to work (it should be the same for PCIe but I haven't explored it yet).
>>
>>>
>>> When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break."
>>
>> From experiments, I know this, as this is the reason the SYSC USB PWRRDY has been implemented in Linux
>> and proposed in this series.
> 
> You mean you call reset assert for USB PWRRDY signal for system transition (AWO Mode to ALL_ON Mode)??

reset assert on suspend, reset de-assert on resume.

And pm_runtime_put()/pm_runtime_resume_and_get() on the version where USB
PHYRDY is provided by SYSC as a PM domain. As I mentioned in the thread
with Ulf, this is something I explored locally.

All these are internal, not published in this version. This version just
adds initial bring-up support.

> 
> 
>  
>>>
>>> Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in linux?
>>
>> As you know, the RZ/G3S USB PM code is already prepared. This is also configuring these signals when
>> going to suspend/exiting from resume. W/o configuring properly these signals the USB is not working
>> after a suspend/resume cycle.
> 
> One option is to handle SYSC USB PWRRDY signal in TF-A, if you plan to handle system transitions there??

As I mentioned, the settings in these registers may be changed by
intermediary booting applications. Depending on that, Linux need to control
it also on probe for USB to work (it should be the same with PCIe, these
signals seems similar from HW manual description).

Thank you,
Claudiu Beznea

> 
> Cheers,
> Biju
> 
> 
>
Biju Das Sept. 2, 2024, 9:18 a.m. UTC | #15
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Monday, September 2, 2024 10:14 AM
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 02.09.2024 11:53, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >> Sent: Monday, September 2, 2024 9:47 AM
> >> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >> RZ/G3S SoC
> >>
> >> Hi, Biju,
> >>
> >> On 02.09.2024 10:54, Biju Das wrote:
> >>> Hi Claudiu,
> >>>
> >>>> -----Original Message-----
> >>>> From: Biju Das
> >>>> Sent: Saturday, August 31, 2024 6:14 AM
> >>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
> >>>> RZ/G3S SoC
> >>>>
> >>>> Hi Claudiu,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>>>> Sent: Friday, August 30, 2024 9:23 AM
> >>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >>>>> RZ/G3S SoC
> >>>>>
> >>>>> Hi, Ulf,
> >>>>>
> >>>>> On 29.08.2024 18:26, Ulf Hansson wrote:
> >>>>>> On Thu, 22 Aug 2024 at 17:28, Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >>>>>>>
> >>>>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> Series adds initial USB support for the Renesas RZ/G3S SoC.
> >>>>>>>
> >>>>>>> Series is split as follows:
> >>>>>>>
> >>>>>>> - patch 01/16           - add clock reset and power domain support for USB
> >>>>>>> - patch 02-04/16        - add reset control support for a USB signal
> >>>>>>>                           that need to be controlled before/after
> >>>>>>>                           the power to USB area is turned on/off.
> >>>>>>>
> >>>>>>>                           Philipp, Ulf, Geert, all,
> >>>>>>>
> >>>>>>>                           I detailed my approach for this in patch
> >>>>>>>                           04/16, please have a look and let me know
> >>>>>>>                           your input.
> >>>>>>
> >>>>>> I have looked briefly. Your suggested approach may work, but I
> >>>>>> have a few thoughts, see below.
> >>>>>>
> >>>>>> If I understand correctly, it is the consumer driver for the
> >>>>>> device that is attached to the USB power domain that becomes
> >>>>>> responsible for asserting/de-asserting this new signal. Right?
> >>>>>
> >>>>> Right!
> >>>>>
> >>>>>>
> >>>>>> In this regard, please note that the consumer driver doesn't
> >>>>>> really know when the power domain really gets powered-on/off.
> >>>>>> Calling
> >>>>>> pm_runtime_get|put*() is dealing with the reference counting. For
> >>>>>> example, a call to pm_runtime_get*() just makes sure that the PM
> >>>>>> domain gets-or-remains powered-on. Could this be a problem from
> >>>>>> the reset-signal point of view?
> >>>>>
> >>>>> It should be safe. From the HW manual I understand the hardware
> >>>>> block is something like the
> >>>> following:
> >>>>>
> >>>>>
> >>>>>                   USB area
> >>>>>          +-------------------------+
> >>>>>          |                         |
> >>>>>          | PHY --->USB controller  |
> >>>>> SYSC --> |  ^                      |
> >>>>>          |  |                      |
> >>>>>          | PHY reset               |
> >>>>>          +-------------------------+
> >>>>
> >>>> How USB PWRRDY signal is connected to USB?
> >>>>
> >>>> USB block consists of PHY control, PHY, USB HOST and USB OTG Controller IPs.
> >>>>
> >>>> Is it connected to top level block or connected to each IP's for turning off the USB region
> power?
> >>>>
> >>>> ? Or Just PHY (HW manual mentions for AWO, the USB PWRRDY signal->USB PHY PWRRDY signal control)?
> >>>
> >>> As per the update from HW team,
> >>>
> >>> "SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from
> >>> ALL_ON to AWO (or from AWO to
> >> ALL_ON).
> >>>
> >>> Refer to step 8,9 in Table 41.10 Example Transition Flow Outline from ALL_ON Mode to AWO Mode.
> >>> Refer to step 9,10 in Table 41.11 Example Transition Flow Outline from AWO Mode to ALL_ON Mode.
> >>
> >> All this is not new information.
> >>
> >> From experiments, we need to control these signals also when booting
> >> as intermediary booting application may control and leave it in
> >> improper state. W/o having SYSC signals configured properly there is no chance for USB to work (it
> should be the same for PCIe but I haven't explored it yet).
> >>
> >>>
> >>> When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break."
> >>
> >> From experiments, I know this, as this is the reason the SYSC USB
> >> PWRRDY has been implemented in Linux and proposed in this series.
> >
> > You mean you call reset assert for USB PWRRDY signal for system transition (AWO Mode to ALL_ON
> Mode)??
> 
> reset assert on suspend, reset de-assert on resume.
> 
> And pm_runtime_put()/pm_runtime_resume_and_get() on the version where USB PHYRDY is provided by SYSC
> as a PM domain. As I mentioned in the thread with Ulf, this is something I explored locally.
> 
> All these are internal, not published in this version. This version just adds initial bring-up
> support.
> 
> >
> >
> >
> >>>
> >>> Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in linux?
> >>
> >> As you know, the RZ/G3S USB PM code is already prepared. This is also
> >> configuring these signals when going to suspend/exiting from resume.
> >> W/o configuring properly these signals the USB is not working after a suspend/resume cycle.
> >
> > One option is to handle SYSC USB PWRRDY signal in TF-A, if you plan to handle system transitions
> there??
> 
> As I mentioned, the settings in these registers may be changed by intermediary booting applications.
> Depending on that, Linux need to control it also on probe for USB to work (it should be the same with
> PCIe, these signals seems similar from HW manual description).

You mean system transition settings will be override by U-boot, so Linux needs to restore it back??

Cheers,
Biju
Claudiu Beznea Sept. 2, 2024, 10:40 a.m. UTC | #16
On 02.09.2024 12:18, Biju Das wrote:
>>>>> Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in linux?
>>>> As you know, the RZ/G3S USB PM code is already prepared. This is also
>>>> configuring these signals when going to suspend/exiting from resume.
>>>> W/o configuring properly these signals the USB is not working after a suspend/resume cycle.
>>> One option is to handle SYSC USB PWRRDY signal in TF-A, if you plan to handle system transitions
>> there??
>>
>> As I mentioned, the settings in these registers may be changed by intermediary booting applications.
>> Depending on that, Linux need to control it also on probe for USB to work (it should be the same with
>> PCIe, these signals seems similar from HW manual description).
> You mean system transition settings will be override by U-boot, so Linux needs to restore it back??

It was talking about booting...

You proposed to handle SYSC signals from TF-A in a discussion about system
power transitions:

"One option is to handle SYSC USB PWRRDY signal in TF-A,  if you plan to
handle system transitions"

(I was guessing the "system transition" statement there refers to power
states transitions, ALL_ON <-> AWO/VBAT)

and I gave the booting process as a counter example: if we handle it in
TF-A it may not be enough as these signals might be changed by intermediary
booting applications (e.g., U-Boot).

To conclude, there are 3 scenarios I see where these signals need to be
handled:
1/ booting
2/ suspend to RAM
3/ driver unbind/bind

In case of booting: if we have TF-A to set signals there might be
intermediary booting applications (e.g. U-Boot) that set these signals
also. If it leaves it in improper state and Linux wants to use USB then the
USB will not work (if Linux doesn't handle it).

In case of suspend to RAM: as TF-A is the only application in the suspend
to RAM chain, it should work handling it in TF-A.

In case of unbind/bind: currently we don't know if these signals introduces
any kind of power saving so asserting/de-asserting them in Linux may be
useful from this perspective, if any.

Either way, I think Linux should handle all that it can to make the devices
work and not rely on third party applications.

Thank you,
Claudiu Beznea

> 
> Cheers,
> Biju
Biju Das Sept. 2, 2024, 10:47 a.m. UTC | #17
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Monday, September 2, 2024 11:41 AM
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 02.09.2024 12:18, Biju Das wrote:
> >>>>> Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in linux?
> >>>> As you know, the RZ/G3S USB PM code is already prepared. This is
> >>>> also configuring these signals when going to suspend/exiting from resume.
> >>>> W/o configuring properly these signals the USB is not working after a suspend/resume cycle.
> >>> One option is to handle SYSC USB PWRRDY signal in TF-A, if you plan
> >>> to handle system transitions
> >> there??
> >>
> >> As I mentioned, the settings in these registers may be changed by intermediary booting
> applications.
> >> Depending on that, Linux need to control it also on probe for USB to
> >> work (it should be the same with PCIe, these signals seems similar from HW manual description).
> > You mean system transition settings will be override by U-boot, so Linux needs to restore it back??
> 
> It was talking about booting...

I am also referring to boot. Boot starts with TF-A and it has a system state.

> 
> You proposed to handle SYSC signals from TF-A in a discussion about system power transitions:
> 
> "One option is to handle SYSC USB PWRRDY signal in TF-A,  if you plan to handle system transitions"
> 
> (I was guessing the "system transition" statement there refers to power states transitions, ALL_ON <->
> AWO/VBAT)

That is correct.

> 
> and I gave the booting process as a counter example: if we handle it in TF-A it may not be enough as
> these signals might be changed by intermediary booting applications (e.g., U-Boot).

Why should U-boot override, system state signals such as USB PWRREADY? Can you please give an example.

> 
> To conclude, there are 3 scenarios I see where these signals need to be
> handled:
> 1/ booting 
> 2/ suspend to RAM
> 3/ driver unbind/bind

--> It should be OK as linux is not handling USB PWRREADY signal.

> 
> In case of booting: if we have TF-A to set signals there might be intermediary booting applications
> (e.g. U-Boot) that set these signals also. If it leaves it in improper state and Linux wants to use
> USB then the USB will not work (if Linux doesn't handle it).

That is the problem of U-boot. U-boot should not override system state signals such as USB PWRREADY.

> 
> In case of suspend to RAM: as TF-A is the only application in the suspend to RAM chain, it should work
> handling it in TF-A.

That is correct, TF-A should handle based on system state.

> 
> In case of unbind/bind: currently we don't know if these signals introduces any kind of power saving
> so asserting/de-asserting them in Linux may be useful from this perspective, if any.

These are system signals, according to me should not be used in unbind/bind.

I may be wrong.

Cheers,
Biju
Claudiu Beznea Sept. 3, 2024, 10:25 a.m. UTC | #18
On 03.09.2024 10:18, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Biju Das
>> Sent: Monday, September 2, 2024 11:48 AM
>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>> Hi Claudiu,
>>
>>> -----Original Message-----
>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>> Sent: Monday, September 2, 2024 11:41 AM
>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>> RZ/G3S SoC
>>>
>>>
>>>
>>> On 02.09.2024 12:18, Biju Das wrote:
>>>>>>>> Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in linux?
>>>>>>> As you know, the RZ/G3S USB PM code is already prepared. This is
>>>>>>> also configuring these signals when going to suspend/exiting from resume.
>>>>>>> W/o configuring properly these signals the USB is not working after a suspend/resume cycle.
>>>>>> One option is to handle SYSC USB PWRRDY signal in TF-A, if you
>>>>>> plan to handle system transitions
>>>>> there??
>>>>>
>>>>> As I mentioned, the settings in these registers may be changed by
>>>>> intermediary booting
>>> applications.
>>>>> Depending on that, Linux need to control it also on probe for USB
>>>>> to work (it should be the same with PCIe, these signals seems similar from HW manual
>> description).
>>>> You mean system transition settings will be override by U-boot, so Linux needs to restore it
>> back??
>>>
>>> It was talking about booting...
>>
>> I am also referring to boot. Boot starts with TF-A and it has a system state.
>>
>>>
>>> You proposed to handle SYSC signals from TF-A in a discussion about system power transitions:
>>>
>>> "One option is to handle SYSC USB PWRRDY signal in TF-A,  if you plan to handle system transitions"
>>>
>>> (I was guessing the "system transition" statement there refers to
>>> power states transitions, ALL_ON <->
>>> AWO/VBAT)
>>
>> That is correct.
>>
>>>
>>> and I gave the booting process as a counter example: if we handle it
>>> in TF-A it may not be enough as these signals might be changed by intermediary booting applications
>> (e.g., U-Boot).
>>
>> Why should U-boot override, system state signals such as USB PWRREADY? Can you please give an example.
>>
>>>
>>> To conclude, there are 3 scenarios I see where these signals need to
>>> be
>>> handled:
>>> 1/ booting
>>> 2/ suspend to RAM
>>> 3/ driver unbind/bind
>>
>> --> It should be OK as linux is not handling USB PWRREADY signal.
>>
>>>
>>> In case of booting: if we have TF-A to set signals there might be
>>> intermediary booting applications (e.g. U-Boot) that set these signals
>>> also. If it leaves it in improper state and Linux wants to use USB then the USB will not work (if
>> Linux doesn't handle it).
>>
>> That is the problem of U-boot. U-boot should not override system state signals such as USB PWRREADY.
>>
>>>
>>> In case of suspend to RAM: as TF-A is the only application in the
>>> suspend to RAM chain, it should work handling it in TF-A.
>>
>> That is correct, TF-A should handle based on system state.
>>
>>>
>>> In case of unbind/bind: currently we don't know if these signals
>>> introduces any kind of power saving so asserting/de-asserting them in Linux may be useful from this
>> perspective, if any.
>>
>> These are system signals, according to me should not be used in unbind/bind.
>>
>> I may be wrong.
> 
> Just to add the below are the 4 system states (power mode) for this LSI.
> 
> If I understand correctly, we need to configure USB PWRRDY signal
> only when there is a transition from ALL_ON to AWO mode and vice versa.
> as you see on AWO mode only CM-33 is active.
> 
> • ALL_OFF mode: All CPUs and peripheral modules can not be worked.
> • ALL_ON mode: All CPUs and peripheral modules can be worked.
> • AWO mode: Cortex-M33 and peripheral modules in PD_VCC and PD_VBATT domain can be worked.
> • VBATT mode: Only RTC, tamper detection and backup registers can be worked.
> 
> System manager which is controlling both CA-55 and CM-33, will set USB PWRRDY signal
> based on system state.
> 
> Since we don't have system manager for controlling both CA-55 and CM-33
> Probably from CA-55 perspective, TF-A should be sufficient.
> 
> During boot clr USB PWR READY signal in TF-A.
> STR case, suspend set USB PWR READY signal in TF-A.
> STR case, resume clr USB PWR READY signal in TF-A.

As I said previously, it can be done in different ways. My point was to let
Linux set what it needs for all it's devices to work. I think the way to go
forward is a maintainer decision.

Thank you,
Claudiu Beznea

> 
> I am not expert in this area. I may be wrong.
> 
> Maybe we need to get an opinion from Power experts in Linux.
> 
> Cheers,
> Biju
Biju Das Sept. 3, 2024, 10:31 a.m. UTC | #19
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 11:25 AM
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 03.09.2024 10:18, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: Biju Das
> >> Sent: Monday, September 2, 2024 11:48 AM
> >> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
> >> RZ/G3S SoC
> >>
> >> Hi Claudiu,
> >>
> >>> -----Original Message-----
> >>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>> Sent: Monday, September 2, 2024 11:41 AM
> >>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >>> RZ/G3S SoC
> >>>
> >>>
> >>>
> >>> On 02.09.2024 12:18, Biju Das wrote:
> >>>>>>>> Do you have any plan to control this power transitions(ALL_ON to AWO and vice versa) in
> linux?
> >>>>>>> As you know, the RZ/G3S USB PM code is already prepared. This is
> >>>>>>> also configuring these signals when going to suspend/exiting from resume.
> >>>>>>> W/o configuring properly these signals the USB is not working after a suspend/resume cycle.
> >>>>>> One option is to handle SYSC USB PWRRDY signal in TF-A, if you
> >>>>>> plan to handle system transitions
> >>>>> there??
> >>>>>
> >>>>> As I mentioned, the settings in these registers may be changed by
> >>>>> intermediary booting
> >>> applications.
> >>>>> Depending on that, Linux need to control it also on probe for USB
> >>>>> to work (it should be the same with PCIe, these signals seems
> >>>>> similar from HW manual
> >> description).
> >>>> You mean system transition settings will be override by U-boot, so
> >>>> Linux needs to restore it
> >> back??
> >>>
> >>> It was talking about booting...
> >>
> >> I am also referring to boot. Boot starts with TF-A and it has a system state.
> >>
> >>>
> >>> You proposed to handle SYSC signals from TF-A in a discussion about system power transitions:
> >>>
> >>> "One option is to handle SYSC USB PWRRDY signal in TF-A,  if you plan to handle system
> transitions"
> >>>
> >>> (I was guessing the "system transition" statement there refers to
> >>> power states transitions, ALL_ON <->
> >>> AWO/VBAT)
> >>
> >> That is correct.
> >>
> >>>
> >>> and I gave the booting process as a counter example: if we handle it
> >>> in TF-A it may not be enough as these signals might be changed by
> >>> intermediary booting applications
> >> (e.g., U-Boot).
> >>
> >> Why should U-boot override, system state signals such as USB PWRREADY? Can you please give an
> example.
> >>
> >>>
> >>> To conclude, there are 3 scenarios I see where these signals need to
> >>> be
> >>> handled:
> >>> 1/ booting
> >>> 2/ suspend to RAM
> >>> 3/ driver unbind/bind
> >>
> >> --> It should be OK as linux is not handling USB PWRREADY signal.
> >>
> >>>
> >>> In case of booting: if we have TF-A to set signals there might be
> >>> intermediary booting applications (e.g. U-Boot) that set these
> >>> signals also. If it leaves it in improper state and Linux wants to
> >>> use USB then the USB will not work (if
> >> Linux doesn't handle it).
> >>
> >> That is the problem of U-boot. U-boot should not override system state signals such as USB
> PWRREADY.
> >>
> >>>
> >>> In case of suspend to RAM: as TF-A is the only application in the
> >>> suspend to RAM chain, it should work handling it in TF-A.
> >>
> >> That is correct, TF-A should handle based on system state.
> >>
> >>>
> >>> In case of unbind/bind: currently we don't know if these signals
> >>> introduces any kind of power saving so asserting/de-asserting them
> >>> in Linux may be useful from this
> >> perspective, if any.
> >>
> >> These are system signals, according to me should not be used in unbind/bind.
> >>
> >> I may be wrong.
> >
> > Just to add the below are the 4 system states (power mode) for this LSI.
> >
> > If I understand correctly, we need to configure USB PWRRDY signal only
> > when there is a transition from ALL_ON to AWO mode and vice versa.
> > as you see on AWO mode only CM-33 is active.
> >
> > • ALL_OFF mode: All CPUs and peripheral modules can not be worked.
> > • ALL_ON mode: All CPUs and peripheral modules can be worked.
> > • AWO mode: Cortex-M33 and peripheral modules in PD_VCC and PD_VBATT domain can be worked.
> > • VBATT mode: Only RTC, tamper detection and backup registers can be worked.
> >
> > System manager which is controlling both CA-55 and CM-33, will set USB
> > PWRRDY signal based on system state.
> >
> > Since we don't have system manager for controlling both CA-55 and
> > CM-33 Probably from CA-55 perspective, TF-A should be sufficient.
> >
> > During boot clr USB PWR READY signal in TF-A.
> > STR case, suspend set USB PWR READY signal in TF-A.
> > STR case, resume clr USB PWR READY signal in TF-A.
> 
> As I said previously, it can be done in different ways. My point was to let Linux set what it needs
> for all it's devices to work. I think the way to go forward is a maintainer decision.


I agree, there can be n number of solution for a problem.

Since you modelled system state signal (USB PWRRDY) as reset control signal, it is reset/DT maintainer's decision
to say the final word whether this signal fits in reset system framework or not?

Cheers,
biju
Claudiu Beznea Sept. 3, 2024, 10:58 a.m. UTC | #20
On 03.09.2024 13:35, Ulf Hansson wrote:
> On Sat, 31 Aug 2024 at 12:32, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>>
>> [...]
>>
>>>>
>>>> If not, there are two other options that can be considered I think.
>>>> *) Using the genpd on/off notifiers, to really allow the consumer
>>>> driver of the reset-control to know when the PM domain gets turned
>>>> on/off.
>>>> **) Move the entire reset handling into the PM domain provider, as it
>>>> obviously knows when the domain is getting turned on/off.
>>>
>>> This option is what I've explored, tested on my side.
>>>
>>> I explored it in 2 ways:
>>>
>>> 1/ SYSC modeled as an individual PM domain provider (this is more
>>>    appropriate to how HW manual described the hardware) with this the PHY
>>>    reset DT node would have to get 2 PM domains handlers (one for the
>>>    current PM domain provider and the other one for SYSC):
>>>
>>> +               phyrst: usbphy-ctrl@11e00000 {
>>> +                       compatible = "renesas,r9a08g045-usbphy-ctrl";
>>> +                       reg = <0 0x11e00000 0 0x10000>;
>>> +                       clocks = <&cpg CPG_MOD R9A08G045_USB_PCLK>;
>>> +                       resets = <&cpg R9A08G045_USB_PRESETN>;
>>> +                       power-domain-names = "cpg", "sysc";
>>> +                       power-domains = <&cpg R9A08G045_PD_USB_PHY>, <&sysc
>>> R9A08G045_SYSC_PD_USB>;
>>> +                       #reset-cells = <1>;
>>> +                       status = "disabled";
>>> +
>>> +                       usb0_vbus_otg: regulator-vbus {
>>> +                               regulator-name = "vbus";
>>> +                       };
>>> +               };
>>> +
>>
>> According to what you have described earlier/above, modelling the SYSC
>> as a PM domain provider seems like a better description of the HW to
>> me. Although, as I said earlier, if you prefer the reset approach, I
>> would not object to that.
> 
> Following the discussion I believe I should take this back. If I
> understand correctly, SYSC signal seems best to be modelled as a
> reset.
> 
>  Although, it looks like the USB PM domain provider should rather be
> the consumer of that reset, instead of having the reset being consumed
> by the consumers of the USB PM domain.

The PM domain provider for USB is the provider for the rest of IPs. To work
like this the SYSC these signals should be handled in the USB domains power
on/off function. It's not impossible to have it implemented like this but
it will complicate a bit the code, AFAICT. This will not describe the
hardware, also.

With the information that we had up to yesterday, the connection b/w HW
blocks was something as follows:

                 USB area
              +--------------------------+
      sig     | PHY -> USB controller X  |
SYSC -------->|  ^                       |
              |  |                       |
              | PHY reset                |
              +--------------------------+

In this implementation the SYSC signal was connected to PHY reset block as
it is the root of the devices used in the USB setup and no USB
functionality can exist w/o the PHY reset being setup.

There is a new information arrived just yesterday from hardware team saying
this about SYSC signals: "When turning off USB PHY and PCIe PHY, if they
are not controlled, PHY may break" which may means that it is just
connected to the PHYs not to the USB area/region or PCIe area/region as
initially expressed in HW manual.

With that the HW connection b/w the USB devices and SYSC might become
something like:

                 USB area
              +--------------------------+
     sig   +--->PHY -> USB controller X  |
SYSC ------+  |  ^                       |
              |  |                       |
              | PHY reset                |
              +--------------------------+

I haven't got the chance to test this topology, though.

With this new information would you be OK to still have it as a reset
signal and connected only to the PHY driver ?

Thank you,
Claudiu Beznea

> 
> Did that make sense?
> 
> [...]
> 
> Kind regards
> Uffe
Biju Das Sept. 3, 2024, 11:07 a.m. UTC | #21
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 12:00 PM
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 03.09.2024 13:31, Biju Das wrote:
> >>> During boot clr USB PWR READY signal in TF-A.
> >>> STR case, suspend set USB PWR READY signal in TF-A.
> >>> STR case, resume clr USB PWR READY signal in TF-A.
> >> As I said previously, it can be done in different ways. My point was
> >> to let Linux set what it needs for all it's devices to work. I think the way to go forward is a
> maintainer decision.
> >
> > I agree, there can be n number of solution for a problem.
> >
> > Since you modelled system state signal (USB PWRRDY) as reset control
> > signal, it is reset/DT maintainer's decision to say the final word whether this signal fits in reset
> system framework or not?
> 
> I was thinking:
> 1/ Geert would be the best to say if he considers it OK to handle this
>    in Linux

I agree Geert is the right person for taking SYSTEM decisions,
since the signal is used only during state transitions
(Table 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)


> 2/ if OK, then we should get approval from Ulf and Philipp on the power
>    domain or reset approaches

Ok.

Cheers,
Biju
Ulf Hansson Sept. 3, 2024, 11:50 a.m. UTC | #22
On Tue, 3 Sept 2024 at 12:58, claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>
>
>
> On 03.09.2024 13:35, Ulf Hansson wrote:
> > On Sat, 31 Aug 2024 at 12:32, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >>
> >> [...]
> >>
> >>>>
> >>>> If not, there are two other options that can be considered I think.
> >>>> *) Using the genpd on/off notifiers, to really allow the consumer
> >>>> driver of the reset-control to know when the PM domain gets turned
> >>>> on/off.
> >>>> **) Move the entire reset handling into the PM domain provider, as it
> >>>> obviously knows when the domain is getting turned on/off.
> >>>
> >>> This option is what I've explored, tested on my side.
> >>>
> >>> I explored it in 2 ways:
> >>>
> >>> 1/ SYSC modeled as an individual PM domain provider (this is more
> >>>    appropriate to how HW manual described the hardware) with this the PHY
> >>>    reset DT node would have to get 2 PM domains handlers (one for the
> >>>    current PM domain provider and the other one for SYSC):
> >>>
> >>> +               phyrst: usbphy-ctrl@11e00000 {
> >>> +                       compatible = "renesas,r9a08g045-usbphy-ctrl";
> >>> +                       reg = <0 0x11e00000 0 0x10000>;
> >>> +                       clocks = <&cpg CPG_MOD R9A08G045_USB_PCLK>;
> >>> +                       resets = <&cpg R9A08G045_USB_PRESETN>;
> >>> +                       power-domain-names = "cpg", "sysc";
> >>> +                       power-domains = <&cpg R9A08G045_PD_USB_PHY>, <&sysc
> >>> R9A08G045_SYSC_PD_USB>;
> >>> +                       #reset-cells = <1>;
> >>> +                       status = "disabled";
> >>> +
> >>> +                       usb0_vbus_otg: regulator-vbus {
> >>> +                               regulator-name = "vbus";
> >>> +                       };
> >>> +               };
> >>> +
> >>
> >> According to what you have described earlier/above, modelling the SYSC
> >> as a PM domain provider seems like a better description of the HW to
> >> me. Although, as I said earlier, if you prefer the reset approach, I
> >> would not object to that.
> >
> > Following the discussion I believe I should take this back. If I
> > understand correctly, SYSC signal seems best to be modelled as a
> > reset.
> >
> >  Although, it looks like the USB PM domain provider should rather be
> > the consumer of that reset, instead of having the reset being consumed
> > by the consumers of the USB PM domain.
>
> The PM domain provider for USB is the provider for the rest of IPs. To work
> like this the SYSC these signals should be handled in the USB domains power
> on/off function. It's not impossible to have it implemented like this but
> it will complicate a bit the code, AFAICT. This will not describe the
> hardware, also.
>
> With the information that we had up to yesterday, the connection b/w HW
> blocks was something as follows:
>
>                  USB area
>               +--------------------------+
>       sig     | PHY -> USB controller X  |
> SYSC -------->|  ^                       |
>               |  |                       |
>               | PHY reset                |
>               +--------------------------+
>
> In this implementation the SYSC signal was connected to PHY reset block as
> it is the root of the devices used in the USB setup and no USB
> functionality can exist w/o the PHY reset being setup.
>
> There is a new information arrived just yesterday from hardware team saying
> this about SYSC signals: "When turning off USB PHY and PCIe PHY, if they
> are not controlled, PHY may break" which may means that it is just
> connected to the PHYs not to the USB area/region or PCIe area/region as
> initially expressed in HW manual.
>
> With that the HW connection b/w the USB devices and SYSC might become
> something like:
>
>                  USB area
>               +--------------------------+
>      sig   +--->PHY -> USB controller X  |
> SYSC ------+  |  ^                       |
>               |  |                       |
>               | PHY reset                |
>               +--------------------------+
>
> I haven't got the chance to test this topology, though.
>
> With this new information would you be OK to still have it as a reset
> signal and connected only to the PHY driver ?

As long as it's a better description of the HW, I am fine with that too.

Although, please note that pm_runtime_get|put() doesn't give you full
controll of how the USB PM domain is being powered. So in that case,
it sounds like you need to use the genpd on/off notifiers too.

Kind regards
Uffe
Biju Das Sept. 3, 2024, noon UTC | #23
> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Tuesday, September 3, 2024 12:07 PM
> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson <ulf.hansson@linaro.org>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> p.zabel@pengutronix.de; geert+renesas@glider.be; magnus.damm@gmail.com; gregkh@linuxfoundation.org;
> mturquette@baylibre.com; sboyd@kernel.org; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
> linux-phy@lists.infradead.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org; linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> Hi Claudiu,
> 
> > -----Original Message-----
> > From: claudiu beznea <claudiu.beznea@tuxon.dev>
> > Sent: Tuesday, September 3, 2024 12:00 PM
> > Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> > RZ/G3S SoC
> >
> >
> >
> > On 03.09.2024 13:31, Biju Das wrote:
> > >>> During boot clr USB PWR READY signal in TF-A.
> > >>> STR case, suspend set USB PWR READY signal in TF-A.
> > >>> STR case, resume clr USB PWR READY signal in TF-A.
> > >> As I said previously, it can be done in different ways. My point
> > >> was to let Linux set what it needs for all it's devices to work. I
> > >> think the way to go forward is a
> > maintainer decision.
> > >
> > > I agree, there can be n number of solution for a problem.
> > >
> > > Since you modelled system state signal (USB PWRRDY) as reset control
> > > signal, it is reset/DT maintainer's decision to say the final word
> > > whether this signal fits in reset
> > system framework or not?
> >
> > I was thinking:
> > 1/ Geert would be the best to say if he considers it OK to handle this
> >    in Linux
> 
> I agree Geert is the right person for taking SYSTEM decisions, since the signal is used only during
> state transitions (Table 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)

One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.

All clocks/reset happens after setting USB PWRRDY signal

https://pasteboard.co/qbz021q7KPyi.png

Cheers,
Biju
Claudiu Beznea Sept. 3, 2024, 12:25 p.m. UTC | #24
On 03.09.2024 15:00, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: Biju Das <biju.das.jz@bp.renesas.com>
>> Sent: Tuesday, September 3, 2024 12:07 PM
>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
>> p.zabel@pengutronix.de; geert+renesas@glider.be; magnus.damm@gmail.com; gregkh@linuxfoundation.org;
>> mturquette@baylibre.com; sboyd@kernel.org; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
>> renesas-soc@vger.kernel.org; linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>> Hi Claudiu,
>>
>>> -----Original Message-----
>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>> Sent: Tuesday, September 3, 2024 12:00 PM
>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>> RZ/G3S SoC
>>>
>>>
>>>
>>> On 03.09.2024 13:31, Biju Das wrote:
>>>>>> During boot clr USB PWR READY signal in TF-A.
>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
>>>>> As I said previously, it can be done in different ways. My point
>>>>> was to let Linux set what it needs for all it's devices to work. I
>>>>> think the way to go forward is a
>>> maintainer decision.
>>>>
>>>> I agree, there can be n number of solution for a problem.
>>>>
>>>> Since you modelled system state signal (USB PWRRDY) as reset control
>>>> signal, it is reset/DT maintainer's decision to say the final word
>>>> whether this signal fits in reset
>>> system framework or not?
>>>
>>> I was thinking:
>>> 1/ Geert would be the best to say if he considers it OK to handle this
>>>    in Linux
>>
>> I agree Geert is the right person for taking SYSTEM decisions, since the signal is used only during
>> state transitions (Table 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
> 
> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.

The "controlled by" column mentions CA-55 on PWRRDY signal control line and
it is b/w steps "DDR exits from retention mode" and  "clock start settings
for system bus and peripheral modules". AFAICT, after DDR exists retention
mode Linux is ready to run.

E.g. on resume Linux doesn't sets the clocks of all peripheral in sequence
and then runs the rest of settings for each peripheral, in turn it sets the
clock of one peripheral along with all the other necessary peripheral
settings and then continues with the rest of peripherals (including their
clocks).

> 
> All clocks/reset happens after setting USB PWRRDY signal
> 
> https://pasteboard.co/qbz021q7KPyi.png
> 
> Cheers,
> Biju
Biju Das Sept. 3, 2024, 12:37 p.m. UTC | #25
> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 1:26 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson <ulf.hansson@linaro.org>
> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> p.zabel@pengutronix.de; geert+renesas@glider.be; magnus.damm@gmail.com; gregkh@linuxfoundation.org;
> mturquette@baylibre.com; sboyd@kernel.org; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
> linux-phy@lists.infradead.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org; linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 03.09.2024 15:00, Biju Das wrote:
> >
> >
> >> -----Original Message-----
> >> From: Biju Das <biju.das.jz@bp.renesas.com>
> >> Sent: Tuesday, September 3, 2024 12:07 PM
> >> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
> >> <ulf.hansson@linaro.org>
> >> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> >> geert+renesas@glider.be; magnus.damm@gmail.com;
> >> gregkh@linuxfoundation.org; mturquette@baylibre.com;
> >> sboyd@kernel.org; Yoshihiro Shimoda
> >> <yoshihiro.shimoda.uh@renesas.com>;
> >> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> >> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
> >> <claudiu.beznea.uj@bp.renesas.com>
> >> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
> >> RZ/G3S SoC
> >>
> >> Hi Claudiu,
> >>
> >>> -----Original Message-----
> >>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>> Sent: Tuesday, September 3, 2024 12:00 PM
> >>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >>> RZ/G3S SoC
> >>>
> >>>
> >>>
> >>> On 03.09.2024 13:31, Biju Das wrote:
> >>>>>> During boot clr USB PWR READY signal in TF-A.
> >>>>>> STR case, suspend set USB PWR READY signal in TF-A.
> >>>>>> STR case, resume clr USB PWR READY signal in TF-A.
> >>>>> As I said previously, it can be done in different ways. My point
> >>>>> was to let Linux set what it needs for all it's devices to work. I
> >>>>> think the way to go forward is a
> >>> maintainer decision.
> >>>>
> >>>> I agree, there can be n number of solution for a problem.
> >>>>
> >>>> Since you modelled system state signal (USB PWRRDY) as reset
> >>>> control signal, it is reset/DT maintainer's decision to say the
> >>>> final word whether this signal fits in reset
> >>> system framework or not?
> >>>
> >>> I was thinking:
> >>> 1/ Geert would be the best to say if he considers it OK to handle this
> >>>    in Linux
> >>
> >> I agree Geert is the right person for taking SYSTEM decisions, since
> >> the signal is used only during state transitions (Table 41.6.4 AWO to
> >> ALL_ON and 41.6.3 ALL_ON to AWO)
> >
> > One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
> 
> The "controlled by" column mentions CA-55 on PWRRDY signal control line and it is b/w steps "DDR exits
> from retention mode" and  "clock start settings for system bus and peripheral modules". AFAICT, after
> DDR exists retention mode Linux is ready to run.

DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2 checks for warm or cold reset.
If it is warm reset, it sets required minimal clocks/resets and pass the control to linux by calling the
SMC callback handler. Which in turn calls resume(step 11-->14) path.

Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting for exiting form DDR retention mode
Step 9, Cortex-A55 USB PHY PWRRDY signal control (if use USB) SYS_USB_PWRRDY
Step 10, Cortex-A55 PCIe RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B
Step 11, Cortex-A55 Clock start setting for system bus and desired peripheral modules in PD_ISOVCC CPG_CLKON_***ep 
(***: GIC600, MHU, SDHI, USB, ETH, DDR, PCI,AXI_COM_BUS, PERI_COM, AXI_TZCDDR,
OTFDE_DDR)
Step 12, Cortex-A55 Release reset setting for system bus and desired peripheral modules in PD_ISOVCC CPG_RST_***
(***: GIC600, MHU, SDHI, USB, ETH, DDR, PCI, AXI_COM_BUS, PERI_COM, AXI_TZCDDR,OTFDE_DDR)
Step 13, Cortex-A55 Release MSTOP bit for system bus and desired peripheral modules in PD_ISOVCC CPG_BUS_***_MSTOP
(***: ACPU, PERI_COM, PERI_DDR, TZCDDR),
CPG_MHU_MSTOP.
Step14) Cortex-A55 Clock start setting and reset release setting for Cortex-M33_FPU (if use Cortex-M33_FPU)
CPG_CLKON_CM33, CPG_RST_CM33

Cheers,
Biju
Claudiu Beznea Sept. 3, 2024, 12:57 p.m. UTC | #26
On 03.09.2024 15:37, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>> Sent: Tuesday, September 3, 2024 1:26 PM
>> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
>> p.zabel@pengutronix.de; geert+renesas@glider.be; magnus.damm@gmail.com; gregkh@linuxfoundation.org;
>> mturquette@baylibre.com; sboyd@kernel.org; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>;
>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
>> renesas-soc@vger.kernel.org; linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
>> clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>>
>>
>> On 03.09.2024 15:00, Biju Das wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Biju Das <biju.das.jz@bp.renesas.com>
>>>> Sent: Tuesday, September 3, 2024 12:07 PM
>>>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
>>>> <ulf.hansson@linaro.org>
>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
>>>> sboyd@kernel.org; Yoshihiro Shimoda
>>>> <yoshihiro.shimoda.uh@renesas.com>;
>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
>>>> <claudiu.beznea.uj@bp.renesas.com>
>>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
>>>> RZ/G3S SoC
>>>>
>>>> Hi Claudiu,
>>>>
>>>>> -----Original Message-----
>>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>>> Sent: Tuesday, September 3, 2024 12:00 PM
>>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>>> RZ/G3S SoC
>>>>>
>>>>>
>>>>>
>>>>> On 03.09.2024 13:31, Biju Das wrote:
>>>>>>>> During boot clr USB PWR READY signal in TF-A.
>>>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
>>>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
>>>>>>> As I said previously, it can be done in different ways. My point
>>>>>>> was to let Linux set what it needs for all it's devices to work. I
>>>>>>> think the way to go forward is a
>>>>> maintainer decision.
>>>>>>
>>>>>> I agree, there can be n number of solution for a problem.
>>>>>>
>>>>>> Since you modelled system state signal (USB PWRRDY) as reset
>>>>>> control signal, it is reset/DT maintainer's decision to say the
>>>>>> final word whether this signal fits in reset
>>>>> system framework or not?
>>>>>
>>>>> I was thinking:
>>>>> 1/ Geert would be the best to say if he considers it OK to handle this
>>>>>    in Linux
>>>>
>>>> I agree Geert is the right person for taking SYSTEM decisions, since
>>>> the signal is used only during state transitions (Table 41.6.4 AWO to
>>>> ALL_ON and 41.6.3 ALL_ON to AWO)
>>>
>>> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
>>
>> The "controlled by" column mentions CA-55 on PWRRDY signal control line and it is b/w steps "DDR exits
>> from retention mode" and  "clock start settings for system bus and peripheral modules". AFAICT, after
>> DDR exists retention mode Linux is ready to run.
> 
> DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2 checks for warm or cold reset.
> If it is warm reset, it sets required minimal clocks/resets and pass the control to linux by calling the
> SMC callback handler. Which in turn calls resume(step 11-->14) path.

Is this from HW manual or some specific documentation? I'm referring at
"resume" == "steps 11-->14"

> 
> Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting for exiting form DDR retention mode
> Step 9, Cortex-A55 USB PHY PWRRDY signal control (if use USB) SYS_USB_PWRRDY
> Step 10, Cortex-A55 PCIe RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B

Note *if use*: how does the TF-A know if USB/PCIe is used by Linux? The
documentation mention to set it *if use*. Same note is on ALL_ON to VBATT
transition documentation (namely "if using USB", "if using PCIe"). If TF-A
will do this it should set this signals unconditionally. It will not be
something wrong though. We don't know at the moment what this involves in
terms of power consumption, if it means something...

> Step 11, Cortex-A55 Clock start setting for system bus and desired peripheral modules in PD_ISOVCC CPG_CLKON_***ep 
> (***: GIC600, MHU, SDHI, USB, ETH, DDR, PCI,AXI_COM_BUS, PERI_COM, AXI_TZCDDR,
> OTFDE_DDR)
> Step 12, Cortex-A55 Release reset setting for system bus and desired peripheral modules in PD_ISOVCC CPG_RST_***
> (***: GIC600, MHU, SDHI, USB, ETH, DDR, PCI, AXI_COM_BUS, PERI_COM, AXI_TZCDDR,OTFDE_DDR)
> Step 13, Cortex-A55 Release MSTOP bit for system bus and desired peripheral modules in PD_ISOVCC CPG_BUS_***_MSTOP
> (***: ACPU, PERI_COM, PERI_DDR, TZCDDR),
> CPG_MHU_MSTOP.
> Step14) Cortex-A55 Clock start setting and reset release setting for Cortex-M33_FPU (if use Cortex-M33_FPU)
> CPG_CLKON_CM33, CPG_RST_CM33
> 
> Cheers,
> Biju
Biju Das Sept. 3, 2024, 1:09 p.m. UTC | #27
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 1:57 PM
-
> clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 03.09.2024 15:37, Biju Das wrote:
> >
> >
> >> -----Original Message-----
> >> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >> Sent: Tuesday, September 3, 2024 1:26 PM
> >> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson
> >> <ulf.hansson@linaro.org>
> >> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> >> geert+renesas@glider.be; magnus.damm@gmail.com;
> >> gregkh@linuxfoundation.org; mturquette@baylibre.com;
> >> sboyd@kernel.org; Yoshihiro Shimoda
> >> <yoshihiro.shimoda.uh@renesas.com>;
> >> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> >> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
> >> <claudiu.beznea.uj@bp.renesas.com>
> >> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >> RZ/G3S SoC
> >>
> >>
> >>
> >> On 03.09.2024 15:00, Biju Das wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Biju Das <biju.das.jz@bp.renesas.com>
> >>>> Sent: Tuesday, September 3, 2024 12:07 PM
> >>>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
> >>>> <ulf.hansson@linaro.org>
> >>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> >>>> geert+renesas@glider.be; magnus.damm@gmail.com;
> >>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
> >>>> sboyd@kernel.org; Yoshihiro Shimoda
> >>>> <yoshihiro.shimoda.uh@renesas.com>;
> >>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> >>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
> >>>> Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
> >>>> RZ/G3S SoC
> >>>>
> >>>> Hi Claudiu,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>>>> Sent: Tuesday, September 3, 2024 12:00 PM
> >>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >>>>> RZ/G3S SoC
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 03.09.2024 13:31, Biju Das wrote:
> >>>>>>>> During boot clr USB PWR READY signal in TF-A.
> >>>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
> >>>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
> >>>>>>> As I said previously, it can be done in different ways. My point
> >>>>>>> was to let Linux set what it needs for all it's devices to work.
> >>>>>>> I think the way to go forward is a
> >>>>> maintainer decision.
> >>>>>>
> >>>>>> I agree, there can be n number of solution for a problem.
> >>>>>>
> >>>>>> Since you modelled system state signal (USB PWRRDY) as reset
> >>>>>> control signal, it is reset/DT maintainer's decision to say the
> >>>>>> final word whether this signal fits in reset
> >>>>> system framework or not?
> >>>>>
> >>>>> I was thinking:
> >>>>> 1/ Geert would be the best to say if he considers it OK to handle this
> >>>>>    in Linux
> >>>>
> >>>> I agree Geert is the right person for taking SYSTEM decisions,
> >>>> since the signal is used only during state transitions (Table
> >>>> 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
> >>>
> >>> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
> >>
> >> The "controlled by" column mentions CA-55 on PWRRDY signal control
> >> line and it is b/w steps "DDR exits from retention mode" and  "clock
> >> start settings for system bus and peripheral modules". AFAICT, after DDR exists retention mode
> Linux is ready to run.
> >
> > DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2
> checks for warm or cold reset.
> > If it is warm reset, it sets required minimal clocks/resets and pass
> > the control to linux by calling the SMC callback handler. Which in turn calls resume(step 11-->14)
> path.
> 
> Is this from HW manual or some specific documentation? I'm referring at "resume" == "steps 11-->14"
> 
> >
> > Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting
> > for exiting form DDR retention mode Step 9, Cortex-A55 USB PHY PWRRDY
> > signal control (if use USB) SYS_USB_PWRRDY Step 10, Cortex-A55 PCIe
> > RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B
> 
> Note *if use*: how does the TF-A know if USB/PCIe is used by Linux? The documentation mention to set
> it *if use*. Same note is on ALL_ON to VBATT transition documentation (namely "if using USB", "if
> using PCIe"). If TF-A will do this it should set this signals unconditionally. It will not be
> something wrong though. We don't know at the moment what this involves in terms of power consumption,
> if it means something...

You mean, you modelled this as reset signal just to reduce power consumption by calling runtime PM
calls to turn on/off this signal??

Does will it have any system stability issue as hardware manual says to do it at very early stage
before starting any clocks/resets?? Have you checked with hardware team?

Cheers,
Biju
Biju Das Sept. 3, 2024, 1:45 p.m. UTC | #28
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 1:57 PM
> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 03.09.2024 15:37, Biju Das wrote:
> >
> >
> >> -----Original Message-----
> >> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >> Sent: Tuesday, September 3, 2024 1:26 PM
> >> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson
> >> <ulf.hansson@linaro.org>
> >> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> >> geert+renesas@glider.be; magnus.damm@gmail.com;
> >> gregkh@linuxfoundation.org; mturquette@baylibre.com;
> >> sboyd@kernel.org; Yoshihiro Shimoda
> >> <yoshihiro.shimoda.uh@renesas.com>;
> >> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> >> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
> >> <claudiu.beznea.uj@bp.renesas.com>
> >> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >> RZ/G3S SoC
> >>
> >>
> >>
> >> On 03.09.2024 15:00, Biju Das wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Biju Das <biju.das.jz@bp.renesas.com>
> >>>> Sent: Tuesday, September 3, 2024 12:07 PM
> >>>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
> >>>> <ulf.hansson@linaro.org>
> >>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> >>>> geert+renesas@glider.be; magnus.damm@gmail.com;
> >>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
> >>>> sboyd@kernel.org; Yoshihiro Shimoda
> >>>> <yoshihiro.shimoda.uh@renesas.com>;
> >>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> >>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
> >>>> Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
> >>>> RZ/G3S SoC
> >>>>
> >>>> Hi Claudiu,
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>>>> Sent: Tuesday, September 3, 2024 12:00 PM
> >>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >>>>> RZ/G3S SoC
> >>>>>
> >>>>>
> >>>>>
> >>>>> On 03.09.2024 13:31, Biju Das wrote:
> >>>>>>>> During boot clr USB PWR READY signal in TF-A.
> >>>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
> >>>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
> >>>>>>> As I said previously, it can be done in different ways. My point
> >>>>>>> was to let Linux set what it needs for all it's devices to work.
> >>>>>>> I think the way to go forward is a
> >>>>> maintainer decision.
> >>>>>>
> >>>>>> I agree, there can be n number of solution for a problem.
> >>>>>>
> >>>>>> Since you modelled system state signal (USB PWRRDY) as reset
> >>>>>> control signal, it is reset/DT maintainer's decision to say the
> >>>>>> final word whether this signal fits in reset
> >>>>> system framework or not?
> >>>>>
> >>>>> I was thinking:
> >>>>> 1/ Geert would be the best to say if he considers it OK to handle this
> >>>>>    in Linux
> >>>>
> >>>> I agree Geert is the right person for taking SYSTEM decisions,
> >>>> since the signal is used only during state transitions (Table
> >>>> 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
> >>>
> >>> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
> >>
> >> The "controlled by" column mentions CA-55 on PWRRDY signal control
> >> line and it is b/w steps "DDR exits from retention mode" and  "clock
> >> start settings for system bus and peripheral modules". AFAICT, after DDR exists retention mode
> Linux is ready to run.
> >
> > DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2
> checks for warm or cold reset.
> > If it is warm reset, it sets required minimal clocks/resets and pass
> > the control to linux by calling the SMC callback handler. Which in turn calls resume(step 11-->14)
> path.
> 
> Is this from HW manual or some specific documentation? I'm referring at "resume" == "steps 11-->14"
> 
> >
> > Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting
> > for exiting form DDR retention mode Step 9, Cortex-A55 USB PHY PWRRDY
> > signal control (if use USB) SYS_USB_PWRRDY Step 10, Cortex-A55 PCIe
> > RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B
> 
> Note *if use*: how does the TF-A know if USB/PCIe is used by Linux? The documentation mention to set
> it *if use*. Same note is on ALL_ON to VBATT transition documentation (namely "if using USB", "if
> using PCIe"). If TF-A will do this it should set this signals unconditionally. It will not be
> something wrong though. We don't know at the moment what this involves in terms of power consumption,
> if it means something...

IIUC,
The only information we have is,

"SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to ALL_ON).
"When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break"

ALL_ON to AWO_MODE state transition: 
USB/PCIe are part of PD_ISOVCC power domain and before turning PD_ISOVCC to off,
we need to set USBPWRRDY signal.

AWO_MODE to ALL_ON state transition:

Turn on PD_ISOVCC first, then clr USBPWRRDY signal for USB usage in linux.

Maybe we need to ask hw team, exact usage of USBPWRRDY signal other than state transition.

Cheers,
Biju
Claudiu Beznea Sept. 3, 2024, 2:46 p.m. UTC | #29
On 03.09.2024 16:09, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>> Sent: Tuesday, September 3, 2024 1:57 PM
> -
>> clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>>
>>
>> On 03.09.2024 15:37, Biju Das wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>> Sent: Tuesday, September 3, 2024 1:26 PM
>>>> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson
>>>> <ulf.hansson@linaro.org>
>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
>>>> sboyd@kernel.org; Yoshihiro Shimoda
>>>> <yoshihiro.shimoda.uh@renesas.com>;
>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
>>>> <claudiu.beznea.uj@bp.renesas.com>
>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>> RZ/G3S SoC
>>>>
>>>>
>>>>
>>>> On 03.09.2024 15:00, Biju Das wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>> Sent: Tuesday, September 3, 2024 12:07 PM
>>>>>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
>>>>>> <ulf.hansson@linaro.org>
>>>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>>>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
>>>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
>>>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
>>>>>> sboyd@kernel.org; Yoshihiro Shimoda
>>>>>> <yoshihiro.shimoda.uh@renesas.com>;
>>>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
>>>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
>>>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
>>>>>> Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
>>>>>> RZ/G3S SoC
>>>>>>
>>>>>> Hi Claudiu,
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>>>>> Sent: Tuesday, September 3, 2024 12:00 PM
>>>>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>>>>> RZ/G3S SoC
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 03.09.2024 13:31, Biju Das wrote:
>>>>>>>>>> During boot clr USB PWR READY signal in TF-A.
>>>>>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
>>>>>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
>>>>>>>>> As I said previously, it can be done in different ways. My point
>>>>>>>>> was to let Linux set what it needs for all it's devices to work.
>>>>>>>>> I think the way to go forward is a
>>>>>>> maintainer decision.
>>>>>>>>
>>>>>>>> I agree, there can be n number of solution for a problem.
>>>>>>>>
>>>>>>>> Since you modelled system state signal (USB PWRRDY) as reset
>>>>>>>> control signal, it is reset/DT maintainer's decision to say the
>>>>>>>> final word whether this signal fits in reset
>>>>>>> system framework or not?
>>>>>>>
>>>>>>> I was thinking:
>>>>>>> 1/ Geert would be the best to say if he considers it OK to handle this
>>>>>>>    in Linux
>>>>>>
>>>>>> I agree Geert is the right person for taking SYSTEM decisions,
>>>>>> since the signal is used only during state transitions (Table
>>>>>> 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
>>>>>
>>>>> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
>>>>
>>>> The "controlled by" column mentions CA-55 on PWRRDY signal control
>>>> line and it is b/w steps "DDR exits from retention mode" and  "clock
>>>> start settings for system bus and peripheral modules". AFAICT, after DDR exists retention mode
>> Linux is ready to run.
>>>
>>> DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2
>> checks for warm or cold reset.
>>> If it is warm reset, it sets required minimal clocks/resets and pass
>>> the control to linux by calling the SMC callback handler. Which in turn calls resume(step 11-->14)
>> path.
>>
>> Is this from HW manual or some specific documentation? I'm referring at "resume" == "steps 11-->14"

You branched the discussion, there was at least this question that I've
asked you above that interested me.

>>
>>>
>>> Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting
>>> for exiting form DDR retention mode Step 9, Cortex-A55 USB PHY PWRRDY
>>> signal control (if use USB) SYS_USB_PWRRDY Step 10, Cortex-A55 PCIe
>>> RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B
>>
>> Note *if use*: how does the TF-A know if USB/PCIe is used by Linux? The documentation mention to set
>> it *if use*. Same note is on ALL_ON to VBATT transition documentation (namely "if using USB", "if
>> using PCIe"). If TF-A will do this it should set this signals unconditionally. It will not be
>> something wrong though. We don't know at the moment what this involves in terms of power consumption,
>> if it means something...
> 
> You mean, you modelled this as reset signal just to reduce power consumption by calling runtime PM
> calls to turn on/off this signal??

In this series it is though a reset control driver.

The internal BSP propose the control of this signal though SMC calls in
each individual USB driver; I think the hardware team was checked for this;
I may be wrong, as I don't have this insight.

As you know, the initial control of these bits in the BSP was though SMC
calls and you propose to have a separate Linux driver to control this after
finding that these registers are accessible in normal world. As a result,
this series, with reset approach, which you were against, but I felt this
was the best way (I know) to describe the hardware and the relation b/w
hardware blocks. To conclude, you initially proposed me internally to have
it in Linux.

To answer your question, the answer is no, I didn't try to just model
something fancy just to be fancy. I did it based on what is proposed in BSP
as this may have been checked with hardware team and I did tests around
this. And considering this best describes the HW and the relation b/w
individual hardware blocks and in this way Linux can have at its hand all
the resources it needs w/o relying on third parties. And from the HW manual
description my understanding was that this is possible. I never said that
this solution is the best. I'm just adding information here as I requested
help from maintainers to guide on the proper direction.

You were adding information to sustain your TF-A idea, too.

> 
> Does will it have any system stability issue as hardware manual says to do it at very early stage
> before starting any clocks/resets?? Have you checked with hardware team?

All the implementation of this is based on what has been proposed on BSP,
the same approach was proposed there, meaning the control of these signals
was done on probe/remove, suspend/resume in Linux.


> 
> Cheers,
> Biju
Claudiu Beznea Sept. 3, 2024, 2:48 p.m. UTC | #30
On 03.09.2024 16:45, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>> Sent: Tuesday, September 3, 2024 1:57 PM
>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>
>>
>>
>> On 03.09.2024 15:37, Biju Das wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>> Sent: Tuesday, September 3, 2024 1:26 PM
>>>> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson
>>>> <ulf.hansson@linaro.org>
>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
>>>> sboyd@kernel.org; Yoshihiro Shimoda
>>>> <yoshihiro.shimoda.uh@renesas.com>;
>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
>>>> <claudiu.beznea.uj@bp.renesas.com>
>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>> RZ/G3S SoC
>>>>
>>>>
>>>>
>>>> On 03.09.2024 15:00, Biju Das wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>> Sent: Tuesday, September 3, 2024 12:07 PM
>>>>>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
>>>>>> <ulf.hansson@linaro.org>
>>>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>>>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
>>>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
>>>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
>>>>>> sboyd@kernel.org; Yoshihiro Shimoda
>>>>>> <yoshihiro.shimoda.uh@renesas.com>;
>>>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
>>>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
>>>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
>>>>>> Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
>>>>>> RZ/G3S SoC
>>>>>>
>>>>>> Hi Claudiu,
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>>>>> Sent: Tuesday, September 3, 2024 12:00 PM
>>>>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>>>>> RZ/G3S SoC
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 03.09.2024 13:31, Biju Das wrote:
>>>>>>>>>> During boot clr USB PWR READY signal in TF-A.
>>>>>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
>>>>>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
>>>>>>>>> As I said previously, it can be done in different ways. My point
>>>>>>>>> was to let Linux set what it needs for all it's devices to work.
>>>>>>>>> I think the way to go forward is a
>>>>>>> maintainer decision.
>>>>>>>>
>>>>>>>> I agree, there can be n number of solution for a problem.
>>>>>>>>
>>>>>>>> Since you modelled system state signal (USB PWRRDY) as reset
>>>>>>>> control signal, it is reset/DT maintainer's decision to say the
>>>>>>>> final word whether this signal fits in reset
>>>>>>> system framework or not?
>>>>>>>
>>>>>>> I was thinking:
>>>>>>> 1/ Geert would be the best to say if he considers it OK to handle this
>>>>>>>    in Linux
>>>>>>
>>>>>> I agree Geert is the right person for taking SYSTEM decisions,
>>>>>> since the signal is used only during state transitions (Table
>>>>>> 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
>>>>>
>>>>> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
>>>>
>>>> The "controlled by" column mentions CA-55 on PWRRDY signal control
>>>> line and it is b/w steps "DDR exits from retention mode" and  "clock
>>>> start settings for system bus and peripheral modules". AFAICT, after DDR exists retention mode
>> Linux is ready to run.
>>>
>>> DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2
>> checks for warm or cold reset.
>>> If it is warm reset, it sets required minimal clocks/resets and pass
>>> the control to linux by calling the SMC callback handler. Which in turn calls resume(step 11-->14)
>> path.
>>
>> Is this from HW manual or some specific documentation? I'm referring at "resume" == "steps 11-->14"
>>
>>>
>>> Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting
>>> for exiting form DDR retention mode Step 9, Cortex-A55 USB PHY PWRRDY
>>> signal control (if use USB) SYS_USB_PWRRDY Step 10, Cortex-A55 PCIe
>>> RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B
>>
>> Note *if use*: how does the TF-A know if USB/PCIe is used by Linux? The documentation mention to set
>> it *if use*. Same note is on ALL_ON to VBATT transition documentation (namely "if using USB", "if
>> using PCIe"). If TF-A will do this it should set this signals unconditionally. It will not be
>> something wrong though. We don't know at the moment what this involves in terms of power consumption,
>> if it means something...
> 
> IIUC,
> The only information we have is,
> 
> "SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to ALL_ON).
> "When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break"
> 
> ALL_ON to AWO_MODE state transition: 
> USB/PCIe are part of PD_ISOVCC power domain and before turning PD_ISOVCC to off,
> we need to set USBPWRRDY signal.
> 
> AWO_MODE to ALL_ON state transition:
> 
> Turn on PD_ISOVCC first, then clr USBPWRRDY signal for USB usage in linux.
> 
> Maybe we need to ask hw team, exact usage of USBPWRRDY signal other than state transition.

As you may already know, this is open for quite some time and is ongoing.

> 
> Cheers,
> Biju
> 
>
Biju Das Sept. 3, 2024, 3:30 p.m. UTC | #31
Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> Sent: Tuesday, September 3, 2024 3:46 PM


> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> 
> 
> 
> On 03.09.2024 16:09, Biju Das wrote:
> > Hi Claudiu,
> >
> >> -----Original Message-----
> >> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >> Sent: Tuesday, September 3, 2024 1:57 PM
> > -
> >> clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
> >> <claudiu.beznea.uj@bp.renesas.com>
> >> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >> RZ/G3S SoC
> >>
> >>
> >>
> >> On 03.09.2024 15:37, Biju Das wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>>> Sent: Tuesday, September 3, 2024 1:26 PM
> >>>> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson
> >>>> <ulf.hansson@linaro.org>
> >>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> >>>> geert+renesas@glider.be; magnus.damm@gmail.com;
> >>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
> >>>> sboyd@kernel.org; Yoshihiro Shimoda
> >>>> <yoshihiro.shimoda.uh@renesas.com>;
> >>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> >>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
> >>>> Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> >>>> RZ/G3S SoC
> >>>>
> >>>>
> >>>>
> >>>> On 03.09.2024 15:00, Biju Das wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Biju Das <biju.das.jz@bp.renesas.com>
> >>>>>> Sent: Tuesday, September 3, 2024 12:07 PM
> >>>>>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
> >>>>>> <ulf.hansson@linaro.org>
> >>>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> >>>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> >>>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
> >>>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
> >>>>>> sboyd@kernel.org; Yoshihiro Shimoda
> >>>>>> <yoshihiro.shimoda.uh@renesas.com>;
> >>>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> >>>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> >>>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >>>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
> >>>>>> Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>>>> Subject: RE: [PATCH 00/16] Add initial USB support for the
> >>>>>> Renesas RZ/G3S SoC
> >>>>>>
> >>>>>> Hi Claudiu,
> >>>>>>
> >>>>>>> -----Original Message-----
> >>>>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
> >>>>>>> Sent: Tuesday, September 3, 2024 12:00 PM
> >>>>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the
> >>>>>>> Renesas RZ/G3S SoC
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> On 03.09.2024 13:31, Biju Das wrote:
> >>>>>>>>>> During boot clr USB PWR READY signal in TF-A.
> >>>>>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
> >>>>>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
> >>>>>>>>> As I said previously, it can be done in different ways. My
> >>>>>>>>> point was to let Linux set what it needs for all it's devices to work.
> >>>>>>>>> I think the way to go forward is a
> >>>>>>> maintainer decision.
> >>>>>>>>
> >>>>>>>> I agree, there can be n number of solution for a problem.
> >>>>>>>>
> >>>>>>>> Since you modelled system state signal (USB PWRRDY) as reset
> >>>>>>>> control signal, it is reset/DT maintainer's decision to say the
> >>>>>>>> final word whether this signal fits in reset
> >>>>>>> system framework or not?
> >>>>>>>
> >>>>>>> I was thinking:
> >>>>>>> 1/ Geert would be the best to say if he considers it OK to handle this
> >>>>>>>    in Linux
> >>>>>>
> >>>>>> I agree Geert is the right person for taking SYSTEM decisions,
> >>>>>> since the signal is used only during state transitions (Table
> >>>>>> 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
> >>>>>
> >>>>> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
> >>>>
> >>>> The "controlled by" column mentions CA-55 on PWRRDY signal control
> >>>> line and it is b/w steps "DDR exits from retention mode" and
> >>>> "clock start settings for system bus and peripheral modules".
> >>>> AFAICT, after DDR exists retention mode
> >> Linux is ready to run.
> >>>
> >>> DDR retention exit happens in TF-A and it jumps into reset code
> >>> where it executes BL2 in TF_A. Bl2
> >> checks for warm or cold reset.
> >>> If it is warm reset, it sets required minimal clocks/resets and pass
> >>> the control to linux by calling the SMC callback handler. Which in
> >>> turn calls resume(step 11-->14)
> >> path.
> >>
> >> Is this from HW manual or some specific documentation? I'm referring at "resume" == "steps 11-->14"
> 
> You branched the discussion, there was at least this question that I've asked you above that
> interested me.

if there is DDR retention exit involved,
Step 11->14 is just like resume path, for each IP it need clock , reset and restore registers from DDR backup.

> 
> >>
> >>>
> >>> Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting
> >>> for exiting form DDR retention mode Step 9, Cortex-A55 USB PHY
> >>> PWRRDY signal control (if use USB) SYS_USB_PWRRDY Step 10,
> >>> Cortex-A55 PCIe RST_RSM_B signal control (if use PCIe)
> >>> SYS_PCIE_RST_RSM_B
> >>
> >> Note *if use*: how does the TF-A know if USB/PCIe is used by Linux?
> >> The documentation mention to set it *if use*. Same note is on ALL_ON
> >> to VBATT transition documentation (namely "if using USB", "if using
> >> PCIe"). If TF-A will do this it should set this signals
> >> unconditionally. It will not be something wrong though. We don't know at the moment what this
> involves in terms of power consumption, if it means something...
> >
> > You mean, you modelled this as reset signal just to reduce power
> > consumption by calling runtime PM calls to turn on/off this signal??
> 
> In this series it is though a reset control driver.
> 
> The internal BSP propose the control of this signal though SMC calls in each individual USB driver; I
> think the hardware team was checked for this; I may be wrong, as I don't have this insight.
> 
> As you know, the initial control of these bits in the BSP was though SMC calls and you propose to have
> a separate Linux driver to control this after finding that these registers are accessible in normal
> world. As a result, this series, with reset approach, which you were against, but I felt this was the
> best way (I know) to describe the hardware and the relation b/w hardware blocks. To conclude, you
> initially proposed me internally to have it in Linux.

I agree, I proposed to have Linux SYSC driver instead of calling SMC calls from PHY control driver(Reset)

Currently we explored 4 approaches to avoid SMC calls from PHY control driver.

1) modelling this as reset signal
2) power domain approach 
3) set this in SYSC driver during early boot.
4) handle this in TF-A.

> 
> To answer your question, the answer is no, I didn't try to just model something fancy just to be
> fancy. I did it based on what is proposed in BSP as this may have been checked with hardware team and
> I did tests around this. And considering this best describes the HW and the relation b/w individual
> hardware blocks and in this way Linux can have at its hand all the resources it needs w/o relying on
> third parties. And from the HW manual description my understanding was that this is possible. I never
> said that this solution is the best. I'm just adding information here as I requested help from
> maintainers to guide on the proper direction.
> 
> You were adding information to sustain your TF-A idea, too.

This is just an option based on the hardware manual tables where SYSC signal is turned on 
just after DDR retention exit and before starting the clocks/resets.

> 
> >
> > Does will it have any system stability issue as hardware manual says
> > to do it at very early stage before starting any clocks/resets?? Have you checked with hardware
> team?
> 
> All the implementation of this is based on what has been proposed on BSP, the same approach was
> proposed there, meaning the control of these signals was done on probe/remove, suspend/resume in
> Linux.

Based on Hardware manual state transition tables, USBPWRRDY signal is set before starting clocks/reset of the IPs.
If there is no stability issue with BSP approach, then there is no harm in what you have implemented.

Cheers,
Biju
Geert Uytterhoeven Sept. 24, 2024, 11:32 a.m. UTC | #32
Hi Claudiu,

On Thu, Aug 22, 2024 at 5:28 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> The RZ/G3S SYS Controller has 2 registers (one for PCIE one for USB) that
> need to be configured before/after powering off/on the PCI or USB
> ares. The bits in these registers control signals to PCIE and USB that
> need to be de-asserted/asserted after/before power on/off event. For this
> add SYSC controller driver that registers a reset controller driver on
> auxiliary bus which allows USB, PCIE drivers to control these signals.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/reset/reset-rzg3s-sysc.c
> @@ -0,0 +1,140 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G3S SYSC reset driver
> + *
> + * Copyright (C) 2024 Renesas Electronics Corp.
> + */
> +
> +#include <linux/auxiliary_bus.h>

Using the Auxiliary Bus requires selecting AUXILIARY_BUS.
Elsse you might run into build failures:

aarch64-linux-gnu-ld: drivers/soc/renesas/rzg3s-sysc.o: in function
`rzg3s_sysc_probe':
rzg3s-sysc.c:(.text+0x21c): undefined reference to `auxiliary_device_init'
aarch64-linux-gnu-ld: rzg3s-sysc.c:(.text+0x264): undefined reference
to `__auxiliary_device_add'
aarch64-linux-gnu-ld: drivers/reset/reset-rzg3s-sysc.o: in function
`rzg3s_sysc_reset_driver_init':
reset-rzg3s-sysc.c:(.init.text+0x1c): undefined reference to
`__auxiliary_driver_register'
aarch64-linux-gnu-ld: drivers/reset/reset-rzg3s-sysc.o: in function
`rzg3s_sysc_reset_driver_exit':
reset-rzg3s-sysc.c:(.exit.text+0x10): undefined reference to
`auxiliary_driver_unregister'

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Oct. 8, 2024, 1:54 p.m. UTC | #33
Hi Claudiu,

On Wed, Sep 25, 2024 at 9:50 AM claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
> On 24.09.2024 14:32, Geert Uytterhoeven wrote:
> > On Thu, Aug 22, 2024 at 5:28 PM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> The RZ/G3S SYS Controller has 2 registers (one for PCIE one for USB) that
> >> need to be configured before/after powering off/on the PCI or USB
> >> ares. The bits in these registers control signals to PCIE and USB that
> >> need to be de-asserted/asserted after/before power on/off event. For this
> >> add SYSC controller driver that registers a reset controller driver on
> >> auxiliary bus which allows USB, PCIE drivers to control these signals.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> >> --- /dev/null
> >> +++ b/drivers/reset/reset-rzg3s-sysc.c
> >> @@ -0,0 +1,140 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Renesas RZ/G3S SYSC reset driver
> >> + *
> >> + * Copyright (C) 2024 Renesas Electronics Corp.
> >> + */
> >> +
> >> +#include <linux/auxiliary_bus.h>
> >
> > Using the Auxiliary Bus requires selecting AUXILIARY_BUS.
>
> Thank you for pointing it. I'll adjust it in the next version, if it will
> be one.

Thanks, the rest LGTM.

Gr{oetje,eeting}s,

                        Geert
Claudiu Beznea Nov. 7, 2024, 10 a.m. UTC | #34
Hi, all,

On 03.09.2024 17:48, claudiu beznea wrote:
> 
> 
> On 03.09.2024 16:45, Biju Das wrote:
>> Hi Claudiu,
>>
>>> -----Original Message-----
>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>> Sent: Tuesday, September 3, 2024 1:57 PM
>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
>>>
>>>
>>>
>>> On 03.09.2024 15:37, Biju Das wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>>> Sent: Tuesday, September 3, 2024 1:26 PM
>>>>> To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson
>>>>> <ulf.hansson@linaro.org>
>>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
>>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
>>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
>>>>> sboyd@kernel.org; Yoshihiro Shimoda
>>>>> <yoshihiro.shimoda.uh@renesas.com>;
>>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
>>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
>>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
>>>>> <claudiu.beznea.uj@bp.renesas.com>
>>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>>> RZ/G3S SoC
>>>>>
>>>>>
>>>>>
>>>>> On 03.09.2024 15:00, Biju Das wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>>> Sent: Tuesday, September 3, 2024 12:07 PM
>>>>>>> To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
>>>>>>> <ulf.hansson@linaro.org>
>>>>>>> Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
>>>>>>> krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
>>>>>>> geert+renesas@glider.be; magnus.damm@gmail.com;
>>>>>>> gregkh@linuxfoundation.org; mturquette@baylibre.com;
>>>>>>> sboyd@kernel.org; Yoshihiro Shimoda
>>>>>>> <yoshihiro.shimoda.uh@renesas.com>;
>>>>>>> linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
>>>>>>> linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
>>>>>>> linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>>>>>>> linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
>>>>>>> Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>>> Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
>>>>>>> RZ/G3S SoC
>>>>>>>
>>>>>>> Hi Claudiu,
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: claudiu beznea <claudiu.beznea@tuxon.dev>
>>>>>>>> Sent: Tuesday, September 3, 2024 12:00 PM
>>>>>>>> Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
>>>>>>>> RZ/G3S SoC
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 03.09.2024 13:31, Biju Das wrote:
>>>>>>>>>>> During boot clr USB PWR READY signal in TF-A.
>>>>>>>>>>> STR case, suspend set USB PWR READY signal in TF-A.
>>>>>>>>>>> STR case, resume clr USB PWR READY signal in TF-A.
>>>>>>>>>> As I said previously, it can be done in different ways. My point
>>>>>>>>>> was to let Linux set what it needs for all it's devices to work.
>>>>>>>>>> I think the way to go forward is a
>>>>>>>> maintainer decision.
>>>>>>>>>
>>>>>>>>> I agree, there can be n number of solution for a problem.
>>>>>>>>>
>>>>>>>>> Since you modelled system state signal (USB PWRRDY) as reset
>>>>>>>>> control signal, it is reset/DT maintainer's decision to say the
>>>>>>>>> final word whether this signal fits in reset
>>>>>>>> system framework or not?
>>>>>>>>
>>>>>>>> I was thinking:
>>>>>>>> 1/ Geert would be the best to say if he considers it OK to handle this
>>>>>>>>    in Linux
>>>>>>>
>>>>>>> I agree Geert is the right person for taking SYSTEM decisions,
>>>>>>> since the signal is used only during state transitions (Table
>>>>>>> 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
>>>>>>
>>>>>> One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
>>>>>
>>>>> The "controlled by" column mentions CA-55 on PWRRDY signal control
>>>>> line and it is b/w steps "DDR exits from retention mode" and  "clock
>>>>> start settings for system bus and peripheral modules". AFAICT, after DDR exists retention mode
>>> Linux is ready to run.
>>>>
>>>> DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2
>>> checks for warm or cold reset.
>>>> If it is warm reset, it sets required minimal clocks/resets and pass
>>>> the control to linux by calling the SMC callback handler. Which in turn calls resume(step 11-->14)
>>> path.
>>>
>>> Is this from HW manual or some specific documentation? I'm referring at "resume" == "steps 11-->14"
>>>
>>>>
>>>> Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting
>>>> for exiting form DDR retention mode Step 9, Cortex-A55 USB PHY PWRRDY
>>>> signal control (if use USB) SYS_USB_PWRRDY Step 10, Cortex-A55 PCIe
>>>> RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B
>>>
>>> Note *if use*: how does the TF-A know if USB/PCIe is used by Linux? The documentation mention to set
>>> it *if use*. Same note is on ALL_ON to VBATT transition documentation (namely "if using USB", "if
>>> using PCIe"). If TF-A will do this it should set this signals unconditionally. It will not be
>>> something wrong though. We don't know at the moment what this involves in terms of power consumption,
>>> if it means something...
>>
>> IIUC,
>> The only information we have is,
>>
>> "SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to ALL_ON).
>> "When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break"
>>
>> ALL_ON to AWO_MODE state transition: 
>> USB/PCIe are part of PD_ISOVCC power domain and before turning PD_ISOVCC to off,
>> we need to set USBPWRRDY signal.
>>
>> AWO_MODE to ALL_ON state transition:
>>
>> Turn on PD_ISOVCC first, then clr USBPWRRDY signal for USB usage in linux.
>>
>> Maybe we need to ask hw team, exact usage of USBPWRRDY signal other than state transition.
> 
> As you may already know, this is open for quite some time and is ongoing.

I got more clarification about the USB PWRRDY signal from the HW team.

The conclusion is that the USB PWRRDY is a signal controlled by SYSC
controller that goes to the USB PHY and it tells the USB PHY if the power
supply is ready or not.

In the diagram at [1] the PWRRDY signal need to be asserted/de-asserted
before/after G6, G7, G8, G9, G10 signals.

Philipp,

Can you please confirm that you don't want this signal to be implemented as
a reset signal to know clearly your input on it? I would like to start
looking for another approach in that case.

Thank you,
Claudiu Beznea

[1] https://pasteboard.co/0a1zYBFZXZVb.png

> 
>>
>> Cheers,
>> Biju
>>
>>
Philipp Zabel Nov. 7, 2024, 4:22 p.m. UTC | #35
Hi Claudiu,

On Do, 2024-11-07 at 12:00 +0200, Claudiu Beznea wrote:
> Hi, all,
> 
> On 03.09.2024 17:48, claudiu beznea wrote:
> > 
> > 
> > On 03.09.2024 16:45, Biju Das wrote:
> > > Hi Claudiu,
> > > 
> > > > -----Original Message-----
> > > > From: claudiu beznea <claudiu.beznea@tuxon.dev>
> > > > Sent: Tuesday, September 3, 2024 1:57 PM
> > > > Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas RZ/G3S SoC
> > > > 
> > > > 
> > > > 
> > > > On 03.09.2024 15:37, Biju Das wrote:
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: claudiu beznea <claudiu.beznea@tuxon.dev>
> > > > > > Sent: Tuesday, September 3, 2024 1:26 PM
> > > > > > To: Biju Das <biju.das.jz@bp.renesas.com>; Ulf Hansson
> > > > > > <ulf.hansson@linaro.org>
> > > > > > Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> > > > > > krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> > > > > > geert+renesas@glider.be; magnus.damm@gmail.com;
> > > > > > gregkh@linuxfoundation.org; mturquette@baylibre.com;
> > > > > > sboyd@kernel.org; Yoshihiro Shimoda
> > > > > > <yoshihiro.shimoda.uh@renesas.com>;
> > > > > > linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> > > > > > linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> > > > > > linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > > > linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu Beznea
> > > > > > <claudiu.beznea.uj@bp.renesas.com>
> > > > > > Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> > > > > > RZ/G3S SoC
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 03.09.2024 15:00, Biju Das wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > > -----Original Message-----
> > > > > > > > From: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > > > > Sent: Tuesday, September 3, 2024 12:07 PM
> > > > > > > > To: Claudiu.Beznea <claudiu.beznea@tuxon.dev>; Ulf Hansson
> > > > > > > > <ulf.hansson@linaro.org>
> > > > > > > > Cc: vkoul@kernel.org; kishon@kernel.org; robh@kernel.org;
> > > > > > > > krzk+dt@kernel.org; conor+dt@kernel.org; p.zabel@pengutronix.de;
> > > > > > > > geert+renesas@glider.be; magnus.damm@gmail.com;
> > > > > > > > gregkh@linuxfoundation.org; mturquette@baylibre.com;
> > > > > > > > sboyd@kernel.org; Yoshihiro Shimoda
> > > > > > > > <yoshihiro.shimoda.uh@renesas.com>;
> > > > > > > > linux-phy@lists.infradead.org; devicetree@vger.kernel.org;
> > > > > > > > linux-kernel@vger.kernel.org; linux- renesas-soc@vger.kernel.org;
> > > > > > > > linux-usb@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > > > > > > linux- clk@vger.kernel.org; linux-pm@vger.kernel.org; Claudiu
> > > > > > > > Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > > > > > > Subject: RE: [PATCH 00/16] Add initial USB support for the Renesas
> > > > > > > > RZ/G3S SoC
> > > > > > > > 
> > > > > > > > Hi Claudiu,
> > > > > > > > 
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: claudiu beznea <claudiu.beznea@tuxon.dev>
> > > > > > > > > Sent: Tuesday, September 3, 2024 12:00 PM
> > > > > > > > > Subject: Re: [PATCH 00/16] Add initial USB support for the Renesas
> > > > > > > > > RZ/G3S SoC
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 03.09.2024 13:31, Biju Das wrote:
> > > > > > > > > > > > During boot clr USB PWR READY signal in TF-A.
> > > > > > > > > > > > STR case, suspend set USB PWR READY signal in TF-A.
> > > > > > > > > > > > STR case, resume clr USB PWR READY signal in TF-A.
> > > > > > > > > > > As I said previously, it can be done in different ways. My point
> > > > > > > > > > > was to let Linux set what it needs for all it's devices to work.
> > > > > > > > > > > I think the way to go forward is a
> > > > > > > > > maintainer decision.
> > > > > > > > > > 
> > > > > > > > > > I agree, there can be n number of solution for a problem.
> > > > > > > > > > 
> > > > > > > > > > Since you modelled system state signal (USB PWRRDY) as reset
> > > > > > > > > > control signal, it is reset/DT maintainer's decision to say the
> > > > > > > > > > final word whether this signal fits in reset
> > > > > > > > > system framework or not?
> > > > > > > > > 
> > > > > > > > > I was thinking:
> > > > > > > > > 1/ Geert would be the best to say if he considers it OK to handle this
> > > > > > > > >    in Linux
> > > > > > > > 
> > > > > > > > I agree Geert is the right person for taking SYSTEM decisions,
> > > > > > > > since the signal is used only during state transitions (Table
> > > > > > > > 41.6.4 AWO to ALL_ON and 41.6.3 ALL_ON to AWO)
> > > > > > > 
> > > > > > > One more info, as per [1], this USB PWRRDY signal setting to be before Linux kernel boots.
> > > > > > 
> > > > > > The "controlled by" column mentions CA-55 on PWRRDY signal control
> > > > > > line and it is b/w steps "DDR exits from retention mode" and  "clock
> > > > > > start settings for system bus and peripheral modules". AFAICT, after DDR exists retention mode
> > > > Linux is ready to run.
> > > > > 
> > > > > DDR retention exit happens in TF-A and it jumps into reset code where it executes BL2 in TF_A. Bl2
> > > > checks for warm or cold reset.
> > > > > If it is warm reset, it sets required minimal clocks/resets and pass
> > > > > the control to linux by calling the SMC callback handler. Which in turn calls resume(step 11-->14)
> > > > path.
> > > > 
> > > > Is this from HW manual or some specific documentation? I'm referring at "resume" == "steps 11-->14"
> > > > 
> > > > > 
> > > > > Step 8, Cortex-A55 Exit from DDR retention mode (when using) Setting
> > > > > for exiting form DDR retention mode Step 9, Cortex-A55 USB PHY PWRRDY
> > > > > signal control (if use USB) SYS_USB_PWRRDY Step 10, Cortex-A55 PCIe
> > > > > RST_RSM_B signal control (if use PCIe) SYS_PCIE_RST_RSM_B
> > > > 
> > > > Note *if use*: how does the TF-A know if USB/PCIe is used by Linux? The documentation mention to set
> > > > it *if use*. Same note is on ALL_ON to VBATT transition documentation (namely "if using USB", "if
> > > > using PCIe"). If TF-A will do this it should set this signals unconditionally. It will not be
> > > > something wrong though. We don't know at the moment what this involves in terms of power consumption,
> > > > if it means something...
> > > 
> > > IIUC,
> > > The only information we have is,
> > > 
> > > "SYS_USB_PWRRDY and SYS_PCIE_RST_RSM_B are used when transition from ALL_ON to AWO (or from AWO to ALL_ON).
> > > "When turning off USB PHY and PCIe PHY, if they are not controlled, PHY may break"
> > > 
> > > ALL_ON to AWO_MODE state transition: 
> > > USB/PCIe are part of PD_ISOVCC power domain and before turning PD_ISOVCC to off,
> > > we need to set USBPWRRDY signal.
> > > 
> > > AWO_MODE to ALL_ON state transition:
> > > 
> > > Turn on PD_ISOVCC first, then clr USBPWRRDY signal for USB usage in linux.
> > > 
> > > Maybe we need to ask hw team, exact usage of USBPWRRDY signal other than state transition.
> > 
> > As you may already know, this is open for quite some time and is ongoing.
> 
> I got more clarification about the USB PWRRDY signal from the HW team.
> 
> The conclusion is that the USB PWRRDY is a signal controlled by SYSC
> controller that goes to the USB PHY and it tells the USB PHY if the power
> supply is ready or not.
> 
> In the diagram at [1] the PWRRDY signal need to be asserted/de-asserted
> before/after G6, G7, G8, G9, G10 signals.
> 
> Philipp,
> 
> Can you please confirm that you don't want this signal to be implemented as
> a reset signal to know clearly your input on it? I would like to start
> looking for another approach in that case.

If this is not a reset signal I don't want it to be implemented as one.

regards
Philipp