Re: [PATCH v3 4/7] dt-bindings: clock: mediatek: Add clock driver bindings for MT6795

[David Heidelberg <david.heidelberg@collabora.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 5729 bytes --]

On 25/06/2022 22:29, Krzysztof Kozlowski wrote:
> On 24/06/2022 11:35, AngeloGioacchino Del Regno wrote:
>> Add the bindings for the clock drivers of the MediaTek Helio X10
>> MT6795 SoC.
>>
>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> ---
>>   .../bindings/clock/mediatek,mt6795-clock.yaml | 66 +++++++++++++++++
>>   .../clock/mediatek,mt6795-sys-clock.yaml      | 74 +++++++++++++++++++
>>   2 files changed, 140 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt6795-clock.yaml
>>   create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt6795-sys-clock.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt6795-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt6795-clock.yaml
>> new file mode 100644
>> index 000000000000..795fb18721c3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt6795-clock.yaml
>> @@ -0,0 +1,66 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/clock/mediatek,mt6795-clock.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: MediaTek Functional Clock Controller for MT6795
>> +
>> +maintainers:
>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> +  - Chun-Jie Chen <chun-jie.chen@mediatek.com>
>> +
>> +description: |
>> +  The clock architecture in MediaTek like below
>> +  PLLs -->
>> +          dividers -->
>> +                      muxes
>> +                           -->
>> +                              clock gate
>> +
>> +  The devices provide clock gate control in different IP blocks.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - mediatek,mt6795-mfgcfg
>> +      - mediatek,mt6795-vdecsys
>> +      - mediatek,mt6795-vencsys
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - '#clock-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        mfgcfg: clock-controller@13000000 {
>> +            compatible = "mediatek,mt6795-mfgcfg";
>> +            reg = <0 0x13000000 0 0x1000>;
>> +            #clock-cells = <1>;
>> +        };
>> +
>> +        vdecsys: clock-controller@16000000 {
>> +            compatible = "mediatek,mt6795-vdecsys";
>> +            reg = <0 0x16000000 0 0x1000>;
>> +            #clock-cells = <1>;
>> +        };
>> +
>> +        vencsys: clock-controller@18000000 {
>> +            compatible = "mediatek,mt6795-vencsys";
>> +            reg = <0 0x18000000 0 0x1000>;
>> +            #clock-cells = <1>;
>> +        };
>> +    };
>> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt6795-sys-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt6795-sys-clock.yaml
>> new file mode 100644
>> index 000000000000..44b96af9ceaf
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt6795-sys-clock.yaml
>> @@ -0,0 +1,74 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/clock/mediatek,mt6795-sys-clock.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: MediaTek System Clock Controller for MT6795
>> +
>> +maintainers:
>> +  - AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> +  - Chun-Jie Chen <chun-jie.chen@mediatek.com>
>> +
>> +description:
>> +  The Mediatek system clock controller provides various clocks and system configuration
> Wrap according to Linux coding convention, so at 80.

What I understood that 100 length was agreed [1] as a limit. I haven't 
noticed any recent change regarding to line length.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bdc48fa11e46f867ea4d75fa59ee87a7f48be144

>
>> +  like reset and bus protection on MT6795.
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - mediatek,mt6795-apmixedsys
>> +          - mediatek,mt6795-infracfg
>> +          - mediatek,mt6795-pericfg
>> +          - mediatek,mt6795-topckgen
>> +      - const: syscon
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +  '#reset-cells':
>> +    const: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - '#clock-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    soc {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        topckgen: clock-controller@10000000 {
>> +            compatible = "mediatek,mt6795-topckgen", "syscon";
>> +            reg = <0 0x10000000 0 0x1000>;
>> +            #clock-cells = <1>;
>> +        };
>> +
>> +        infracfg: power-controller@10001000 {
>> +            compatible = "mediatek,mt6795-infracfg", "syscon";
>> +            reg = <0 0x10001000 0 0x1000>;
>> +            #clock-cells = <1>;
>> +            #reset-cells = <1>;
> No need for four examples of the same. They differ only by compatible,
> so this is just unnecessary code... which as you can see does not pass
> the checks. This also has to be fixed.
>
> Maybe keep it as clock-controller?
>
>
> Best regards,
> Krzysztof
>
-- 
David Heidelberg
Consultant Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 695 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]