diff mbox series

[02/16] dt-bindings: soc: renesas: renesas,rzg2l-sysc: Add #reset-cells for RZ/G3S

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

Commit Message

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

The RZ/G3S System controller has registers to control signals that need
to be de-asserted/asserted before/after different SoC areas are power
on/off. This signals are implemented as reset signals. For this document
the #reset-cells property.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---
 .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Conor Dooley Aug. 22, 2024, 4:42 p.m. UTC | #1
On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> The RZ/G3S System controller has registers to control signals that need
> to be de-asserted/asserted before/after different SoC areas are power
> on/off. This signals are implemented as reset signals. For this document
> the #reset-cells property.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> ---
>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> index 4386b2c3fa4d..6b0bb34485d9 100644
> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> @@ -42,12 +42,28 @@ properties:
>        - const: cm33stbyr_int
>        - const: ca55_deny
>  
> +  "#reset-cells":
> +    const: 1
> +
>  required:
>    - compatible
>    - reg
>  
>  additionalProperties: false
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a08g045-sysc
> +    then:
> +      required:
> +        - "#reset-cells"

Given this is new required property on an existing platform, I'd expect
some mention of why it used to be okay to not have this but is now
required. Did firmware or a bootloader stage take things out of reset?

> +    else:
> +      properties:
> +        "#reset-cells": false
> +
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> -- 
> 2.39.2
>
Conor Dooley Aug. 22, 2024, 4:44 p.m. UTC | #2
On Thu, Aug 22, 2024 at 05:42:57PM +0100, Conor Dooley wrote:
> On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
> > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > 
> > The RZ/G3S System controller has registers to control signals that need
> > to be de-asserted/asserted before/after different SoC areas are power
> > on/off. This signals are implemented as reset signals. For this document
> > the #reset-cells property.
> > 
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > ---
> >  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > index 4386b2c3fa4d..6b0bb34485d9 100644
> > --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> > @@ -42,12 +42,28 @@ properties:
> >        - const: cm33stbyr_int
> >        - const: ca55_deny
> >  
> > +  "#reset-cells":
> > +    const: 1
> > +
> >  required:
> >    - compatible
> >    - reg
> >  
> >  additionalProperties: false
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,r9a08g045-sysc
> > +    then:
> > +      required:
> > +        - "#reset-cells"
> 
> Given this is new required property on an existing platform, I'd expect
> some mention of why it used to be okay to not have this but is now
> required. Did firmware or a bootloader stage take things out of reset?

Reading a bit more into the series, the peripherals in question were
just never used nor did a driver for the sysc exist, so there's neither
explanation of prior behaviour nor concerns about compatibility?

> 
> > +    else:
> > +      properties:
> > +        "#reset-cells": false
> > +
> >  examples:
> >    - |
> >      #include <dt-bindings/interrupt-controller/arm-gic.h>
> > -- 
> > 2.39.2
> >
Claudiu Beznea Aug. 23, 2024, 7:54 a.m. UTC | #3
Hi, Conor,

On 22.08.2024 19:42, Conor Dooley wrote:
> On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> The RZ/G3S System controller has registers to control signals that need
>> to be de-asserted/asserted before/after different SoC areas are power
>> on/off. This signals are implemented as reset signals. For this document
>> the #reset-cells property.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> ---
>>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>> index 4386b2c3fa4d..6b0bb34485d9 100644
>> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>> @@ -42,12 +42,28 @@ properties:
>>        - const: cm33stbyr_int
>>        - const: ca55_deny
>>  
>> +  "#reset-cells":
>> +    const: 1
>> +
>>  required:
>>    - compatible
>>    - reg
>>  
>>  additionalProperties: false
>>  
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: renesas,r9a08g045-sysc
>> +    then:
>> +      required:
>> +        - "#reset-cells"
> 
> Given this is new required property on an existing platform, I'd expect
> some mention of why it used to be okay to not have this but is now
> required. Did firmware or a bootloader stage take things out of reset?

On previous SoCs the SYS controller has no support for controlling the
signals going to different peripherals (USB, PCIE in case of RZ/G3S).
I'll add a note about this on next version.

Thank you,
Claudiu Beznea


