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

[David Heidelberg]

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@xxxxxxxxxxxxx>
> > ---
> >   .../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@xxxxxxxxxxxxx>
> > +  - Chun-Jie Chen <chun-jie.chen@xxxxxxxxxxxx>
> > +
> > +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@xxxxxxxxxxxxx>
> > +  - Chun-Jie Chen <chun-jie.chen@xxxxxxxxxxxx>
> > +
> > +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: OpenPGP_0x69F567861C1EC014.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature