Re: [PATCH v3 2/5] clk: samsung: Add dt bindings for Exynos4412 ISP clock controller

[Krzysztof Kozlowski <krzk@kernel.org>]

On Wed, Oct 11, 2017 at 11:25:12AM +0200, Marek Szyprowski wrote:
> Some registers for the Exynos 4412 ISP (Camera subsystem) clocks are
> located in the ISP power domain. Because those registers are also
> located in a different memory region than the main clock controller,
> support for them can be provided by a separate clock controller.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/clock/exynos4-clock.txt    | 43 ++++++++++++++++++++++
>  include/dt-bindings/clock/exynos4.h                | 35 ++++++++++++++++++
>  2 files changed, 78 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/exynos4-clock.txt b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
> index f5a5b19ed3b2..bc61c952cb0b 100644
> --- a/Documentation/devicetree/bindings/clock/exynos4-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/exynos4-clock.txt
> @@ -41,3 +41,46 @@ Example 2: UART controller node that consumes the clock generated by the clock
>  		clocks = <&clock CLK_UART2>, <&clock CLK_SCLK_UART2>;
>  		clock-names = "uart", "clk_uart_baud0";
>  	};
> +
> +Exynos4412 SoC contains some additional clocks for FIMC-ISP (Camera ISP)
> +subsystem. Registers for those clocks are located in the ISP power domain.
> +Because those registers are also located in a different memory region than
> +the main clock controller, a separate clock controller has to be defined for
> +handling them.
> +
> +Required Properties:
> +
> +- compatible: should be "samsung,exynos4412-isp-clock".
> +
> +- reg: physical base address of the ISP clock controller and length of memory
> +  mapped region.
> +
> +- #clock-cells: should be 1.
> +
> +- clocks: list of the clock controller input clock identifiers,
> +  from common clock bindings, should point to CLK_ACLK200 and
> +  CLK_ACLK400_MCUISP clocks from the main clock controller.
> +
> +- clock-names: list of the clock controller input clock names,
> +  as described in clock-bindings.txt, should be "aclk200" and
> +  "aclk400_mcuisp".
> +
> +- power-domains: a phandle to ISP power domain node as described by
> +  generic PM domain bindings.
> +
> +Example 3: The clock controllers bindings for Exynos4412 SoCs.
> +
> +	clock: clock-controller@10030000 {
> +		compatible = "samsung,exynos4412-clock";
> +		reg = <0x10030000 0x18000>;
> +		#clock-cells = <1>;
> +	};
> +
> +	isp_clock: clock-controller@10048000 {
> +		compatible = "samsung,exynos4412-isp-clock";
> +		reg = <0x10048000 0x1000>;
> +		#clock-cells = <1>;
> +		power-domains = <&pd_isp>;
> +		clocks = <&clock CLK_ACLK200>, <&clock CLK_ACLK400_MCUISP>;
> +		clock-names = "aclk200", "aclk400_mcuisp";
> +	};
> diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
> index c40111f36d5e..e9f9d400c322 100644
> --- a/include/dt-bindings/clock/exynos4.h
> +++ b/include/dt-bindings/clock/exynos4.h
> @@ -272,4 +272,39 @@
>  /* must be greater than maximal clock id */
>  #define CLK_NR_CLKS		461
>  
> +/* Exynos4x12 ISP clocks */

Shouldn't the clock IDs go with driver changes? I think only the
Documentation/ part should be separate.

Best regards,
Krzysztof


> +#define CLK_ISP_FIMC_ISP		 1
> +#define CLK_ISP_FIMC_DRC		 2
> +#define CLK_ISP_FIMC_FD			 3
> +#define CLK_ISP_FIMC_LITE0		 4
> +#define CLK_ISP_FIMC_LITE1		 5
> +#define CLK_ISP_MCUISP			 6
> +#define CLK_ISP_GICISP			 7
> +#define CLK_ISP_SMMU_ISP		 8
> +#define CLK_ISP_SMMU_DRC		 9
> +#define CLK_ISP_SMMU_FD			10
> +#define CLK_ISP_SMMU_LITE0		11
> +#define CLK_ISP_SMMU_LITE1		12
> +#define CLK_ISP_PPMUISPMX		13
> +#define CLK_ISP_PPMUISPX		14
> +#define CLK_ISP_MCUCTL_ISP		15
> +#define CLK_ISP_MPWM_ISP		16
> +#define CLK_ISP_I2C0_ISP		17
> +#define CLK_ISP_I2C1_ISP		18
> +#define CLK_ISP_MTCADC_ISP		19
> +#define CLK_ISP_PWM_ISP			20
> +#define CLK_ISP_WDT_ISP			21
> +#define CLK_ISP_UART_ISP		22
> +#define CLK_ISP_ASYNCAXIM		23
> +#define CLK_ISP_SMMU_ISPCX		24
> +#define CLK_ISP_SPI0_ISP		25
> +#define CLK_ISP_SPI1_ISP		26
> +
> +#define CLK_ISP_DIV_ISP0		27
> +#define CLK_ISP_DIV_ISP1		28
> +#define CLK_ISP_DIV_MCUISP0		29
> +#define CLK_ISP_DIV_MCUISP1		30
> +
> +#define CLK_NR_ISP_CLKS			31
> +
>  #endif /* _DT_BINDINGS_CLOCK_EXYNOS_4_H */
> -- 
> 2.14.2
>