| Message ID | 20171002104759.25944-1-m.szyprowski@samsung.com |
|---|---|
| Headers | show |
| Series | Fix problems with Exynos4412 ISP clocks | expand |
On Mon, Oct 02, 2017 at 12:47:55PM +0200, Marek Szyprowski wrote: > Hi! > > Exynos4412 ISP clock controller is located in the SOC area, which belongs > to ISP power domain. This was not properly handled by the current > Exynos4-clk driver. This patchset instantiates a separate clock driver > for those clocks, updates all clients of ISP clocks and ensures that > the driver is properly integrated in ISP power domin using runtime PM > feature of the clock framework. > > This finally solves all the mysterious freezes in accessing ISP clocks > when ISP power domain is disabled. > > The last patch breaks support for old dtbs. It can be applied when all > boards are updated. Exynos4412 ISP subsystem is only used by Trats2 > boards, for which kernel is updated always together with the dtb file, > so the last patch can be applied to the next kernel release after merging > the DTS patch. I am fine with this approach. However I see that third patch (dts) depends on previous so it will also wait one cycle. arm-soc folks for some time do not accept code mixed with dts. Unless you can split dts into two patches: adding new clock controller and removing old. In such case the new clock could be applied in parallel to clk code changes. Best regards, Krzysztof > > This patchset requires clocks runtime PM support ("Add runtime PM support > for clocks (on Exynos SoC example)" v9 patchset), which I hope to be > merged soon to clk-next. > > Best regards > Marek Szyprowski > Samsung R&D Institute Poland > > > Patch summary: > > Marek Szyprowski (4): > clk: samsung: Instantiate Exynos4412 ISP clocks only when available > clk: samsung: Add a separate driver for Exynos4412 ISP clocks > ARM: dts: exynos: Add Exynos4412 ISP clock controller > clk: samsung: Remove obsolete code for Exynos4412 ISP clocks > > .../devicetree/bindings/clock/exynos4-clock.txt | 27 ++++ > arch/arm/boot/dts/exynos4412.dtsi | 71 ++++---- > drivers/clk/samsung/Makefile | 1 + > drivers/clk/samsung/clk-exynos4.c | 66 +------- > drivers/clk/samsung/clk-exynos4412-isp.c | 179 +++++++++++++++++++++ > include/dt-bindings/clock/exynos4.h | 65 ++++---- > 6 files changed, 287 insertions(+), 122 deletions(-) > create mode 100644 drivers/clk/samsung/clk-exynos4412-isp.c > > -- > 2.14.2 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Krzysztof, On 2017-10-02 19:36, Krzysztof Kozlowski wrote: > On Mon, Oct 02, 2017 at 12:47:55PM +0200, Marek Szyprowski wrote: >> Exynos4412 ISP clock controller is located in the SOC area, which belongs >> to ISP power domain. This was not properly handled by the current >> Exynos4-clk driver. This patchset instantiates a separate clock driver >> for those clocks, updates all clients of ISP clocks and ensures that >> the driver is properly integrated in ISP power domin using runtime PM >> feature of the clock framework. >> >> This finally solves all the mysterious freezes in accessing ISP clocks >> when ISP power domain is disabled. >> >> The last patch breaks support for old dtbs. It can be applied when all >> boards are updated. Exynos4412 ISP subsystem is only used by Trats2 >> boards, for which kernel is updated always together with the dtb file, >> so the last patch can be applied to the next kernel release after merging >> the DTS patch. > I am fine with this approach. However I see that third patch (dts) > depends on previous so it will also wait one cycle. arm-soc folks for > some time do not accept code mixed with dts. > > Unless you can split dts into two patches: adding new clock controller > and removing old. In such case the new clock could be applied in > parallel to clk code changes. DTS patch cannot be split into two patches without breaking ISP support, because adding a new clock controller requires changes in the clients and register resource size of the old one, so either old or new one has to be fully defined. If arm-soc cannot pull mixed branches, then I'm okay with postponing DTS changes by one release. > ... Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/02/2017 12:47 PM, Marek Szyprowski wrote: > Patch summary: > > Marek Szyprowski (4): > clk: samsung: Instantiate Exynos4412 ISP clocks only when available > clk: samsung: Add a separate driver for Exynos4412 ISP clocks > ARM: dts: exynos: Add Exynos4412 ISP clock controller > clk: samsung: Remove obsolete code for Exynos4412 ISP clocks I have applied whole series, with v2 of patch 2/4, thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 9, 2017 at 12:34 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 10/02/2017 12:47 PM, Marek Szyprowski wrote: >> Patch summary: >> >> Marek Szyprowski (4): >> clk: samsung: Instantiate Exynos4412 ISP clocks only when available >> clk: samsung: Add a separate driver for Exynos4412 ISP clocks >> ARM: dts: exynos: Add Exynos4412 ISP clock controller >> clk: samsung: Remove obsolete code for Exynos4412 ISP clocks > > I have applied whole series, with v2 of patch 2/4, thanks. You mean you applied 1/4 and 2/4 for now, right? 3/4 will go through my tree in release after this and 4/4 even later? BR, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/09/2017 12:49 PM, Krzysztof Kozlowski wrote: > On Mon, Oct 9, 2017 at 12:34 PM, Sylwester Nawrocki > <s.nawrocki@samsung.com> wrote: >> On 10/02/2017 12:47 PM, Marek Szyprowski wrote: >>> Patch summary: >>> >>> Marek Szyprowski (4): >>> clk: samsung: Instantiate Exynos4412 ISP clocks only when available >>> clk: samsung: Add a separate driver for Exynos4412 ISP clocks >>> ARM: dts: exynos: Add Exynos4412 ISP clock controller >>> clk: samsung: Remove obsolete code for Exynos4412 ISP clocks >> I have applied whole series, with v2 of patch 2/4, thanks. > You mean you applied 1/4 and 2/4 for now, right? 3/4 will go through > my tree in release after this and 4/4 even later? Ah yes, sorry, just noticed that. Indeed I've taken only first two patches, the third is for dts and the last one will be applied after some time as you say. -- Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 02, 2017 at 12:47:57PM +0200, Marek Szyprowski wrote: > Some registers for the Exynos 4412 ISP (Camera subsystem) clocks are > partially located in the SOC area, which belongs to 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. This in turn allows to almost seamlessly > make it aware of the power domain using recently introduced runtime > PM support for clocks. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > .../devicetree/bindings/clock/exynos4-clock.txt | 27 ++++ Please split binding doc and include to separate patch. > drivers/clk/samsung/Makefile | 1 + > drivers/clk/samsung/clk-exynos4412-isp.c | 179 +++++++++++++++++++++ > include/dt-bindings/clock/exynos4.h | 35 ++++ > 4 files changed, 242 insertions(+) > create mode 100644 drivers/clk/samsung/clk-exynos4412-isp.c > > diff --git a/Documentation/devicetree/bindings/clock/exynos4-clock.txt b/Documentation/devicetree/bindings/clock/exynos4-clock.txt > index f5a5b19ed3b2..9b260e0d1e66 100644 > --- a/Documentation/devicetree/bindings/clock/exynos4-clock.txt > +++ b/Documentation/devicetree/bindings/clock/exynos4-clock.txt > @@ -41,3 +41,30 @@ 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 partially located in the SOC area, The comma here should be dropped. Can't you just say "...located in the ISP power domain."? What does partially mean? Some registers aren't in the ISP power domain? Where are they then? > +which belongs to 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. The compatible string to such > +controller is "samsung,exynos4412-isp-clock". It also has two input clocks Once you have a 2nd compatible string, this sentence doesn't work. > +from the main Exynos4412 clock controller: "aclk200" and "aclk400_mcuisp". > +The ISP clock controller has to be linked with respective ISP power domain > +(for more information, see Samsung Exynos power domains bindings). List out properties and their constraints, not prose describing the binding. > + > +Example 3: An example of a clock controllers 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"; > + }; An example is not a binding. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/09/2017 12:58 PM, Sylwester Nawrocki wrote: > On 10/09/2017 12:49 PM, Krzysztof Kozlowski wrote: >> On Mon, Oct 9, 2017 at 12:34 PM, Sylwester Nawrocki >> <s.nawrocki@samsung.com> wrote: >>> On 10/02/2017 12:47 PM, Marek Szyprowski wrote: >>>> Patch summary: >>>> >>>> Marek Szyprowski (4): >>>> clk: samsung: Instantiate Exynos4412 ISP clocks only when available >>>> clk: samsung: Add a separate driver for Exynos4412 ISP clocks >>>> ARM: dts: exynos: Add Exynos4412 ISP clock controller >>>> clk: samsung: Remove obsolete code for Exynos4412 ISP clocks >>> I have applied whole series, with v2 of patch 2/4, thanks. >> You mean you applied 1/4 and 2/4 for now, right? 3/4 will go through >> my tree in release after this and 4/4 even later? > Ah yes, sorry, just noticed that. Indeed I've taken only first two > patches, the third is for dts and the last one will be applied after > some time as you say. For the record, I have dropped these 2 patches as there is now posted v3 addressing review comments. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi! Exynos4412 ISP clock controller is located in the SOC area, which belongs to ISP power domain. This was not properly handled by the current Exynos4-clk driver. This patchset instantiates a separate clock driver for those clocks, updates all clients of ISP clocks and ensures that the driver is properly integrated in ISP power domin using runtime PM feature of the clock framework. This finally solves all the mysterious freezes in accessing ISP clocks when ISP power domain is disabled. The last patch breaks support for old dtbs. It can be applied when all boards are updated. Exynos4412 ISP subsystem is only used by Trats2 boards, for which kernel is updated always together with the dtb file, so the last patch can be applied to the next kernel release after merging the DTS patch. This patchset requires clocks runtime PM support ("Add runtime PM support for clocks (on Exynos SoC example)" v9 patchset), which I hope to be merged soon to clk-next. Best regards Marek Szyprowski Samsung R&D Institute Poland Patch summary: Marek Szyprowski (4): clk: samsung: Instantiate Exynos4412 ISP clocks only when available clk: samsung: Add a separate driver for Exynos4412 ISP clocks ARM: dts: exynos: Add Exynos4412 ISP clock controller clk: samsung: Remove obsolete code for Exynos4412 ISP clocks .../devicetree/bindings/clock/exynos4-clock.txt | 27 ++++ arch/arm/boot/dts/exynos4412.dtsi | 71 ++++---- drivers/clk/samsung/Makefile | 1 + drivers/clk/samsung/clk-exynos4.c | 66 +------- drivers/clk/samsung/clk-exynos4412-isp.c | 179 +++++++++++++++++++++ include/dt-bindings/clock/exynos4.h | 65 ++++---- 6 files changed, 287 insertions(+), 122 deletions(-) create mode 100644 drivers/clk/samsung/clk-exynos4412-isp.c -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html