Message-ID: <7bd6db22-210b-d8ff-7476-b2e38dc7c683@loongson.cn>
Date:   Thu, 27 Oct 2022 11:52:36 +0800
From:   Yinbo Zhu <zhuyinbo@...ngson.cn>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        zhuyinbo@...ngson.cn, Arnd Bergmann <arnd@...db.de>,
        Hector Martin <marcan@...can.st>,
        Lubomir Rintel <lkundrak@...sk>,
        Conor Dooley <conor.dooley@...rochip.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Hitomi Hasegawa <hasegawa-hitomi@...itsu.com>,
        Heiko Stuebner <heiko@...ech.de>,
        Brian Norris <briannorris@...omium.org>,
        Sven Peter <sven@...npeter.dev>, loongarch@...ts.linux.dev,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] dt-bindings: soc: add loongson2 guts



在 2022/10/26 下午10:10, Krzysztof Kozlowski 写道:
> On 26/10/2022 03:22, Yinbo Zhu wrote:
>>
>>
>> 在 2022/10/26 上午3:40, Krzysztof Kozlowski 写道:
>>> On 24/10/2022 23:51, Yinbo Zhu wrote:
>>>> Add the loongson2 soc guts driver binding with DT schema format
>>>> using json-schema.
>>>>
>>>> Signed-off-by: Yinbo Zhu <zhuyinbo@...ngson.cn>
>>>> ---
>>>>    .../soc/loongson/loongson,ls2k-guts.yaml      | 37 +++++++++++++++++++
>>>
>>> Looks like wrong location, although difficult to judge because you did
>>> not describe the hardware at all. If this is chipinfo-like device, then
>>> Documentation/devicetree/bindings/hwinfo/.
yes it is a chipinfo/socinfo device, I will following your advice.
>> My guts driver is refer fsl platform. It was was to manage and access
>> global utilities register block for SoC and it was only used in SoC
>> platform. when driver need use Soc ops to do some function the this
>> driver was needed.  the dcfg (device config) was a function in guts
>> (global utilities) block.
> 
> I can barely understand it.
My description is about chipinfo/socinfo definition. and I have a look
/bindings/hwinfo/, I think move binding file to hwinfo that is okay for me.

Documentation/devicetree/bindings/hwinfo/ti,k3-socinfo.yaml

> 
>> For these type of driver, other platforms were initially placed on
>> Documentation/devicetree/bindings/arm/   if it is arm/arm64
>> architecture. Later, move it to the soc directory.
>>
>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
> 
> How is this related? This is Layerscape, not Loongson2. Describe the
> hardware you are adding bindings for.
The driver functions/type are the same, the driver was register a struct 
soc_device_attribute by soc_device_register then other peripheral driver
can call SoC ops, such as soc_device_match.

then layerscape guts module bindings are placed in
Documentation/devicetree/bindings/soc/, the loongson guts module 
bindings was follow that layerscape and are placed in
Documentation/devicetree/bindings/soc/

In a words,  It is a question about where the binding file should be 
placed.  I think move binding file to hwinfo that is okay for me.
> 
>>
>> So, do you still think it is inappropriate to place it in the soc dir?
>>>
>>>
>>>>    MAINTAINERS                                   |  1 +
>>>>    2 files changed, 38 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>> new file mode 100644
>>>> index 000000000000..2502f8aeb74d
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/soc/loongson/loongson,ls2k-guts.yaml
>>>> @@ -0,0 +1,37 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/soc/loongson/loongson,ls2k-guts.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Loongson2 GUTS driver.
>>>
>>> Drop "driver." unless you refer to some hardware (like motor driver?).
>> this need refer hardware soc datasheet to gain soc register (global
>> utilities register block ).
>> so keep "driver" string that whether was more appropriate?
> 
> What? I cannot parse it.
> 
> Did you understand my comment? If yes, please point to Wikipedia article
> explaining this "Driver" you refer to.
I will remove the "driver" string.
> 
> 
>>>
>>>> +
>>>> +maintainers:
>>>> +  - Yinbo Zhu <zhuyinbo@...ngson.cn>
>>>> +
>>>> +description: |
>>>> +  GUTS driver was to manage and access global utilities block. Initially
>>>
>>> Drop "driver" and describe instead what is GUTS, including its acronym,
>>>
>>>> +  only reading SVR and registering soc device are supported.
>>>
>>> Entire sentence describe Linux driver - drop it. Instead describe the
>>> device, the hardware.
>>>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: loongson,ls2k-guts
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  little-endian: true
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    guts: guts@...00000 {
>>>
>>> Node names should be generic.
>> dcfg/scfg (device cfg/ soc cfg)was the key function of guts (global
>> utilities) block. and guts name I was refer fsl soc driver.
>> "drivers/soc/fsl/guts.c"
>> this binding file was follows of fsl guts.
>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-dcfg.yaml
>> Documentation/devicetree/bindings/soc/fsl/fsl,layerscape-scfg.yaml
>>
>> or, I was use scfg as node name, Do you think it's appropriate?
> 
> No, these are not generic node names.
I was refer "ti,k3-socinfo.yaml",  Do you think it's appropriate that 
socinfo as node name?
> 
>>
>>
>>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> 
> Best regards,
> Krzysztof
> 

Powered by blists - more mailing lists