On Thu, Oct 12, 2017 at 09:08:32AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Oct 12, 2017 at 8:47 AM, Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx> wrote:
> > 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@xxxxxxxxxxx>
> >>> ---
> >>> .../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.
First, I mainly review the bindings and not the drivers and splitting
them more accurately captures what was reviewed. Second, for the DT only
tree generated from the kernel using git-filter-branch[1], the commit
history will be more logical if it is keeping and dropping entire
commits rather than 1/2 a commit.
> I understand... actually I think it does not matter where the clock
> IDs would go. From my perspective both approaches look fine so in any
> case:
> Acked-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
The IDs are part of the ABI which is defined by the binding doc.
Rob
[1] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
