Re: [PATCH 2/4] dt-bindings: clock: Add Intel Agilex5 clocks and resets

[wen . ping . teh]

>From: Krzysztof Kozlowski @ 2023-06-20 11:06 UTC (permalink / raw)
>>>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: intel,agilex5-clkmgr
>>>
>>>
>>> Why "clkmgr", not "clk"? You did not call it Clock manager anywhere in
>>> the description or title.
>>>
>> 
>> The register in Agilex5 handling the clock is named clock_mgr.
>> Previous IntelSocFPGA, Agilex and Stratix10, are also named clkmgr.
>
>So use it in description.

Noted. Will update the description in V2.

>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  # Clock controller node:
>>>> +  - |
>>>> +    clkmgr: clock-controller@10d10000 {
>>>> +      compatible = "intel,agilex5-clkmgr";
>>>> +      reg = <0x10d10000 0x1000>;
>>>> +      #clock-cells = <1>;
>>>> +    };
>>>> +...
>>>> diff --git a/include/dt-bindings/clock/agilex5-clock.h b/include/dt-bindings/clock/agilex5-clock.h
>>>> new file mode 100644
>>>> index 000000000000..4505b352cd83
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/clock/agilex5-clock.h
>>>
>>> Filename the same as binding. Missing vendor prefix, entirely different
>>> device name.
>>>
>> 
>> Will change filename to intel,agilex5-clock.h in V2.
>
>Read the comment - same as binding. You did not call binding that way...
>unless you rename the binding.

Just to confirm, the binding name you are referring to is "intel,agilex5-clkmgr"?
I will change the filename to intel,agilex5-clkmgr.h in V2.

Best Regards,
Wen Ping