Re: [PATCH v2 01/15] dt-bindings: soc: renesas: renesas,rzg2l-sysc: Add #renesas,sysc-signal-cells

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

Hi, Geert,

On 29.11.2024 10:38, Geert Uytterhoeven wrote:
> Hi Claudiu,
> 
> On Fri, Nov 29, 2024 at 9:21 AM Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
>> On 28.11.2024 17:46, Geert Uytterhoeven wrote:
>>> On Tue, Nov 26, 2024 at 10:21 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> The RZ/G3S system controller (SYSC) has registers to control signals that
>>>> are routed to various IPs. These signals must be controlled during
>>>> configuration of the respective IPs. One such signal is the USB PWRRDY,
>>>> which connects the SYSC and the USB PHY. This signal must to be controlled
>>>> before and after the power to the USB PHY is turned off/on.
>>>>
>>>> Other similar signals include the following (according to the RZ/G3S
>>>> hardware manual):
>>>>
>>>> * PCIe:
>>>> - ALLOW_ENTER_L1 signal controlled through the SYS_PCIE_CFG register
>>>> - PCIE_RST_RSM_B signal controlled through the SYS_PCIE_RST_RSM_B
>>>>   register
>>>> - MODE_RXTERMINATION signal controlled through SYS_PCIE_PHY register
>>>>
>>>> * SPI:
>>>> - SEL_SPI_OCTA signal controlled through SYS_IPCONT_SEL_SPI_OCTA
>>>>   register
>>>>
>>>> * I2C/I3C:
>>>> - af_bypass I2C signals controlled through SYS_I2Cx_CFG registers
>>>>   (x=0..3)
>>>> - af_bypass I3C signal controlled through SYS_I3C_CFG register
>>>>
>>>> * Ethernet:
>>>> - FEC_GIGA_ENABLE Ethernet signals controlled through SYS_GETHx_CFG
>>>>   registers (x=0..1)
>>>>
>>>> Add #renesas,sysc-signal-cells DT property to allow different SYSC signals
>>>> consumers to manage these signals.
>>>>
>>>> The goal is to enable consumers to specify the required access data for
>>>> these signals (through device tree) and let their respective drivers
>>>> control these signals via the syscon regmap provided by the system
>>>> controller driver. For example, the USB PHY will describe this relation
>>>> using the following DT property:
>>>>
>>>> usb2_phy1: usb-phy@11e30200 {
>>>>         // ...
>>>>         renesas,sysc-signal = <&sysc 0xd70 0x1>;
>>>>         // ...
>>>> };
>>>
>>> IIUIC, the consumer driver will  appear to control the SYSC bits
>>> directly, but due to the use of custom validating regmap accessors
>>> and reference counting in the SYSC driver, this is safe?
>>
>> I'm not sure I fully understand the safety concern.
> 
> Sorry for my bad expression, this was more like a rhetorical question.
> I meant that it is safe because:
>   1. Consumers cannot perform arbitrary register accesses,
>   2. The reference counting guarantees correct operation, despite
>       both usb-phy nodes using the same renesas,sysc-signal.
> 
> So everything is fine.
> 
>>> The extra safety requires duplicating the register bits in both DT
>>> and the SYSC driver.
>>
>> One other option I saw was to have common defines for registers that could
>> have been shared b/w driver and DTSes. But it looked better to me the way
>> it has been presented in this series.
>>
>>> Both usb-phy nodes on RZG3S use the same renesas,sysc-signal, so the
>>> reference counting is indeed needed.  They are in different power
>>> domains, could that be an issue w.r.t. ordering?
>>
>> In chapter "32.4.2.1 USB/PHY related pins", section "When either Port1 or
>> Port2 is unused" of the RZ/G3S HW manual it is mentioned "Since USB_VDD18 /
>> USB_VDD33 are common to 2 Port PHY, it is necessary to supply power even
>> when one of the
>>  ports is not in use".
> 
> Does that mean you have to power the other PHY on through the
> CPG_BUS_PERI_COM_MSTOP register, too?

I don't know at the moment if there is hard requirement b/w USB PWRRDY and
the USB PHY CPG MSTOP bit. I'll have to ask further internally.

Thank you,
Claudiu

> (I know you haven't added R9A08G045_PD_USBx to the USB nodes yet,
>  as #power-domain-cells is still 0).
> 
>> (From the discussions w/ the internal HW team) The PWRRDY is an (software
>> controlled) indicator to the USB PHY that power supply is ready.
>>
>> From that and [1] I get that both PHYs are powered by the same regulators
>> (USB_VDD18/USB_VDD33) and the USB PWRRDY signal need to be set before/after
>> the USB PHY power off/on. Because of this I consider the order doesn't matter.
>>
>> [1] https://gcdnb.pbrd.co/images/0a1zYBFZXZVb.png
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>