> 
>> +    else:
>> +      properties:
>> +        "#reset-cells": false
>> +
>>  examples:
>>    - |
>>      #include <dt-bindings/interrupt-controller/arm-gic.h>
>> -- 
>> 2.39.2
>>
Claudiu Beznea Aug. 23, 2024, 7:59 a.m. UTC | #4
On 22.08.2024 19:44, Conor Dooley wrote:
> On Thu, Aug 22, 2024 at 05:42:57PM +0100, Conor Dooley wrote:
>> On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>> The RZ/G3S System controller has registers to control signals that need
>>> to be de-asserted/asserted before/after different SoC areas are power
>>> on/off. This signals are implemented as reset signals. For this document
>>> the #reset-cells property.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>> ---
>>>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
>>>  1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>> index 4386b2c3fa4d..6b0bb34485d9 100644
>>> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>> @@ -42,12 +42,28 @@ properties:
>>>        - const: cm33stbyr_int
>>>        - const: ca55_deny
>>>  
>>> +  "#reset-cells":
>>> +    const: 1
>>> +
>>>  required:
>>>    - compatible
>>>    - reg
>>>  
>>>  additionalProperties: false
>>>  
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: renesas,r9a08g045-sysc
>>> +    then:
>>> +      required:
>>> +        - "#reset-cells"
>>
>> Given this is new required property on an existing platform, I'd expect
>> some mention of why it used to be okay to not have this but is now
>> required. Did firmware or a bootloader stage take things out of reset?
> 
> Reading a bit more into the series, the peripherals in question were
> just never used nor did a driver for the sysc exist, so there's neither

Exactly.

> explanation of prior behaviour nor concerns about compatibility?

The newly introduced sysc driver is probed only for RZ/G3S and used to
control the USB, PCIe signals though reset control driver (registered by
sysc driver on auxiliary bus) and to identify the chip. The intention is to
later migrate the chip identification support for the rest of RZ/G2 devices
to this new driver and add more functionalities, when/if needed.

Thank you,
Claudiu Beznea

> 
>>
>>> +    else:
>>> +      properties:
>>> +        "#reset-cells": false
>>> +
>>>  examples:
>>>    - |
>>>      #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> -- 
>>> 2.39.2
>>>
> 
>
Conor Dooley Aug. 23, 2024, 4:18 p.m. UTC | #5
On Fri, Aug 23, 2024 at 10:54:06AM +0300, claudiu beznea wrote:
> Hi, Conor,
> 
> On 22.08.2024 19:42, Conor Dooley wrote:
> > On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
> >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>
> >> The RZ/G3S System controller has registers to control signals that need
> >> to be de-asserted/asserted before/after different SoC areas are power
> >> on/off. This signals are implemented as reset signals. For this document
> >> the #reset-cells property.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >> ---
> >>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >> index 4386b2c3fa4d..6b0bb34485d9 100644
> >> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >> @@ -42,12 +42,28 @@ properties:
> >>        - const: cm33stbyr_int
> >>        - const: ca55_deny
> >>  
> >> +  "#reset-cells":
> >> +    const: 1
> >> +
> >>  required:
> >>    - compatible
> >>    - reg
> >>  
> >>  additionalProperties: false
> >>  
> >> +allOf:
> >> +  - if:
> >> +      properties:
> >> +        compatible:
> >> +          contains:
> >> +            const: renesas,r9a08g045-sysc
> >> +    then:
> >> +      required:
> >> +        - "#reset-cells"
> > 
> > Given this is new required property on an existing platform, I'd expect
> > some mention of why it used to be okay to not have this but is now
> > required. Did firmware or a bootloader stage take things out of reset?
> 
> On previous SoCs the SYS controller has no support for controlling the
> signals going to different peripherals (USB, PCIE in case of RZ/G3S).
> I'll add a note about this on next version.

