Re: [PATCH v2 1/3] dt-bindings: arm: renesas: Document Renesas RZ/V2M System Configuration

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 14/11/2022 17:18, Biju Das wrote:
> Hi All,
> 
>> -----Original Message-----
>> From: Biju Das
>> Sent: 11 November 2022 11:17
>  >
>>> On 11/11/2022 10:06, Biju Das wrote:
>>>> Hi Krzysztof Kozlowski,
>>>>
>>>>> -----Original Message-----
>>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>>> Sent: 11 November 2022 08:34
>>>>> To: Biju Das <biju.das.jz@bp.renesas.com>; Rob Herring
>>>>> <robh+dt@kernel.org>; Krzysztof Kozlowski
>>>>> <krzysztof.kozlowski+dt@linaro.org>
>>>>> Cc: Phil Edworthy <phil.edworthy@renesas.com>; Geert Uytterhoeven
>>>>> <geert+renesas@glider.be>; Magnus Damm <magnus.damm@gmail.com>;
>>>>> linux- renesas-soc@vger.kernel.org; devicetree@vger.kernel.org;
>>>>> Chris Paterson <Chris.Paterson2@renesas.com>; Fabrizio Castro
>>>>> <fabrizio.castro.jz@renesas.com>
>>>>> Subject: Re: [PATCH v2 1/3] dt-bindings: arm: renesas: Document
>>>>> Renesas RZ/V2M System Configuration
>>>>>
>>>>> On 10/11/2022 17:21, Biju Das wrote:
>>>>>> From: Phil Edworthy <phil.edworthy@renesas.com>
>>>>>>
>>>>>> Add DT binding documentation for System Configuration (SYS) found
>>>>>> on RZ/V2M SoC's.
>>>>>>
>>>>>> SYS block contains the SYS_VERSION register which can be used to
>>>>>> retrieve SoC version information.
>>>>>>
>>>>>> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
>>>>>> [biju: Updated the example ]
>>>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>>>>>> ---
>>>>>> v1->v2:
>>>>>>  * Moved the file from arm->soc/renesas
>>>>>>  * Updated the path for binding file
>>>>>>  * Updated the example
>>>>>> ---
>>>>>>  .../soc/renesas/renesas,rzv2m-sys.yaml        | 39 +++++++++++++++++++
>>>>>>  1 file changed, 39 insertions(+)
>>>>>>  create mode 100644
>>>>>> Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.ya
>>>>>> ml
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.
>>>>>> ya
>>>>>> ml
>>>>>> b/Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.
>>>>>> ya
>>>>>> ml
>>>>>> new file mode 100644
>>>>>> index 000000000000..cc41747798e2
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/soc/renesas/renesas,rzv2m-sys.
>>>>>> +++ yaml
>>>>>
>>>>> Filename should be based on the compatible. Pretty often some
>>>>> common parts of both are fine (e.g. when file contains multiple
>>>>> compatibles), but this very different then what I see below.
>>>>
>>>> We plan to upstream another similar SoC, RZ/V2MA which has similar
>>>> IP, so we may need to add generic compatible rzv2m-sys. I am
>>>> checking with HW people to get more info about RZ/V2MA.
>>>>
>>>>>
>>>>>> @@ -0,0 +1,39 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
>>>>>> +1.2
>>>>>> +---
>>>>>> +$id:
>>>>> "
>>>> "
>>>>>> +$schema:
>>>>> "
>>>> "
>>>>>
>>>>> Drop quotes from both.
>>>>
>>>> OK.
>>>>
>>>>>
>>>>>> +
>>>>>> +title: Renesas RZ/V2M System Configuration (SYS)
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Geert Uytterhoeven <geert+renesas@glider.be>
>>>>>> +
>>>>>> +description:
>>>>>> +  The RZ/V2M System Configuration (SYS) performs system control
>>>>>> +of the LSI
>>>>>> +  and supports the following functions,
>>>>>> +  - LSI version
>>>>>> +  - 34-bit address space access function
>>>>>> +  - PCIe related settings
>>>>>> +  - WDT stop control
>>>>>> +  - Temperature sensor (TSU) monitor
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: renesas,r9a09g011-sys
>>>>>> +
>>>>>> +  reg:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +required:
>>>>>> +  - compatible
>>>>>> +  - reg
>>>>>> +
>>>>>> +additionalProperties: false
>>>>>> +
>>>>>> +examples:
>>>>>> +  - |
>>>>>> +    sysc: system-configuration@a3f03000 {
>>>>>
>>>>> If I get properly the purpose of the device, usually this is called
>>>>> "system- controller". Use that as device node.
>>>>
>>>> The hardware manual mentions the below. So want to consistent with
>>>> HW
>>> manual.
>>>
>>> If the hardware manual said this is called "foo-whatever-name" or
>>> "rz85736dfnx2", you would use it as well?
>>>
>>> Node names should be generic.
> 
> I got the following feedback from LSI team.
> 
> *The reason why we use "configuration" and not "controller" is the possibility that users might take it to mean 
> "control" of system operation (product operation). This unit determines how the whole LSI configure to operate LSI, 
> so we use "configuration" for this IP name.
> 
> As per this it is not a controller. It is a Configuration IP.
> 
> What should be generic node name for system configuration IP?

Then it's closer to chipid and should be in hwinfo?

Anyway, you wrote in bindings description: "performs system control", so
what can I say?

Best regards,
Krzysztof