Re: [PATCH v4 2/4] dt-bindings: clock: mediatek: add clock controllers of MT7988

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 09/12/23 22:55, Daniel Golle ha scritto:
> Add various clock controllers found in the MT7988 SoC to existing
> bindings (if applicable) and add files for the new ethwarp, mcusys
> and xfi-pll clock controllers not previously present in any SoC.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v4:
>   * add subnodes for controllers acting as MFD
> 
> v3:
>   * move clock bindings to clock folder
>   * drop ti,syscon-reset from bindings and example
>   * merge mcusys with topckgen bindings
> 
> v2:
>   * dropped unused labels
>   * add 'type: object' declaration for reset-controller found in new
>     ethwarp controller and represented as ti,syscon-reset
>   * rebase on top of
>     "dt-bindings: arm: mediatek: move ethsys controller & convert to DT schema"
> 
>   .../arm/mediatek/mediatek,infracfg.yaml       |  1 +
>   .../bindings/clock/mediatek,apmixedsys.yaml   |  1 +
>   .../bindings/clock/mediatek,ethsys.yaml       |  1 +
>   .../clock/mediatek,mt7988-ethwarp.yaml        | 66 +++++++++++++++++++
>   .../clock/mediatek,mt7988-xfi-pll.yaml        | 48 ++++++++++++++
>   .../bindings/clock/mediatek,topckgen.yaml     |  2 +
>   .../bindings/net/pcs/mediatek,sgmiisys.yaml   | 61 ++++++++++++++---
>   7 files changed, 171 insertions(+), 9 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7988-ethwarp.yaml
>   create mode 100644 Documentation/devicetree/bindings/clock/mediatek,mt7988-xfi-pll.yaml
> 

..snip..

> diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt7988-ethwarp.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt7988-ethwarp.yaml
> new file mode 100644
> index 0000000000000..e126f3fe0856c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/mediatek,mt7988-ethwarp.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,mt7988-ethwarp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MT7988 ethwarp Controller
> +
> +maintainers:
> +  - Daniel Golle <daniel@makrotopia.org>
> +
> +description:
> +  The Mediatek MT7988 ethwarp controller provides clocks and resets for the
> +  Ethernet related subsystems found the MT7988 SoC.
> +  The clock values can be found in <dt-bindings/clock/mt*-clk.h>.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: mediatek,mt7988-ethwarp
> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  reset-controller:
> +    type: object
> +    properties:
> +      compatible:
> +        const: ti,syscon-reset
> +
> +    # TODO: Convert to DT schema
> +    additionalProperties: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/reset/ti-syscon.h>
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        clock-controller@15031000 {
> +            compatible = "mediatek,mt7988-ethwarp", "syscon", "simple-mfd";
> +            reg = <0 0x15031000 0 0x1000>;
> +            #clock-cells = <1>;
> +
> +            reset-controller {
> +                compatible = "ti,syscon-reset";

NACK! This is not the right way to do it. Please do *not* use ti,syscon-reset.

Declare your resets in the relevant clock controller driver, like was done with
all (or the vast majority of) the other MediaTek clock drivers.

This means that you don't need simple-mfd here (and probably syscon as well).

Regards,
Angelo