Message-ID: <54f51fa0-7821-b67b-b782-eb9a35b7bba9@linaro.org>
Date:   Tue, 28 Feb 2023 11:37:20 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH v4 4/4] dt-bindings: syscon: Add StarFive syscon doc
Content-Language: en-US
To:     William Qiu <william.qiu@starfivetech.com>,
        Rob Herring <robh@kernel.org>
Cc:     linux-riscv@lists.infradead.org, devicetree@vger.kernel.org,
        linux-mmc@vger.kernel.org,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Jaehoon Chung <jh80.chung@samsung.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        linux-kernel@vger.kernel.org
References: <20230215113249.47727-1-william.qiu@starfivetech.com>
 <20230215113249.47727-5-william.qiu@starfivetech.com>
 <20230220234335.GA615198-robh@kernel.org>
 <348796cc-72d9-4dcf-9f09-4c2aa55cb858@starfivetech.com>
 <20230227222904.GC1048218-robh@kernel.org>
 <f8d2b665-ce5d-81f8-8c55-81f1a4cb62b9@starfivetech.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <f8d2b665-ce5d-81f8-8c55-81f1a4cb62b9@starfivetech.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 28/02/2023 10:05, William Qiu wrote:
> 
> 
> On 2023/2/28 6:29, Rob Herring wrote:
>> On Tue, Feb 21, 2023 at 10:44:02AM +0800, William Qiu wrote:
>>>
>>>
>>> On 2023/2/21 7:43, Rob Herring wrote:
>>>> On Wed, Feb 15, 2023 at 07:32:49PM +0800, William Qiu wrote:
>>>>> Add documentation to describe StarFive System Controller Registers.
>>>>>
>>>>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>>>>> ---
>>>>>  .../bindings/soc/starfive/jh7110-syscon.yaml  | 51 +++++++++++++++++++
>>>>>  MAINTAINERS                                   |  5 ++
>>>>>  2 files changed, 56 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..fa4d8522a454
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml
>>>>> @@ -0,0 +1,51 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/soc/starfive/jh7110-syscon.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: StarFive JH7110 SoC system controller
>>>>> +
>>>>> +maintainers:
>>>>> +  - William Qiu <william.qiu@starfivetech.com>
>>>>> +
>>>>> +description: |
>>>>> +  The StarFive JH7110 SoC system controller provides register information such
>>>>> +  as offset, mask and shift to configure related modules such as MMC and PCIe.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    items:
>>>>> +      - enum:
>>>>> +          - starfive,jh7110-stg-syscon
>>>>> +          - starfive,jh7110-sys-syscon
>>>>> +          - starfive,jh7110-aon-syscon
>>>>
>>>> Is 'syscon' really part of what the blocks are called? Is just 'stg', 
>>>> 'sys' and 'aon' not unique enough?
>>>>
>>>> Rob
>>> Hi Rob,
>>>
>>> In StarFive SoC, we do have syscrg/aoncrg/stgcrg, which is uesd to be the clock
>>> controller, so 'syscon' is added to avoid confusion.
>>
>> You've only added to my confusion. 'syscrg' and 'sys-syscon' are 2 
>> different h/w blocks and unrelated to each other? Or 'syscrg' is the 
>> clock portion of 'sys-syscon'? In that case, 'syscrg' should be a child 
>> of 'sys-syscon' or possibly just all one node. Please provide details on 
>> the entire h/w block so we can provide better input on the bindings.
>>
>> Rob
> 
> Hi Rob,
> 
> It's my description that's problematic.'syscon' here refers to the hardware module
> inside our JH7110, which is different from the syscon interface in linux. The syscon
> I added now uses the syscon interface of linux to read and write the syscon register
> in our JH7110. So we decided to name it that way.

You didn't really answer Rob's questions.

Also, syscon is Linux term, so are you sure hardware module is called
like this? Hardware engineers took pure Linux name and used it?

Best regards,
Krzysztof