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

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

[Kamil Konieczny]

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

Changes since v4:
- added Acked-by to 5th patch,
- removed all clock ids except CLK_ACLK_SLIMSSS/CLK_PCLK_SLIMSSS and rewrite
  commit message.

Changes since v3:
- added Acked-by and Reviewed-by to patches 2 and 3
- dropped "[PATCH v3 4/5] arm64: dts: exynos: add imem clock" because it
  was already applied
- split "[PATCH v3 5/5] clk: samsung: exynos5433: add imem clocks"
  into two parts, first for DT bindings and second for clock driver. In
  clock driver I dropped all SSS clocks and left only two slimSSS, as I
  was unable to test SSS with crypto driver s5p-sss.

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 Krzysztof
  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
  dt-bindings: clk: exynos5433: add imem clock
  clk: samsung: exynos5433: add imem clocks

 .../bindings/clock/exynos5433-clock.txt       | 23 +++++++++++
 drivers/clk/samsung/clk-exynos5433.c          | 38 +++++++++++++++++--
 include/dt-bindings/clock/exynos5433.h        |  8 +++-
 3 files changed, 65 insertions(+), 4 deletions(-)

-- 
2.20.0

[PATCH v5 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.20.0

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

[Greg KH]

On Tue, Jan 22, 2019 at 10:02:28AM +0100, Kamil Konieczny wrote:
> 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(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

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

[Kamil Konieczny]

On 22.01.2019 10:33, Greg KH wrote:
> On Tue, Jan 22, 2019 at 10:02:28AM +0100, Kamil Konieczny wrote:
>> 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(-)
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read: [...]
After reconsideration that there are no users of imem clock in old kernels,
please drop this patch from stable.

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

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

[Kamil Konieczny]

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

Acked-by: Rob Herring <robh@kernel.org>
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.20.0

[PATCH v5 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.

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Stephen Boyd <sboyd@kernel.org>
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.20.0

[PATCH v5 4/5] dt-bindings: clk: exynos5433: add imem clock

[Kamil Konieczny]

Add Exynos5433 DT bindings to describe imem clocks for SlimSSS (Slim
Security SubSystem).

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

diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
index 87bb2b017143..52652aaabc06 100644
--- a/include/dt-bindings/clock/exynos5433.h
+++ b/include/dt-bindings/clock/exynos5433.h
@@ -1406,4 +1406,10 @@
 
 #define CAM1_NR_CLK					113
 
+/* CMU_IMEM */
+#define CLK_ACLK_SLIMSSS		2
+#define CLK_PCLK_SLIMSSS		35
+
+#define IMEM_NR_CLK			52
+
 #endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */
-- 
2.20.0

Re: [PATCH v5 4/5] dt-bindings: clk: exynos5433: add imem clock

[Rob Herring]

On Tue, Jan 22, 2019 at 10:02:31AM +0100, Kamil Konieczny wrote:
> Add Exynos5433 DT bindings to describe imem clocks for SlimSSS (Slim
> Security SubSystem).
> 
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
>  include/dt-bindings/clock/exynos5433.h | 6 ++++++
>  1 file changed, 6 insertions(+)

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

Re: [PATCH v5 4/5] dt-bindings: clk: exynos5433: add imem clock

[Chanwoo Choi]

Hi,

On 19. 1. 22. 오후 6:02, Kamil Konieczny wrote:
> Add Exynos5433 DT bindings to describe imem clocks for SlimSSS (Slim
> Security SubSystem).
> 
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
>  include/dt-bindings/clock/exynos5433.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
> index 87bb2b017143..52652aaabc06 100644
> --- a/include/dt-bindings/clock/exynos5433.h
> +++ b/include/dt-bindings/clock/exynos5433.h
> @@ -1406,4 +1406,10 @@
>  
>  #define CAM1_NR_CLK					113
>  
> +/* CMU_IMEM */
> +#define CLK_ACLK_SLIMSSS		2
> +#define CLK_PCLK_SLIMSSS		35
> +
> +#define IMEM_NR_CLK			52
> +
>  #endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */
> 

If you define that IMEM_NR_CLK is 52, clock driver allocates
the memory for 52 clocks. It cause the memory waste.
IMO, you better to change it as following:

#define CLK_ACLK_SLIMSSS		1
#define CLK_PCLK_SLIMSSS		2

#define IMEM_NR_CLK			3


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v5 4/5] dt-bindings: clk: exynos5433: add imem clock

[Sylwester Nawrocki]

On 1/31/19 13:30, Chanwoo Choi wrote:
>> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
>> index 87bb2b017143..52652aaabc06 100644
>> --- a/include/dt-bindings/clock/exynos5433.h
>> +++ b/include/dt-bindings/clock/exynos5433.h
>> @@ -1406,4 +1406,10 @@
>>  
>>  #define CAM1_NR_CLK					113
>>  
>> +/* CMU_IMEM */
>> +#define CLK_ACLK_SLIMSSS		2
>> +#define CLK_PCLK_SLIMSSS		35
>> +
>> +#define IMEM_NR_CLK			52
>> +
>>  #endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */
>>
> If you define that IMEM_NR_CLK is 52, clock driver allocates
> the memory for 52 clocks. It cause the memory waste.
> IMO, you better to change it as following:
> 
> #define CLK_ACLK_SLIMSSS		1
> #define CLK_PCLK_SLIMSSS		2
> 
> #define IMEM_NR_CLK			3

Let's keep the clock ID enumeration as is, so sorting is per clock
type as for all other CMUs in this header file. I.e. first all ACLK
then all PCLK clocks in case remaining clock ID get added in future. 
I'm going to adjust IMEM_NR_CLK to 36, as is not a part of DT ABI 
definition and for now it will allow us to save some memory. Although
I think the memory saving is negligible, we have already similar gaps 
in the clock ID enumeration.

I'm going to apply the patch with IMEM_NR_CLK changed 36, assuming
that Rob's Reviewed-by still stands, if not please let me know.

-- 
Thanks,
Sylwester

Re: [PATCH v5 4/5] dt-bindings: clk: exynos5433: add imem clock

[Chanwoo Choi]

[-- Attachment #1: Type: text/plain, Size: 1767 bytes --]

2019년 2월 1일 (금) 19:59에 Sylwester Nawrocki <s.nawrocki@samsung.com>님이 작성:

> On 1/31/19 13:30, Chanwoo Choi wrote:
> >> diff --git a/include/dt-bindings/clock/exynos5433.h
> b/include/dt-bindings/clock/exynos5433.h
> >> index 87bb2b017143..52652aaabc06 100644
> >> --- a/include/dt-bindings/clock/exynos5433.h
> >> +++ b/include/dt-bindings/clock/exynos5433.h
> >> @@ -1406,4 +1406,10 @@
> >>
> >>  #define CAM1_NR_CLK                                 113
> >>
> >> +/* CMU_IMEM */
> >> +#define CLK_ACLK_SLIMSSS            2
> >> +#define CLK_PCLK_SLIMSSS            35
> >> +
> >> +#define IMEM_NR_CLK                 52
> >> +
> >>  #endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */
> >>
> > If you define that IMEM_NR_CLK is 52, clock driver allocates
> > the memory for 52 clocks. It cause the memory waste.
> > IMO, you better to change it as following:
> >
> > #define CLK_ACLK_SLIMSSS              1
> > #define CLK_PCLK_SLIMSSS              2
> >
> > #define IMEM_NR_CLK                   3
>
> Let's keep the clock ID enumeration as is, so sorting is per clock
> type as for all other CMUs in this header file. I.e. first all ACLK
> then all PCLK clocks in case remaining clock ID get added in future.
> I'm going to adjust IMEM_NR_CLK to 36, as is not a part of DT ABI
> definition and for now it will allow us to save some memory. Although
> I think the memory saving is negligible, we have already similar gaps
> in the clock ID enumeration.
>
> I'm going to apply the patch with IMEM_NR_CLK changed 36, assuming
> that Rob's Reviewed-by still stands, if not please let me know.
>
> --
> Thanks,
> Sylwester
>

OK.

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

Regards,
Chanwoo Choi

>

[-- Attachment #2: Type: text/html, Size: 2681 bytes --]

[PATCH v5 5/5] clk: samsung: exynos5433: add imem clocks

[Kamil Konieczny]

Add imem clocks for exynos5433. This will enable to use crypto Slim
Security SubSystem (in short SlimSSS) IP block.

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

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index 24c3360db65b..dae1c96de933 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -5467,6 +5467,35 @@ static const struct samsung_cmu_info cam1_cmu_info __initconst = {
 	.clk_name		= "aclk_cam1_400",
 };
 
+/*
+ * Register offset definitions for CMU_IMEM
+ */
+#define ENABLE_ACLK_IMEM_SLIMSSS		0x080c
+#define ENABLE_PCLK_IMEM_SLIMSSS		0x0908
+
+static const unsigned long imem_clk_regs[] __initconst = {
+	ENABLE_ACLK_IMEM_SLIMSSS,
+	ENABLE_PCLK_IMEM_SLIMSSS,
+};
+
+static const struct samsung_gate_clock imem_gate_clks[] __initconst = {
+	/* ENABLE_ACLK_IMEM_SLIMSSS */
+	GATE(CLK_ACLK_SLIMSSS, "aclk_slimsss", "aclk_imem_sssx_266",
+			ENABLE_ACLK_IMEM_SLIMSSS, 0, CLK_IGNORE_UNUSED, 0),
+
+	/* ENABLE_PCLK_IMEM_SLIMSSS */
+	GATE(CLK_PCLK_SLIMSSS, "pclk_slimsss", "aclk_imem_200",
+			ENABLE_PCLK_IMEM_SLIMSSS, 0, CLK_IGNORE_UNUSED, 0),
+};
+
+static const struct samsung_cmu_info imem_cmu_info __initconst = {
+	.gate_clks		= imem_gate_clks,
+	.nr_gate_clks		= ARRAY_SIZE(imem_gate_clks),
+	.nr_clk_ids		= IMEM_NR_CLK,
+	.clk_regs		= imem_clk_regs,
+	.nr_clk_regs		= ARRAY_SIZE(imem_clk_regs),
+	.clk_name		= "aclk_imem_200",
+};
 
 struct exynos5433_cmu_data {
 	struct samsung_clk_reg_dump *clk_save;
@@ -5654,6 +5683,9 @@ static const struct of_device_id exynos5433_cmu_of_match[] = {
 	}, {
 		.compatible = "samsung,exynos5433-cmu-mscl",
 		.data = &mscl_cmu_info,
+	}, {
+		.compatible = "samsung,exynos5433-cmu-imem",
+		.data = &imem_cmu_info,
 	}, {
 	},
 };
-- 
2.20.0

Re: [PATCH v5 0/5] Add imem clock for Exynos 5433

[Sylwester Nawrocki]

Hi,

I applied whole series with minor commit message edits and change as
below squashed to patch 4/5.

diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
index 52652aaabc06..25ffa53573a5 100644
--- a/include/dt-bindings/clock/exynos5433.h
+++ b/include/dt-bindings/clock/exynos5433.h
@@ -1410,6 +1410,6 @@
 #define CLK_ACLK_SLIMSSS               2
 #define CLK_PCLK_SLIMSSS               35
 
-#define IMEM_NR_CLK                    52
+#define IMEM_NR_CLK                    36
 
 #endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */


-- 
Thanks,
Sylwester