My initial thought here wasn't about previous SoCs though, it was
because you didn't add the compatible in this series for /this/ SoC.
What's worth noting isn't about the prior SoCs, it is about what makes
it okay for this one.
Claudiu Beznea Aug. 23, 2024, 4:26 p.m. UTC | #6
On 23.08.2024 19:18, Conor Dooley wrote:
> On Fri, Aug 23, 2024 at 10:54:06AM +0300, claudiu beznea wrote:
>> Hi, Conor,
>>
>> On 22.08.2024 19:42, Conor Dooley wrote:
>>> On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> The RZ/G3S System controller has registers to control signals that need
>>>> to be de-asserted/asserted before/after different SoC areas are power
>>>> on/off. This signals are implemented as reset signals. For this document
>>>> the #reset-cells property.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>> ---
>>>>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
>>>>  1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>>> index 4386b2c3fa4d..6b0bb34485d9 100644
>>>> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>>> @@ -42,12 +42,28 @@ properties:
>>>>        - const: cm33stbyr_int
>>>>        - const: ca55_deny
>>>>  
>>>> +  "#reset-cells":
>>>> +    const: 1
>>>> +
>>>>  required:
>>>>    - compatible
>>>>    - reg
>>>>  
>>>>  additionalProperties: false
>>>>  
>>>> +allOf:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: renesas,r9a08g045-sysc
>>>> +    then:
>>>> +      required:
>>>> +        - "#reset-cells"
>>>
>>> Given this is new required property on an existing platform, I'd expect
>>> some mention of why it used to be okay to not have this but is now
>>> required. Did firmware or a bootloader stage take things out of reset?
>>
>> On previous SoCs the SYS controller has no support for controlling the
>> signals going to different peripherals (USB, PCIE in case of RZ/G3S).
>> I'll add a note about this on next version.
> 
> My initial thought here wasn't about previous SoCs though, it was
> because you didn't add the compatible in this series for /this/ SoC.

RZ/G3S compatible is already present in this file:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml#n26

Thank you,
Claudiu Beznea

> What's worth noting isn't about the prior SoCs, it is about what makes
> it okay for this one.
Conor Dooley Aug. 23, 2024, 4:33 p.m. UTC | #7
On Fri, Aug 23, 2024 at 07:26:42PM +0300, claudiu beznea wrote:
> On 23.08.2024 19:18, Conor Dooley wrote:
> > On Fri, Aug 23, 2024 at 10:54:06AM +0300, claudiu beznea wrote:
> >> Hi, Conor,
> >>
> >> On 22.08.2024 19:42, Conor Dooley wrote:
> >>> On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
> >>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>>
> >>>> The RZ/G3S System controller has registers to control signals that need
> >>>> to be de-asserted/asserted before/after different SoC areas are power
> >>>> on/off. This signals are implemented as reset signals. For this document
> >>>> the #reset-cells property.
> >>>>
> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>> ---
> >>>>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
> >>>>  1 file changed, 16 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >>>> index 4386b2c3fa4d..6b0bb34485d9 100644
> >>>> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >>>> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >>>> @@ -42,12 +42,28 @@ properties:
> >>>>        - const: cm33stbyr_int
> >>>>        - const: ca55_deny
> >>>>  
> >>>> +  "#reset-cells":
> >>>> +    const: 1
> >>>> +
> >>>>  required:
> >>>>    - compatible
> >>>>    - reg
> >>>>  
> >>>>  additionalProperties: false
> >>>>  
> >>>> +allOf:
> >>>> +  - if:
> >>>> +      properties:
> >>>> +        compatible:
> >>>> +          contains:
> >>>> +            const: renesas,r9a08g045-sysc
> >>>> +    then:
> >>>> +      required:
> >>>> +        - "#reset-cells"
> >>>
> >>> Given this is new required property on an existing platform, I'd expect
> >>> some mention of why it used to be okay to not have this but is now
> >>> required. Did firmware or a bootloader stage take things out of reset?
> >>
> >> On previous SoCs the SYS controller has no support for controlling the
> >> signals going to different peripherals (USB, PCIE in case of RZ/G3S).
> >> I'll add a note about this on next version.
> > 
> > My initial thought here wasn't about previous SoCs though, it was
> > because you didn't add the compatible in this series for /this/ SoC.
> 
> RZ/G3S compatible is already present in this file:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml#n26

I know, first thing I did when I read the original patch was open the
file ;)
I don't care about the old SoCs, cos you're not applying the property to
them, so what's changed between SoCs isn't really relevant. It's a mention
of why, on this SoC, it is safe to add new required properties that I want.

AFAIU the answer is that no consumer of the resets existed before, so
there's not some special state there, and I am guessing that the new
sysc driver you're adding isn't going to fail to probe if there are no
resets, it just won't register a reset controller? Which is fine, cos all
devicetrees that have the new peripherals will have #reset-cells etc.

