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

[PATCH v2 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 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
  arm64: dts: bindings: document imem clock
  arm64: dts: exynos: add imem clock
  clk: samsung: exynos5433: add imem clock

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

-- 
2.19.1

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

[Kamil Konieczny]

Fix typo in imem clock divider 200 switched with 266.

Fixes: 5785d6e61f27 ("clk: samsung: exynos5433: Add clocks for CMU_BUS{0|1|2} domains")
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 v2 2/5] clk: samsung: exynos5433: fix name typo in sssx

[Kamil Konieczny]

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

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

[PATCH v2 3/5] arm64: dts: bindings: document imem clock

[Kamil Konieczny]

Document imem clock bindings for 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..4e4352bf5a0b 100644
--- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
+++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
@@ -19,6 +19,8 @@ Required Properties:
     which generates clocks for PMU/TMU/MCT/WDT/RTC/SECKEY/TZPC IPs.
   - "samsung,exynos5433-cmu-fsys"  - clock controller compatible for CMU_FSYS
     which generates clocks for USB/UFS/SDMMC/TSI/PDMA IPs.
+  - "samsung,exynos5433-cmu-imem"   - clock controller compatible for CMU_IMEM
+    which generates clocks for SSS (Security SubSystem) and SlimSSS IPs.
   - "samsung,exynos5433-cmu-g2d"   - clock controller compatible for CMU_G2D
     which generates clocks for G2D/MDMA IPs.
   - "samsung,exynos5433-cmu-disp"  - clock controller compatible for CMU_DISP
@@ -88,6 +90,12 @@ Required Properties:
 		- sclk_usbhost30_fsys
 		- sclk_usbdrd30_fsys
 
+	Input clocks for imem clock controller:
+		- oscclk
+		- aclk_imem_sssx_266
+		- aclk_imem_266
+		- aclk_imem_200
+
 	Input clocks for g2d clock controller:
 		- oscclk
 		- aclk_g2d_266
@@ -264,6 +272,21 @@ Example 2: Examples of clock controller nodes are listed below.
 		       <&cmu_top CLK_SCLK_USBDRD30_FSYS>;
 	};
 
+	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>;
+	};
+
 	cmu_g2d: clock-controller@12460000 {
 		compatible = "samsung,exynos5433-cmu-g2d";
 		reg = <0x12460000 0x0b08>;
-- 
2.19.1

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

[Kamil Konieczny]

Add description for imem clock. 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..34055fa3a9d0 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -340,6 +340,21 @@
 				<&cmu_top CLK_SCLK_USBDRD30_FSYS>;
 		};
 
+		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>;
+		};
+
 		cmu_g2d: clock-controller@12460000 {
 			compatible = "samsung,exynos5433-cmu-g2d";
 			reg = <0x12460000 0x1000>;
-- 
2.19.1

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

[Chanwoo Choi]

Hi,

On 2018년 11월 30일 00:51, Kamil Konieczny wrote:
> Fix typo in imem clock divider 200 switched with 266.
> 
> Fixes: 5785d6e61f27 ("clk: samsung: exynos5433: Add clocks for CMU_BUS{0|1|2} domains")
> 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",
> 

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

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

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

[Chanwoo Choi]

Hi,

On 2018년 11월 30일 00:51, Kamil Konieczny wrote:
> Fix typo in sssx name, there should be three letters 's'.
> 
> 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
> 

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

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 3/5] arm64: dts: bindings: document imem clock

[Stephen Boyd]

Quoting Kamil Konieczny (2018-11-29 07:51:32)
> Document imem clock bindings for SSS (Security SubSystem) and SlimSSS IPs.
> 
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---

Fix the subject of this patch please. It's not arm64: or dts:

>  .../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..4e4352bf5a0b 100644
> --- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> @@ -264,6 +272,21 @@ Example 2: Examples of clock controller nodes are listed below.
>                        <&cmu_top CLK_SCLK_USBDRD30_FSYS>;
>         };
>  
> +       cmu_imem: clock-controller@11060000 {
> +               compatible = "samsung,exynos5433-cmu-imem";
> +               reg = <0x11060000 0x1000>;
> +               #clock-cells = <1>;
> +
> +               clock-names = "oscclk",

How about just osc? clk seems redundant.

> +                       "aclk_imem_sssx_266",
> +                       "aclk_imem_266",
> +                       "aclk_imem_200";

And what is 'aclk'? Also redundant?

> +               clocks = <&xxti>,
> +                       <&cmu_top CLK_DIV_ACLK_IMEM_SSSX_266>,
> +                       <&cmu_top CLK_DIV_ACLK_IMEM_266>,
> +                       <&cmu_top CLK_DIV_ACLK_IMEM_200>;
> +       };
> +
>         cmu_g2d: clock-controller@12460000 {
>                 compatible = "samsung,exynos5433-cmu-g2d";
>                 reg = <0x12460000 0x0b08>;
> -- 
> 2.19.1
> 

Re: [PATCH v2 3/5] arm64: dts: bindings: document imem clock

[Sylwester Nawrocki]

Hi,

On 11/30/18 00:42, Stephen Boyd wrote:
> Quoting Kamil Konieczny (2018-11-29 07:51:32)
>> Document imem clock bindings for SSS (Security SubSystem) and SlimSSS IPs.
>>
>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>> ---
> 
> Fix the subject of this patch please. It's not arm64: or dts:
> 
>>  .../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..4e4352bf5a0b 100644
>> --- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
>> +++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
>> @@ -264,6 +272,21 @@ Example 2: Examples of clock controller nodes are listed below.
>>                        <&cmu_top CLK_SCLK_USBDRD30_FSYS>;
>>         };
>>  
>> +       cmu_imem: clock-controller@11060000 {
>> +               compatible = "samsung,exynos5433-cmu-imem";
>> +               reg = <0x11060000 0x1000>;
>> +               #clock-cells = <1>;
>> +
>> +               clock-names = "oscclk",
> 
> How about just osc? clk seems redundant.

Can we keep this "oscclk" name? This patch just adds missing definition
of one of multiple CMUs in the SoC and for the all other CMUs "oscclk"
name has been adopted for the OSCCLK root oscillator clock. The name is 
exactly as on the diagrams in the datasheet, just in lower case.
To avoid confusion I would prefer to stay with same name as used for the
other CMUs.

>> +                       "aclk_imem_sssx_266",
>> +                       "aclk_imem_266",
>> +                       "aclk_imem_200";
> 
> And what is 'aclk'? Also redundant?

Again the names used here are exactly as in the SoC datasheet. I know we 
could use shorted names for these consumer clocks however at this point 
I would prefer to use same convention as for the other CMUs.

ACLK means AXI bus clock, the IMEM CMU is basically just gates that pass
the clocks further to the SoC peripheral blocks.

>> +               clocks = <&xxti>,
>> +                       <&cmu_top CLK_DIV_ACLK_IMEM_SSSX_266>,
>> +                       <&cmu_top CLK_DIV_ACLK_IMEM_266>,
>> +                       <&cmu_top CLK_DIV_ACLK_IMEM_200>;
>> +       };
>> 
-- 
Regards,
Sylwester

Re: [PATCH v2 3/5] arm64: dts: bindings: document imem clock

[Stephen Boyd]

Quoting Sylwester Nawrocki (2018-12-03 02:44:17)
> Hi,
> 
> On 11/30/18 00:42, Stephen Boyd wrote:
> > Quoting Kamil Konieczny (2018-11-29 07:51:32)
> >> Document imem clock bindings for SSS (Security SubSystem) and SlimSSS IPs.
> >>
> >> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> >> ---
> > 
> > Fix the subject of this patch please. It's not arm64: or dts:
> > 
> >>  .../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..4e4352bf5a0b 100644
> >> --- a/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> >> +++ b/Documentation/devicetree/bindings/clock/exynos5433-clock.txt
> >> @@ -264,6 +272,21 @@ Example 2: Examples of clock controller nodes are listed below.
> >>                        <&cmu_top CLK_SCLK_USBDRD30_FSYS>;
> >>         };
> >>  
> >> +       cmu_imem: clock-controller@11060000 {
> >> +               compatible = "samsung,exynos5433-cmu-imem";
> >> +               reg = <0x11060000 0x1000>;
> >> +               #clock-cells = <1>;
> >> +
> >> +               clock-names = "oscclk",
> > 
> > How about just osc? clk seems redundant.
> 
> Can we keep this "oscclk" name? This patch just adds missing definition
> of one of multiple CMUs in the SoC and for the all other CMUs "oscclk"
> name has been adopted for the OSCCLK root oscillator clock. The name is 
> exactly as on the diagrams in the datasheet, just in lower case.
> To avoid confusion I would prefer to stay with same name as used for the
> other CMUs.

Ok.

> 
> >> +                       "aclk_imem_sssx_266",
> >> +                       "aclk_imem_266",
> >> +                       "aclk_imem_200";
> > 
> > And what is 'aclk'? Also redundant?
> 
> Again the names used here are exactly as in the SoC datasheet. I know we 
> could use shorted names for these consumer clocks however at this point 
> I would prefer to use same convention as for the other CMUs.
> 
> ACLK means AXI bus clock, the IMEM CMU is basically just gates that pass
> the clocks further to the SoC peripheral blocks.

Sure! Was just hoping to avoid 'clk' everywhere.