Message-ID:
 <TY3PR01MB11346676D07F8434BBD68E62786932@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Tue, 3 Sep 2024 15:30:43 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Claudiu.Beznea <claudiu.beznea@...on.dev>, Ulf Hansson
	<ulf.hansson@...aro.org>
CC: "vkoul@...nel.org" <vkoul@...nel.org>, "kishon@...nel.org"
	<kishon@...nel.org>, "robh@...nel.org" <robh@...nel.org>,
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
	<conor+dt@...nel.org>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
	"geert+renesas@...der.be" <geert+renesas@...der.be>, "magnus.damm@...il.com"
	<magnus.damm@...il.com>, "gregkh@...uxfoundation.org"
	<gregkh@...uxfoundation.org>, "mturquette@...libre.com"
	<mturquette@...libre.com>, "sboyd@...nel.org" <sboyd@...nel.org>, Yoshihiro
 Shimoda <yoshihiro.shimoda.uh@...esas.com>, "linux-phy@...ts.infradead.org"
	<linux-phy@...ts.infradead.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
	<linux-renesas-soc@...r.kernel.org>, "linux-usb@...r.kernel.org"
	<linux-usb@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-clk@...r.kernel.org"
	<linux-clk@...r.kernel.org>, "linux-pm@...r.kernel.org"
	<linux-pm@...r.kernel.org>, Claudiu Beznea <claudiu.beznea.uj@...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@...on.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@...on.dev>
> >> Sent: Tuesday, September 3, 2024 1:57 PM
> > -
> >> clk@...r.kernel.org; linux-pm@...r.kernel.org; Claudiu Beznea
> >> <claudiu.beznea.uj@...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@...on.dev>
> >>>> Sent: Tuesday, September 3, 2024 1:26 PM
> >>>> To: Biju Das <biju.das.jz@...renesas.com>; Ulf Hansson
> >>>> <ulf.hansson@...aro.org>
> >>>> Cc: vkoul@...nel.org; kishon@...nel.org; robh@...nel.org;
> >>>> krzk+dt@...nel.org; conor+dt@...nel.org; p.zabel@...gutronix.de;
> >>>> geert+renesas@...der.be; magnus.damm@...il.com;
> >>>> gregkh@...uxfoundation.org; mturquette@...libre.com;
> >>>> sboyd@...nel.org; Yoshihiro Shimoda
> >>>> <yoshihiro.shimoda.uh@...esas.com>;
> >>>> linux-phy@...ts.infradead.org; devicetree@...r.kernel.org;
> >>>> linux-kernel@...r.kernel.org; linux- renesas-soc@...r.kernel.org;
> >>>> linux-usb@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> >>>> linux- clk@...r.kernel.org; linux-pm@...r.kernel.org; Claudiu
> >>>> Beznea <claudiu.beznea.uj@...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@...renesas.com>
> >>>>>> Sent: Tuesday, September 3, 2024 12:07 PM
> >>>>>> To: Claudiu.Beznea <claudiu.beznea@...on.dev>; Ulf Hansson
> >>>>>> <ulf.hansson@...aro.org>
> >>>>>> Cc: vkoul@...nel.org; kishon@...nel.org; robh@...nel.org;
> >>>>>> krzk+dt@...nel.org; conor+dt@...nel.org; p.zabel@...gutronix.de;
> >>>>>> geert+renesas@...der.be; magnus.damm@...il.com;
> >>>>>> gregkh@...uxfoundation.org; mturquette@...libre.com;
> >>>>>> sboyd@...nel.org; Yoshihiro Shimoda
> >>>>>> <yoshihiro.shimoda.uh@...esas.com>;
> >>>>>> linux-phy@...ts.infradead.org; devicetree@...r.kernel.org;
> >>>>>> linux-kernel@...r.kernel.org; linux- renesas-soc@...r.kernel.org;
> >>>>>> linux-usb@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> >>>>>> linux- clk@...r.kernel.org; linux-pm@...r.kernel.org; Claudiu
> >>>>>> Beznea <claudiu.beznea.uj@...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@...on.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

Powered by blists - more mailing lists