> > What's worth noting isn't about the prior SoCs, it is about what makes
> > it okay for this one.
Claudiu Beznea Aug. 26, 2024, 10:15 a.m. UTC | #8
On 23.08.2024 19:33, Conor Dooley wrote:
> On Fri, Aug 23, 2024 at 07:26:42PM +0300, claudiu beznea wrote:
>> On 23.08.2024 19:18, Conor Dooley wrote:
>>> On Fri, Aug 23, 2024 at 10:54:06AM +0300, claudiu beznea wrote:
>>>> Hi, Conor,
>>>>
>>>> On 22.08.2024 19:42, Conor Dooley wrote:
>>>>> On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
>>>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>>
>>>>>> The RZ/G3S System controller has registers to control signals that need
>>>>>> to be de-asserted/asserted before/after different SoC areas are power
>>>>>> on/off. This signals are implemented as reset signals. For this document
>>>>>> the #reset-cells property.
>>>>>>
>>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>>> ---
>>>>>>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
>>>>>>  1 file changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>>>>> index 4386b2c3fa4d..6b0bb34485d9 100644
>>>>>> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
>>>>>> @@ -42,12 +42,28 @@ properties:
>>>>>>        - const: cm33stbyr_int
>>>>>>        - const: ca55_deny
>>>>>>  
>>>>>> +  "#reset-cells":
>>>>>> +    const: 1
>>>>>> +
>>>>>>  required:
>>>>>>    - compatible
>>>>>>    - reg
>>>>>>  
>>>>>>  additionalProperties: false
>>>>>>  
>>>>>> +allOf:
>>>>>> +  - if:
>>>>>> +      properties:
>>>>>> +        compatible:
>>>>>> +          contains:
>>>>>> +            const: renesas,r9a08g045-sysc
>>>>>> +    then:
>>>>>> +      required:
>>>>>> +        - "#reset-cells"
>>>>>
>>>>> Given this is new required property on an existing platform, I'd expect
>>>>> some mention of why it used to be okay to not have this but is now
>>>>> required. Did firmware or a bootloader stage take things out of reset?
>>>>
>>>> On previous SoCs the SYS controller has no support for controlling the
>>>> signals going to different peripherals (USB, PCIE in case of RZ/G3S).
>>>> I'll add a note about this on next version.
>>>
>>> My initial thought here wasn't about previous SoCs though, it was
>>> because you didn't add the compatible in this series for /this/ SoC.
>>
>> RZ/G3S compatible is already present in this file:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml#n26
> 
> I know, first thing I did when I read the original patch was open the
> file ;)
> I don't care about the old SoCs, cos you're not applying the property to
> them, so what's changed between SoCs isn't really relevant. It's a mention
> of why, on this SoC, it is safe to add new required properties that I want.


> 
> AFAIU the answer is that no consumer of the resets existed before, so

That's true.

> there's not some special state there, and I am guessing that the new
> sysc driver you're adding isn't going to fail to probe if there are no
> resets, 

That's true.

it just won't register a reset controller?

It will register it but,

the new sysc driver is going to probe only for this SoC (RZ/G3S). On RZ/G3S
we have 2 resets. These well be registered unconditionally, currently, only
for RZ/G3S. If there will be no DT users for it then it should be no
problem, AFAICT.

SYSC variants have common features b/w different SoC variants (one of them
being chip identification). The feature implemented though reset controller
in this series is not common but particular to RZ/G3S.

When the SYSC will be extended for other SoCs the reset driver registration
would have to be adapted to not be registered. At the moment, as the SYC is
compatible only with RZ/G3S and the reset driver is registered on auxiliary
bus though SYSC there is no restriction, reset is registered all the time,
but SYSC is only compatible with RZ/G3S.

> Which is fine, cos all
> devicetrees that have the new peripherals will have #reset-cells etc.
> 
>>> What's worth noting isn't about the prior SoCs, it is about what makes
>>> it okay for this one.
Conor Dooley Aug. 26, 2024, 5:09 p.m. UTC | #9
On Mon, Aug 26, 2024 at 01:15:43PM +0300, claudiu beznea wrote:
> 
> 
> On 23.08.2024 19:33, Conor Dooley wrote:
> > On Fri, Aug 23, 2024 at 07:26:42PM +0300, claudiu beznea wrote:
> >> On 23.08.2024 19:18, Conor Dooley wrote:
> >>> On Fri, Aug 23, 2024 at 10:54:06AM +0300, claudiu beznea wrote:
> >>>> Hi, Conor,
> >>>>
> >>>> On 22.08.2024 19:42, Conor Dooley wrote:
> >>>>> On Thu, Aug 22, 2024 at 06:27:47PM +0300, Claudiu wrote:
> >>>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>>>>
> >>>>>> The RZ/G3S System controller has registers to control signals that need
> >>>>>> to be de-asserted/asserted before/after different SoC areas are power
> >>>>>> on/off. This signals are implemented as reset signals. For this document
> >>>>>> the #reset-cells property.
> >>>>>>
> >>>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> >>>>>> ---
> >>>>>>  .../bindings/soc/renesas/renesas,rzg2l-sysc.yaml | 16 ++++++++++++++++
> >>>>>>  1 file changed, 16 insertions(+)
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >>>>>> index 4386b2c3fa4d..6b0bb34485d9 100644
> >>>>>> --- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >>>>>> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
> >>>>>> @@ -42,12 +42,28 @@ properties:
> >>>>>>        - const: cm33stbyr_int
> >>>>>>        - const: ca55_deny
> >>>>>>  
> >>>>>> +  "#reset-cells":
> >>>>>> +    const: 1
> >>>>>> +
> >>>>>>  required:
> >>>>>>    - compatible
> >>>>>>    - reg
> >>>>>>  
> >>>>>>  additionalProperties: false
> >>>>>>  
> >>>>>> +allOf:
> >>>>>> +  - if:
> >>>>>> +      properties:
> >>>>>> +        compatible:
> >>>>>> +          contains:
> >>>>>> +            const: renesas,r9a08g045-sysc
> >>>>>> +    then:
> >>>>>> +      required:
> >>>>>> +        - "#reset-cells"
> >>>>>
> >>>>> Given this is new required property on an existing platform, I'd expect
> >>>>> some mention of why it used to be okay to not have this but is now
> >>>>> required. Did firmware or a bootloader stage take things out of reset?
> >>>>
> >>>> On previous SoCs the SYS controller has no support for controlling the
> >>>> signals going to different peripherals (USB, PCIE in case of RZ/G3S).
> >>>> I'll add a note about this on next version.
> >>>
> >>> My initial thought here wasn't about previous SoCs though, it was
> >>> because you didn't add the compatible in this series for /this/ SoC.
> >>
> >> RZ/G3S compatible is already present in this file:
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml#n26
> > 
> > I know, first thing I did when I read the original patch was open the
> > file ;)
> > I don't care about the old SoCs, cos you're not applying the property to
> > them, so what's changed between SoCs isn't really relevant. It's a mention
> > of why, on this SoC, it is safe to add new required properties that I want.
> 
> 
> > 
> > AFAIU the answer is that no consumer of the resets existed before, so
> 
> That's true.
> 
> > there's not some special state there, and I am guessing that the new
> > sysc driver you're adding isn't going to fail to probe if there are no
> > resets, 
> 
> That's true.
> 
> it just won't register a reset controller?
> 
> It will register it but,
> 
> the new sysc driver is going to probe only for this SoC (RZ/G3S). On RZ/G3S
> we have 2 resets. These well be registered unconditionally, currently, only
> for RZ/G3S. If there will be no DT users for it then it should be no
> problem, AFAICT.

Okay, sounds it doesn't break for existing devicetrees.
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
Geert Uytterhoeven Oct. 8, 2024, 1:24 p.m. UTC | #10
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 System controller has registers to control signals that need
> to be de-asserted/asserted before/after different SoC areas are power
> on/off. This signals are implemented as reset signals. For this document
> the #reset-cells property.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
index 4386b2c3fa4d..6b0bb34485d9 100644
--- a/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
+++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzg2l-sysc.yaml
@@ -42,12 +42,28 @@  properties:
       - const: cm33stbyr_int
       - const: ca55_deny
 
+  "#reset-cells":
+    const: 1
+
 required:
   - compatible
   - reg
 
 additionalProperties: false
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a08g045-sysc
+    then:
+      required:
+        - "#reset-cells"
+    else:
+      properties:
+        "#reset-cells": false
+
 examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>