diff mbox series

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

Message ID 20190122090232.29194-5-k.konieczny@partner.samsung.com (mailing list archive)
State Not Applicable
Headers show
Series [v5,1/5] clk: samsung: exynos5433: fix typo in imem divider | expand

Commit Message

Kamil Konieczny Jan. 22, 2019, 9:02 a.m. UTC
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(+)

Comments

Rob Herring (Arm) Jan. 30, 2019, 3:46 p.m. UTC | #1
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>
Chanwoo Choi Jan. 31, 2019, 12:30 p.m. UTC | #2
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
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.
diff mbox series

Patch

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 */