Re: [PATCH 3/5] dt-bindings: clk: Add Baikal-T1 System Devices CCU bindings

[Stephen Boyd]

Quoting Sergey.Semin@xxxxxxxxxxxxxxxxxxxx (2020-03-06 05:00:46)
> diff --git a/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
> new file mode 100644
> index 000000000000..aea09fbafc89
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/be,bt1-ccu-sys.yaml
> @@ -0,0 +1,169 @@
[..]
> +  assigned-clock-rates: true
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/bt1-ccu.h>
> +
> +    ccu_sys: ccu_sys@1F04D060 {

Node name should be clock-controller@1f04d060.

Also, binding looks wrong because that address isn't aligned. Most
likely it's one hardware block that has many different functionalities
so splitting it up into different regions isn't doing anything besides
logically splitting up the register space for software benefits.

> +      compatible = "be,bt1-ccu-sys";
> +      reg = <0x1F04D060 0x0A0>,
> +            <0x1F04D150 0x004>;
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;
> +
> +      clocks = <&osc25>,
> +               <&ccu_pll CCU_SATA_PLL>,
> +               <&ccu_pll CCU_PCIE_PLL>,
> +               <&ccu_pll CCU_ETH_PLL>;
> +      clock-names = "ref_clk", "sata_clk", "pcie_clk",
> +                    "eth_clk";
> +
> +      clock-output-names = "sys_sata_ref_clk", "sys_apb_clk",
> +                           "sys_gmac0_csr_clk", "sys_gmac0_tx_clk",
> +                           "sys_gmac0_ptp_clk", "sys_gmac1_csr_clk",
> +                           "sys_gmac1_tx_clk", "sys_gmac1_ptp_clk",
> +                           "sys_xgmac_ref_clk", "sys_xgmac_ptp_clk",
> +                           "sys_usb_clk", "sys_pvt_clk",
> +                           "sys_hwa_clk", "sys_uart_clk",
> +                           "sys_spi_clk", "sys_i2c1_clk",
> +                           "sys_i2c2_clk", "sys_gpio_clk",
> +                           "sys_timer0_clk", "sys_timer1_clk",
> +                           "sys_timer2_clk", "sys_wdt_clk";
> +      };
> +...
> diff --git a/include/dt-bindings/reset/bt1-ccu.h b/include/dt-bindings/reset/bt1-ccu.h
> index 4de5b6bcd433..0bd8fd0edb41 100644
> --- a/include/dt-bindings/reset/bt1-ccu.h
> +++ b/include/dt-bindings/reset/bt1-ccu.h
> @@ -20,4 +20,8 @@
>  #define CCU_AXI_HWA_RST                        9
>  #define CCU_AXI_SRAM_RST               10
>  
> +/* Baikal-T1 System Devices CCU Reset indeces. */

indeces is not a word.

> +#define CCU_SYS_SATA_REF_RST           0
> +#define CCU_SYS_APB_RST                        1
> +