[PATCH v3 0/5] Add imem clock for Exynos 5433

[PATCH v3 0/5] Add imem clock for Exynos 5433

[Kamil Konieczny]

Add imem clock for Exynos 5433. This will allow to use SSS and SlimSSS
(Security SubSystem) with crypto functions.

Changes since v2:
- added Reviewed-by to two first patches
- fixed subject for 3rd patch and commit message
- in patch 3 moved documentation of imem to end of clocks after CMU_CAM1
- in patch 4 moved cmu_imem declarations after cmu_cam1
- in patch 5 added newlines after clock registers, moved code after cmu_cam1,
  removed CLK_OF_DECLARE and exynos5433_cmu_imem_init, moved cmu_imem
  compatible to the end of exynos5433_cmu_of_match

Changes since v1:
- splitted typo patch into two, one of them for stable, suggested by Krzystof
  Kozlowski
- added more registers as suggested by Chanwoo Choi

Kamil Konieczny (5):
  clk: samsung: exynos5433: fix typo in imem divider
  clk: samsung: exynos5433: fix name typo in sssx
  dt-bindings: clk: exynos5433: document imem clock
  arm64: dts: exynos: add imem clock
  clk: samsung: exynos5433: add imem clocks

 .../bindings/clock/exynos5433-clock.txt       |  23 ++
 arch/arm64/boot/dts/exynos/exynos5433.dtsi    |  15 ++
 drivers/clk/samsung/clk-exynos5433.c          | 199 +++++++++++++++++-
 include/dt-bindings/clock/exynos5433.h        |  57 ++++-
 4 files changed, 290 insertions(+), 4 deletions(-)

-- 
2.19.1

[PATCH v3 1/5] clk: samsung: exynos5433: fix typo in imem divider

[Kamil Konieczny]

Fix typo in imem clock divider 200 switched with 266.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
 drivers/clk/samsung/clk-exynos5433.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index 751e2c4fb65b..ea47f49abc7f 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -568,10 +568,10 @@ static const struct samsung_gate_clock top_gate_clks[] __initconst = {
 	GATE(CLK_ACLK_BUS1_400, "aclk_bus1_400", "div_aclk_bus1_400",
 			ENABLE_ACLK_TOP, 25,
 			CLK_IS_CRITICAL | CLK_SET_RATE_PARENT, 0),
-	GATE(CLK_ACLK_IMEM_200, "aclk_imem_200", "div_aclk_imem_266",
+	GATE(CLK_ACLK_IMEM_200, "aclk_imem_200", "div_aclk_imem_200",
 			ENABLE_ACLK_TOP, 24,
 			CLK_IS_CRITICAL | CLK_SET_RATE_PARENT, 0),
-	GATE(CLK_ACLK_IMEM_266, "aclk_imem_266", "div_aclk_imem_200",
+	GATE(CLK_ACLK_IMEM_266, "aclk_imem_266", "div_aclk_imem_266",
 			ENABLE_ACLK_TOP, 23,
 			CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_ACLK_PERIC_66, "aclk_peric_66", "div_aclk_peric_66_b",
-- 
2.19.1

[PATCH v3 2/5] clk: samsung: exynos5433: fix name typo in sssx

[Kamil Konieczny]

Fix typo in sssx name, there should be three letters 's'.

Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
 drivers/clk/samsung/clk-exynos5433.c   | 2 +-
 include/dt-bindings/clock/exynos5433.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index ea47f49abc7f..24c3360db65b 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -559,7 +559,7 @@ static const struct samsung_gate_clock top_gate_clks[] __initconst = {
 	/* ENABLE_ACLK_TOP */
 	GATE(CLK_ACLK_G3D_400, "aclk_g3d_400", "div_aclk_g3d_400",
 			ENABLE_ACLK_TOP, 30, CLK_IS_CRITICAL, 0),
-	GATE(CLK_ACLK_IMEM_SSX_266, "aclk_imem_ssx_266",
+	GATE(CLK_ACLK_IMEM_SSSX_266, "aclk_imem_sssx_266",
 			"div_aclk_imem_sssx_266", ENABLE_ACLK_TOP,
 			29, CLK_IGNORE_UNUSED, 0),
 	GATE(CLK_ACLK_BUS0_400, "aclk_bus0_400", "div_aclk_bus0_400",
diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
index 98bd85ce1e45..87bb2b017143 100644
--- a/include/dt-bindings/clock/exynos5433.h
+++ b/include/dt-bindings/clock/exynos5433.h
@@ -156,7 +156,7 @@
 #define CLK_ACLK_G2D_266		220
 #define CLK_ACLK_G2D_400		221
 #define CLK_ACLK_G3D_400		222
-#define CLK_ACLK_IMEM_SSX_266		223
+#define CLK_ACLK_IMEM_SSSX_266		223
 #define CLK_ACLK_BUS0_400		224
 #define CLK_ACLK_BUS1_400		225
 #define CLK_ACLK_IMEM_200		226
-- 
2.19.1

Re: [PATCH v3 2/5] clk: samsung: exynos5433: fix name typo in sssx

[Rob Herring]

On Tue, Dec 04, 2018 at 05:52:45PM +0100, Kamil Konieczny wrote:
> Fix typo in sssx name, there should be three letters 's'.
> 
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos5433.c   | 2 +-
>  include/dt-bindings/clock/exynos5433.h | 2 +-

Acked-by: Rob Herring <robh@kernel.org>

>  2 files changed, 2 insertions(+), 2 deletions(-)

[PATCH v3 3/5] dt-bindings: clk: exynos5433: document imem clock

[Kamil Konieczny]

Document DT bindings for imem clock of the Samsung Exynos5433 SSS (Security
SubSystem) and SlimSSS IPs.

Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
 .../bindings/clock/exynos5433-clock.txt       | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
index 50d5897c9849..183c327a7d6b 100644
--- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
+++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
@@ -50,6 +50,8 @@ Required Properties:
     IPs.
   - "samsung,exynos5433-cmu-cam1" - clock controller compatible for CMU_CAM1
     which generates clocks for Cortex-A5/MIPI_CSIS2/FIMC-LITE_C/FIMC-FD IPs.
+  - "samsung,exynos5433-cmu-imem"   - clock controller compatible for CMU_IMEM
+    which generates clocks for SSS (Security SubSystem) and SlimSSS IPs.
 
 - reg: physical base address of the controller and length of memory mapped
   region.
@@ -168,6 +170,12 @@ Required Properties:
 		- aclk_cam1_400
 		- aclk_cam1_552
 
+	Input clocks for imem clock controller:
+		- oscclk
+		- aclk_imem_sssx_266
+		- aclk_imem_266
+		- aclk_imem_200
+
 Optional properties:
   - power-domains: a phandle to respective power domain node as described by
 	generic PM domain bindings (see power/power_domain.txt for more
@@ -469,6 +477,21 @@ Example 2: Examples of clock controller nodes are listed below.
 		power-domains = <&pd_cam1>;
 	};
 
+	cmu_imem: clock-controller@11060000 {
+		compatible = "samsung,exynos5433-cmu-imem";
+		reg = <0x11060000 0x1000>;
+		#clock-cells = <1>;
+
+		clock-names = "oscclk",
+			"aclk_imem_sssx_266",
+			"aclk_imem_266",
+			"aclk_imem_200";
+		clocks = <&xxti>,
+			<&cmu_top CLK_DIV_ACLK_IMEM_SSSX_266>,
+			<&cmu_top CLK_DIV_ACLK_IMEM_266>,
+			<&cmu_top CLK_DIV_ACLK_IMEM_200>;
+	};
+
 Example 3: UART controller node that consumes the clock generated by the clock
 	   controller.
 
-- 
2.19.1

Re: [PATCH v3 3/5] dt-bindings: clk: exynos5433: document imem clock

[Stephen Boyd]

Quoting Kamil Konieczny (2018-12-04 08:52:46)
> Document DT bindings for imem clock of the Samsung Exynos5433 SSS (Security
> SubSystem) and SlimSSS IPs.
> 
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---

Acked-by: Stephen Boyd <sboyd@kernel.org>

Re: [PATCH v3 3/5] dt-bindings: clk: exynos5433: document imem clock

[Rob Herring]

On Tue,  4 Dec 2018 17:52:46 +0100, Kamil Konieczny wrote:
> Document DT bindings for imem clock of the Samsung Exynos5433 SSS (Security
> SubSystem) and SlimSSS IPs.
> 
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
>  .../bindings/clock/exynos5433-clock.txt       | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 3/5] dt-bindings: clk: exynos5433: document imem clock

[Chanwoo Choi]

Hi Kamil,

On 2018년 12월 05일 01:52, Kamil Konieczny wrote:
> Document DT bindings for imem clock of the Samsung Exynos5433 SSS (Security
> SubSystem) and SlimSSS IPs.
> 
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
>  .../bindings/clock/exynos5433-clock.txt       | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

> 
> diff --git a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> index 50d5897c9849..183c327a7d6b 100644
> --- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> @@ -50,6 +50,8 @@ Required Properties:
>      IPs.
>    - "samsung,exynos5433-cmu-cam1" - clock controller compatible for CMU_CAM1
>      which generates clocks for Cortex-A5/MIPI_CSIS2/FIMC-LITE_C/FIMC-FD IPs.
> +  - "samsung,exynos5433-cmu-imem"   - clock controller compatible for CMU_IMEM
> +    which generates clocks for SSS (Security SubSystem) and SlimSSS IPs.
>  
>  - reg: physical base address of the controller and length of memory mapped
>    region.
> @@ -168,6 +170,12 @@ Required Properties:
>  		- aclk_cam1_400
>  		- aclk_cam1_552
>  
> +	Input clocks for imem clock controller:
> +		- oscclk
> +		- aclk_imem_sssx_266
> +		- aclk_imem_266
> +		- aclk_imem_200
> +
>  Optional properties:
>    - power-domains: a phandle to respective power domain node as described by
>  	generic PM domain bindings (see power/power_domain.txt for more
> @@ -469,6 +477,21 @@ Example 2: Examples of clock controller nodes are listed below.
>  		power-domains = <&pd_cam1>;
>  	};
>  
> +	cmu_imem: clock-controller@11060000 {
> +		compatible = "samsung,exynos5433-cmu-imem";
> +		reg = <0x11060000 0x1000>;
> +		#clock-cells = <1>;
> +
> +		clock-names = "oscclk",
> +			"aclk_imem_sssx_266",
> +			"aclk_imem_266",
> +			"aclk_imem_200";
> +		clocks = <&xxti>,
> +			<&cmu_top CLK_DIV_ACLK_IMEM_SSSX_266>,
> +			<&cmu_top CLK_DIV_ACLK_IMEM_266>,
> +			<&cmu_top CLK_DIV_ACLK_IMEM_200>;
> +	};
> +
>  Example 3: UART controller node that consumes the clock generated by the clock
>  	   controller.
>  
> 

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

[PATCH v3 4/5] arm64: dts: exynos: add imem clock

[Kamil Konieczny]

Add description for imem clock in the Samsung Exynos5433. The users can use
compatibility "samsung,exynos5433-cmu-imem".

Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
 arch/arm64/boot/dts/exynos/exynos5433.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index 2131f12364cb..378cd3d87ff0 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -544,6 +544,21 @@
 			power-domains = <&pd_cam1>;
 		};
 
+		cmu_imem: clock-controller@11060000 {
+			compatible = "samsung,exynos5433-cmu-imem";
+			reg = <0x11060000 0x1000>;
+			#clock-cells = <1>;
+
+			clock-names = "oscclk",
+				"aclk_imem_sssx_266",
+				"aclk_imem_266",
+				"aclk_imem_200";
+			clocks = <&xxti>,
+				<&cmu_top CLK_DIV_ACLK_IMEM_SSSX_266>,
+				<&cmu_top CLK_DIV_ACLK_IMEM_266>,
+				<&cmu_top CLK_DIV_ACLK_IMEM_200>;
+		};
+
 		pd_gscl: power-domain@105c4000 {
 			compatible = "samsung,exynos5433-pd";
 			reg = <0x105c4000 0x20>;
-- 
2.19.1

Re: [PATCH v3 4/5] arm64: dts: exynos: add imem clock

[Krzysztof Kozlowski]

On Tue, Dec 04, 2018 at 05:52:47PM +0100, Kamil Konieczny wrote:
> Add description for imem clock in the Samsung Exynos5433. The users can use
> compatibility "samsung,exynos5433-cmu-imem".

Thanks, applied but the commit message above is not matching contents at all.
You did not add the description. Mentioning some users and compatibility
also seems unrelated. Instead:
1. You added DT node for IMEM clock controller on Exynos5433.
2. Because it is necessary for SSS to work (beside obvious reasons: for
   DTS completeness and allowing to gate them when not used).
Also, please use SoC name in commit title (for example at the end) to
differentiate from Exynos7 or any other future SoCs.

Best regards,
Krzysztof

Re: [PATCH v3 4/5] arm64: dts: exynos: add imem clock

[Kamil Konieczny]

On 13.12.2018 22:10, Krzysztof Kozlowski wrote:
> On Tue, Dec 04, 2018 at 05:52:47PM +0100, Kamil Konieczny wrote:
>> Add description for imem clock in the Samsung Exynos5433. The users can use
>> compatibility "samsung,exynos5433-cmu-imem".
> 
> Thanks, applied but the commit message above is not matching contents at all.
> You did not add the description. Mentioning some users and compatibility
> also seems unrelated. Instead:
> 1. You added DT node for IMEM clock controller on Exynos5433.
> 2. Because it is necessary for SSS to work (beside obvious reasons: for
>    DTS completeness and allowing to gate them when not used).
> Also, please use SoC name in commit title (for example at the end) to
> differentiate from Exynos7 or any other future SoCs.

Thank you for your work and insightfull comments, I hope to improve my
commit messages.

-- 
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland