Message-ID: <c6ca0988-f2a4-54ea-941b-1c84d1368239@gmail.com>
Date:   Mon, 20 Mar 2023 14:26:52 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH 09/15] dt-bindings: reset: Document ma35d1 reset
 controller bindings
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
        lee@kernel.org, mturquette@baylibre.com, sboyd@kernel.org,
        p.zabel@pengutronix.de, gregkh@linuxfoundation.org,
        jirislaby@kernel.org
Cc:     devicetree@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
        schung@nuvoton.com, Jacky Huang <ychuang3@nuvoton.com>
References: <20230315072902.9298-1-ychuang570808@gmail.com>
 <20230315072902.9298-10-ychuang570808@gmail.com>
 <cee0497e-c441-3937-07ec-0b6c4621f4e4@linaro.org>
 <b9f93711-7302-4ed6-3f71-7bb792507136@linaro.org>
 <4d62376f-1b33-62ac-2ed9-6b71ae7485a2@gmail.com>
 <fd0cb506-707a-4f5d-b917-de6a03787f6a@linaro.org>
Content-Language: en-US
From:   Jacky Huang <ychuang570808@gmail.com>
In-Reply-To: <fd0cb506-707a-4f5d-b917-de6a03787f6a@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 2023/3/19 下午 07:05, Krzysztof Kozlowski wrote:
> On 18/03/2023 05:30, Jacky Huang wrote:
>> Dear Krzysztof,
>>
>>
>> Thanks for your advice.
>>
>>
>> On 2023/3/16 下午 03:39, Krzysztof Kozlowski wrote:
>>> On 16/03/2023 08:37, Krzysztof Kozlowski wrote:
>>>> On 15/03/2023 08:28, Jacky Huang wrote:
>>>>> From: Jacky Huang <ychuang3@nuvoton.com>
>>>>>
>>>>> Add documentation to describe nuvoton ma35d1 reset driver bindings.
>>>> Subject: drop second/last, redundant "bindings". The "dt-bindings"
>>>> prefix is already stating that these are bindings.
>>
>> OK, I will fix it.
>>
>>
>>>>> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com>
>>>>> ---
>>>>>    .../bindings/reset/nuvoton,ma35d1-reset.yaml  | 50 +++++++++++++++++++
>>>>>    1 file changed, 50 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..f66c566c6dce
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml
>>>>> @@ -0,0 +1,50 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/reset/nuvoton,ma35d1-reset.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Nuvoton MA35D1 Reset Controller
>>>>> +
>>>>> +maintainers:
>>>>> +  - Chi-Fang Li <cfli0@nuvoton.com>
>>>>> +  - Jacky Huang <ychuang3@nuvoton.com>
>>>>> +
>>>>> +description:
>>>>> +  The system reset controller can be used to reset various peripheral
>>>>> +  controllers in MA35D1 SoC.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: nuvoton,ma35d1-reset
>>>>> +
>>>>> +  regmap:
>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>>>> +    description: Phandle to the register map node.
>>>> You need to be specific what is this. As you can easily check, there is
>>>> no such property in any devices. I don't understand why do you need it
>>>> in the first place.
>>           reset: reset-controller {
>>               compatible = "nuvoton,ma35d1-reset";
>>               regmap = <&sys>;
>>               #reset-cells = <1>;
>>           };
>>
>> The dt_binding_check check report an error about the above "regmap".
>>
>> I found that add this can pass the test.
> Do not add properties to bindings to "pass the test". That's not the
> goal of bindings. Add there properties because they make sense...
>
> Anyway, you did not answer my question at all. So one by one - address them:
> 1. As you can easily check, there is no such property in any devices.
> Explanation: do you see it anywhere in existing bindings?

Yes, I cannot find it in all bindings. I know it's wrong.

> 2. I don't understand why do you need it in the first place.
> Explanation: your binding suggest this is not needed. If you think
> otherwise, you need to provide rationale.
>
>
>
> Best regards,
> Krzysztof
>

Now we have removed regmap and modify the dtsi as:

     sys: system-management@40460000 {
         compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd";
         reg = <0x0 0x40460000 0x0 0x200>;

         reset: reset-controller {
             compatible = "nuvoton,ma35d1-reset";
             #reset-cells = <1>;
         };
     };

In the reset driver, we obtain the regmap by parent node:
     parent = of_get_parent(dev->of_node); /* parent should be syscon 
node */
     reset_data->regmap = syscon_node_to_regmap(parent);
     of_node_put(parent);

We have it tested OK on ma35d1 SOM board.
And it pass the dt_binding_check and dtbs_check.

Best regards,
Jacky Huang