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

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hi Krzysztof,

On 2017-10-11 19:05, Krzysztof Kozlowski wrote:
> 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.

Well, Rob asked to move dt bindings and dt include to the separate patch.

I see no value-added by such split, as both dt and driver patches will be
processed together anyway, but I didn't want to have this patchset blocked
by this issue.

 > ...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland