Re: [PATCH 09/15] dt-bindings: reset: Document ma35d1 reset controller bindings

[Jacky Huang <ychuang570808@gmail.com>]

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