* [PATCH v4 0/5] Add imem clock for Exynos 5433
[not found] <CGME20190118131710eucas1p1b4d7f67c5aa2832b669b1f3cff0235aa@eucas1p1.samsung.com>
@ 2019-01-18 13:16 ` Kamil Konieczny
[not found] ` <CGME20190118131711eucas1p190e9b63ac047566ba84fb4f8749aceee@eucas1p1.samsung.com>
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Kamil Konieczny @ 2019-01-18 13:16 UTC (permalink / raw)
To: k.konieczny, linux-samsung-soc
Cc: linux-clk, Chanwoo Choi, devicetree, Krzysztof Kozlowski,
Kukjin Kim, Mark Rutland, Rob Herring, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Marek Szyprowski
Add imem clock for Exynos 5433. This will allow to use SlimSSS (Slim
Security SubSystem) with crypto functions.
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 | 57 ++++++++++++++++++-
3 files changed, 114 insertions(+), 4 deletions(-)
--
2.20.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/5] clk: samsung: exynos5433: fix typo in imem divider
[not found] ` <CGME20190118131711eucas1p190e9b63ac047566ba84fb4f8749aceee@eucas1p1.samsung.com>
@ 2019-01-18 13:16 ` Kamil Konieczny
0 siblings, 0 replies; 10+ messages in thread
From: Kamil Konieczny @ 2019-01-18 13:16 UTC (permalink / raw)
To: k.konieczny, linux-samsung-soc
Cc: linux-clk, Chanwoo Choi, devicetree, Krzysztof Kozlowski,
Kukjin Kim, Mark Rutland, Rob Herring, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Marek Szyprowski, stable
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/5] clk: samsung: exynos5433: fix name typo in sssx
[not found] ` <CGME20190118131712eucas1p2b7e11ff6df0a78d8e7ea50b34f9cf737@eucas1p2.samsung.com>
@ 2019-01-18 13:16 ` Kamil Konieczny
0 siblings, 0 replies; 10+ messages in thread
From: Kamil Konieczny @ 2019-01-18 13:16 UTC (permalink / raw)
To: k.konieczny, linux-samsung-soc
Cc: linux-clk, Chanwoo Choi, devicetree, Krzysztof Kozlowski,
Kukjin Kim, Mark Rutland, Rob Herring, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Marek Szyprowski, Rob Herring
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 3/5] dt-bindings: clk: exynos5433: document imem clock
[not found] ` <CGME20190118131712eucas1p2cf278b13bb4b51c2331a7e2c9d864e9d@eucas1p2.samsung.com>
@ 2019-01-18 13:16 ` Kamil Konieczny
0 siblings, 0 replies; 10+ messages in thread
From: Kamil Konieczny @ 2019-01-18 13:16 UTC (permalink / raw)
To: k.konieczny, linux-samsung-soc
Cc: linux-clk, Chanwoo Choi, devicetree, Krzysztof Kozlowski,
Kukjin Kim, Mark Rutland, Rob Herring, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Marek Szyprowski, Rob Herring,
Stephen Boyd
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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 4/5] dt-bindings: clk: exynos5433: add imem clock
[not found] ` <CGME20190118131713eucas1p21e6deb9c80627cbf2e09c5b578cd410a@eucas1p2.samsung.com>
@ 2019-01-18 13:16 ` Kamil Konieczny
2019-01-19 12:40 ` Chanwoo Choi
2019-01-21 14:52 ` Rob Herring
0 siblings, 2 replies; 10+ messages in thread
From: Kamil Konieczny @ 2019-01-18 13:16 UTC (permalink / raw)
To: k.konieczny, linux-samsung-soc
Cc: linux-clk, Chanwoo Choi, devicetree, Krzysztof Kozlowski,
Kukjin Kim, Mark Rutland, Rob Herring, Sylwester Nawrocki,
Bartlomiej Zolnierkiewicz, Marek Szyprowski
Add DT bindings to describe imem clocks for SSS (Security SubSystem) and
SlimSSS in Exynos5433.
Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
---
include/dt-bindings/clock/exynos5433.h | 55 ++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
index 87bb2b017143..cc6153a0b5f7 100644
--- a/include/dt-bindings/clock/exynos5433.h
+++ b/include/dt-bindings/clock/exynos5433.h
@@ -1406,4 +1406,59 @@
#define CAM1_NR_CLK 113
+/* CMU_IMEM */
+#define CLK_ACLK_SSS 1
+#define CLK_ACLK_SLIMSSS 2
+#define CLK_ACLK_RTIC 3
+#define CLK_ACLK_XIU_SSSX 4
+#define CLK_ACLK_ASYNCAHBM_SSS_ATLAS 5
+#define CLK_ACLK_ASYNCAXI_SSSX 6
+#define CLK_ACLK_BTS_SSS_CCI 7
+#define CLK_ACLK_BTS_SSS_DRAM 8
+#define CLK_ACLK_BTS_SLIMSSS 9
+#define CLK_ACLK_SMMU_SSS_CCI 10
+#define CLK_ACLK_SMMU_SSS_DRAM 11
+#define CLK_ACLK_SMMU_SLIMSSS 12
+#define CLK_ACLK_SMMU_RTIC 13
+#define CLK_ACLK_IMEMND_266 14
+#define CLK_ACLK_ALB_IMEM 15
+#define CLK_ACLK_XIU_IMEMX 16
+#define CLK_ACLK_AXIUS_IMEMX 17
+#define CLK_ACLK_ASYNCAXI_IMEMX 18
+#define CLK_ACLK_ARBG_TX 19
+#define CLK_ACLK_BTS_ARBG_TX 20
+#define CLK_ACLK_SMMU_ARBG_TX 21
+#define CLK_ACLK_GIC 22
+#define CLK_ACLK_INT_MEM 23
+#define CLK_ACLK_XIU_PIMEMX 24
+#define CLK_ACLK_AXI2APB_IMEM0P 25
+#define CLK_ACLK_AXI2APB_IMEM1P 26
+#define CLK_ACLK_ASYNCAXIS_MIF_PIMEMX 27
+#define CLK_ACLK_AXIDS_PIMEMX_GIC 28
+#define CLK_ACLK_AXIDS_PIMEMX_IMEM0P 29
+#define CLK_ACLK_AXIDS_PIMEMX_IMEM1P 30
+#define CLK_ACLK_SROMC 31
+#define CLK_ACLK_AXIDS_SROMC 32
+#define CLK_ACLK_AXI2AHB_IMEMH 33
+#define CLK_PCLK_SSS 34
+#define CLK_PCLK_SLIMSSS 35
+#define CLK_PCLK_RTIC 36
+#define CLK_PCLK_SYSREG_IMEM 37
+#define CLK_PCLK_PMU_IMEM 38
+#define CLK_PCLK_ALB_IMEM 39
+#define CLK_PCLK_BTS_SSS_CCI 40
+#define CLK_PCLK_BTS_SSS_DRAM 41
+#define CLK_PCLK_BTS_SLIMSSS 42
+#define CLK_PCLK_SMMU_SSS_CCI 43
+#define CLK_PCLK_SMMU_SSS_DRAM 44
+#define CLK_PCLK_SMMU_SLIMSSS 45
+#define CLK_PCLK_SMMU_RTIC 46
+#define CLK_PCLK_ASYNCAPB_ARBG_TX 47
+#define CLK_PCLK_BTS_ARBG_TX 48
+#define CLK_PCLK_SMMU_ARBG_TX 49
+#define CLK_PCLK_ASYNCAXI_IMEMX 50
+#define CLK_PCLK_GPIO_IMEM 51
+
+#define IMEM_NR_CLK 52
+
#endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */
--
2.20.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/5] dt-bindings: clk: exynos5433: add imem clock
2019-01-18 13:16 ` [PATCH v4 4/5] dt-bindings: clk: exynos5433: add " Kamil Konieczny
@ 2019-01-19 12:40 ` Chanwoo Choi
2019-01-21 14:51 ` Rob Herring
2019-01-21 14:52 ` Rob Herring
1 sibling, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2019-01-19 12:40 UTC (permalink / raw)
To: Kamil Konieczny
Cc: linux-samsung-soc, linux-clk, Chanwoo Choi, devicetree,
Krzysztof Kozlowski, Kukjin Kim, Mark Rutland, Rob Herring,
Sylwester Nawrocki, Bartlomiej Zolnierkiewicz, Marek Szyprowski
Hi Kamil,
This version only adds following two clocks without that other clocks
be included in imem block.
- CLK_ACLK_SLIMSSS
- CLK_PCLK_SLIMSSS
Usually, the users only refer to
'include/dt-bindings/clock/exynos5433.h' in order to get the supported
clocks. It means that if there are different between the clock list in
'include/dt-bindings/clock/exynos5433.h' and the supported clock list
of drivers/clock/', it make the confusion for users. So,
'include/dt-bindings/clock/exynos5433.h' have to define the only
supported clock ids.
Please remove other clock ids except for CLK_ACLK_SLIMSSS/CLK_PCLK_SLIMSSS.
Best Regards,
Chanwoo Choi
Samsung Electronics
2019년 1월 18일 (금) 오후 10:17, Kamil Konieczny
<k.konieczny@partner.samsung.com>님이 작성:
>
> Add DT bindings to describe imem clocks for SSS (Security SubSystem) and
> SlimSSS in Exynos5433.
>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> include/dt-bindings/clock/exynos5433.h | 55 ++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/include/dt-bindings/clock/exynos5433.h b/include/dt-bindings/clock/exynos5433.h
> index 87bb2b017143..cc6153a0b5f7 100644
> --- a/include/dt-bindings/clock/exynos5433.h
> +++ b/include/dt-bindings/clock/exynos5433.h
> @@ -1406,4 +1406,59 @@
>
> #define CAM1_NR_CLK 113
>
> +/* CMU_IMEM */
> +#define CLK_ACLK_SSS 1
> +#define CLK_ACLK_SLIMSSS 2
> +#define CLK_ACLK_RTIC 3
> +#define CLK_ACLK_XIU_SSSX 4
> +#define CLK_ACLK_ASYNCAHBM_SSS_ATLAS 5
> +#define CLK_ACLK_ASYNCAXI_SSSX 6
> +#define CLK_ACLK_BTS_SSS_CCI 7
> +#define CLK_ACLK_BTS_SSS_DRAM 8
> +#define CLK_ACLK_BTS_SLIMSSS 9
> +#define CLK_ACLK_SMMU_SSS_CCI 10
> +#define CLK_ACLK_SMMU_SSS_DRAM 11
> +#define CLK_ACLK_SMMU_SLIMSSS 12
> +#define CLK_ACLK_SMMU_RTIC 13
> +#define CLK_ACLK_IMEMND_266 14
> +#define CLK_ACLK_ALB_IMEM 15
> +#define CLK_ACLK_XIU_IMEMX 16
> +#define CLK_ACLK_AXIUS_IMEMX 17
> +#define CLK_ACLK_ASYNCAXI_IMEMX 18
> +#define CLK_ACLK_ARBG_TX 19
> +#define CLK_ACLK_BTS_ARBG_TX 20
> +#define CLK_ACLK_SMMU_ARBG_TX 21
> +#define CLK_ACLK_GIC 22
> +#define CLK_ACLK_INT_MEM 23
> +#define CLK_ACLK_XIU_PIMEMX 24
> +#define CLK_ACLK_AXI2APB_IMEM0P 25
> +#define CLK_ACLK_AXI2APB_IMEM1P 26
> +#define CLK_ACLK_ASYNCAXIS_MIF_PIMEMX 27
> +#define CLK_ACLK_AXIDS_PIMEMX_GIC 28
> +#define CLK_ACLK_AXIDS_PIMEMX_IMEM0P 29
> +#define CLK_ACLK_AXIDS_PIMEMX_IMEM1P 30
> +#define CLK_ACLK_SROMC 31
> +#define CLK_ACLK_AXIDS_SROMC 32
> +#define CLK_ACLK_AXI2AHB_IMEMH 33
> +#define CLK_PCLK_SSS 34
> +#define CLK_PCLK_SLIMSSS 35
> +#define CLK_PCLK_RTIC 36
> +#define CLK_PCLK_SYSREG_IMEM 37
> +#define CLK_PCLK_PMU_IMEM 38
> +#define CLK_PCLK_ALB_IMEM 39
> +#define CLK_PCLK_BTS_SSS_CCI 40
> +#define CLK_PCLK_BTS_SSS_DRAM 41
> +#define CLK_PCLK_BTS_SLIMSSS 42
> +#define CLK_PCLK_SMMU_SSS_CCI 43
> +#define CLK_PCLK_SMMU_SSS_DRAM 44
> +#define CLK_PCLK_SMMU_SLIMSSS 45
> +#define CLK_PCLK_SMMU_RTIC 46
> +#define CLK_PCLK_ASYNCAPB_ARBG_TX 47
> +#define CLK_PCLK_BTS_ARBG_TX 48
> +#define CLK_PCLK_SMMU_ARBG_TX 49
> +#define CLK_PCLK_ASYNCAXI_IMEMX 50
> +#define CLK_PCLK_GPIO_IMEM 51
> +
> +#define IMEM_NR_CLK 52
> +
> #endif /* _DT_BINDINGS_CLOCK_EXYNOS5433_H */
> --
> 2.20.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/5] dt-bindings: clk: exynos5433: add imem clock
2019-01-19 12:40 ` Chanwoo Choi
@ 2019-01-21 14:51 ` Rob Herring
2019-01-22 0:27 ` Chanwoo Choi
0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-01-21 14:51 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Kamil Konieczny, linux-samsung-soc, linux-clk, Chanwoo Choi,
devicetree, Krzysztof Kozlowski, Kukjin Kim, Mark Rutland,
Sylwester Nawrocki, Bartlomiej Zolnierkiewicz, Marek Szyprowski
On Sat, Jan 19, 2019 at 09:40:10PM +0900, Chanwoo Choi wrote:
> Hi Kamil,
>
> This version only adds following two clocks without that other clocks
> be included in imem block.
> - CLK_ACLK_SLIMSSS
> - CLK_PCLK_SLIMSSS
>
> Usually, the users only refer to
> 'include/dt-bindings/clock/exynos5433.h' in order to get the supported
> clocks. It means that if there are different between the clock list in
> 'include/dt-bindings/clock/exynos5433.h' and the supported clock list
> of drivers/clock/', it make the confusion for users. So,
> 'include/dt-bindings/clock/exynos5433.h' have to define the only
> supported clock ids.
>
> Please remove other clock ids except for CLK_ACLK_SLIMSSS/CLK_PCLK_SLIMSSS.
And add clocks 1-by-1 instead? The binding headers are a pain to merge
because they are a dts and driver dependency. Having a complete header
is preferred unless there is doubt in the correctness of it.
The driver can warn users about unsupported IDs.
Rob
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/5] dt-bindings: clk: exynos5433: add imem clock
2019-01-18 13:16 ` [PATCH v4 4/5] dt-bindings: clk: exynos5433: add " Kamil Konieczny
2019-01-19 12:40 ` Chanwoo Choi
@ 2019-01-21 14:52 ` Rob Herring
1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2019-01-21 14:52 UTC (permalink / raw)
To: Kamil Konieczny
Cc: k.konieczny, linux-samsung-soc, linux-clk, Chanwoo Choi,
devicetree, Krzysztof Kozlowski, Kukjin Kim, Mark Rutland,
Rob Herring, Sylwester Nawrocki, Bartlomiej Zolnierkiewicz,
Marek Szyprowski
On Fri, 18 Jan 2019 14:16:38 +0100, Kamil Konieczny wrote:
> Add DT bindings to describe imem clocks for SSS (Security SubSystem) and
> SlimSSS in Exynos5433.
>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> include/dt-bindings/clock/exynos5433.h | 55 ++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/5] dt-bindings: clk: exynos5433: add imem clock
2019-01-21 14:51 ` Rob Herring
@ 2019-01-22 0:27 ` Chanwoo Choi
2019-01-22 9:41 ` Sylwester Nawrocki
0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2019-01-22 0:27 UTC (permalink / raw)
To: Rob Herring, Chanwoo Choi
Cc: Kamil Konieczny, linux-samsung-soc, linux-clk, devicetree,
Krzysztof Kozlowski, Kukjin Kim, Mark Rutland,
Sylwester Nawrocki, Bartlomiej Zolnierkiewicz, Marek Szyprowski
On 19. 1. 21. 오후 11:51, Rob Herring wrote:
> On Sat, Jan 19, 2019 at 09:40:10PM +0900, Chanwoo Choi wrote:
>> Hi Kamil,
>>
>> This version only adds following two clocks without that other clocks
>> be included in imem block.
>> - CLK_ACLK_SLIMSSS
>> - CLK_PCLK_SLIMSSS
>>
>> Usually, the users only refer to
>> 'include/dt-bindings/clock/exynos5433.h' in order to get the supported
>> clocks. It means that if there are different between the clock list in
>> 'include/dt-bindings/clock/exynos5433.h' and the supported clock list
>> of drivers/clock/', it make the confusion for users. So,
>> 'include/dt-bindings/clock/exynos5433.h' have to define the only
>> supported clock ids.
>>
>> Please remove other clock ids except for CLK_ACLK_SLIMSSS/CLK_PCLK_SLIMSSS.
>
> And add clocks 1-by-1 instead? The binding headers are a pain to merge
> because they are a dts and driver dependency. Having a complete header
> is preferred unless there is doubt in the correctness of it.
As you commented, I knew the dependency between dt-binding head file and
drivers/dts. But, IMO, I'm not sure that it is right to define
the unsupported ID in dt-bindings header file because we don't know
the correctness without any real test. The author explained why dropped
the clocks except for two clocks on cover-letter. It means that the clocks
except for two clocks might be never added to clock driver because He was
unable to test them..
>
> The driver can warn users about unsupported IDs.
>
> Rob
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/5] dt-bindings: clk: exynos5433: add imem clock
2019-01-22 0:27 ` Chanwoo Choi
@ 2019-01-22 9:41 ` Sylwester Nawrocki
0 siblings, 0 replies; 10+ messages in thread
From: Sylwester Nawrocki @ 2019-01-22 9:41 UTC (permalink / raw)
To: Chanwoo Choi, Rob Herring, Chanwoo Choi
Cc: Kamil Konieczny, linux-samsung-soc, linux-clk, devicetree,
Krzysztof Kozlowski, Kukjin Kim, Mark Rutland,
Bartlomiej Zolnierkiewicz, Marek Szyprowski, Stephen Boyd
On 1/22/19 01:27, Chanwoo Choi wrote:
> On 19. 1. 21. 오후 11:51, Rob Herring wrote:
>> On Sat, Jan 19, 2019 at 09:40:10PM +0900, Chanwoo Choi wrote:
>>> This version only adds following two clocks without that other clocks
>>> be included in imem block.
>>> - CLK_ACLK_SLIMSSS
>>> - CLK_PCLK_SLIMSSS
>>>
>>> Usually, the users only refer to
>>> 'include/dt-bindings/clock/exynos5433.h' in order to get the supported
>>> clocks. It means that if there are different between the clock list in
>>> 'include/dt-bindings/clock/exynos5433.h' and the supported clock list
>>> of drivers/clock/', it make the confusion for users. So,
>>> 'include/dt-bindings/clock/exynos5433.h' have to define the only
>>> supported clock ids.
>>>
>>> Please remove other clock ids except for CLK_ACLK_SLIMSSS/CLK_PCLK_SLIMSSS.
>>
>> And add clocks 1-by-1 instead? The binding headers are a pain to merge
>> because they are a dts and driver dependency. Having a complete header
>> is preferred unless there is doubt in the correctness of it.
Exactly, this is also what we agreed with Stephen, to make a complete header
with all IDs. I believe the list of clocks is correct, this clock unit is
pretty straightforward, it's just a collection of gate clocks.
But chances are that the other clocks will never get added, as those seem
to be normally handled outside of Linux kernel. It still might not be a good
argument against having complete header describing the hardware completely,
independently of an OS, the bootloader might use those IDs, etc.
In practise it will be adding likely never used code though.
> As you commented, I knew the dependency between dt-binding head file and
> drivers/dts. But, IMO, I'm not sure that it is right to define
> the unsupported ID in dt-bindings header file because we don't know
> the correctness without any real test. The author explained why dropped
> the clocks except for two clocks on cover-letter. It means that the clocks
> except for two clocks might be never added to clock driver because He was
> unable to test them..
I agree, I doubt the remaining clocks will ever get added in case of this SoC.
>> The driver can warn users about unsupported IDs
--
Regards,
Sylwester
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-01-22 9:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20190118131710eucas1p1b4d7f67c5aa2832b669b1f3cff0235aa@eucas1p1.samsung.com>
2019-01-18 13:16 ` [PATCH v4 0/5] Add imem clock for Exynos 5433 Kamil Konieczny
[not found] ` <CGME20190118131711eucas1p190e9b63ac047566ba84fb4f8749aceee@eucas1p1.samsung.com>
2019-01-18 13:16 ` [PATCH v4 1/5] clk: samsung: exynos5433: fix typo in imem divider Kamil Konieczny
[not found] ` <CGME20190118131712eucas1p2b7e11ff6df0a78d8e7ea50b34f9cf737@eucas1p2.samsung.com>
2019-01-18 13:16 ` [PATCH v4 2/5] clk: samsung: exynos5433: fix name typo in sssx Kamil Konieczny
[not found] ` <CGME20190118131712eucas1p2cf278b13bb4b51c2331a7e2c9d864e9d@eucas1p2.samsung.com>
2019-01-18 13:16 ` [PATCH v4 3/5] dt-bindings: clk: exynos5433: document imem clock Kamil Konieczny
[not found] ` <CGME20190118131713eucas1p21e6deb9c80627cbf2e09c5b578cd410a@eucas1p2.samsung.com>
2019-01-18 13:16 ` [PATCH v4 4/5] dt-bindings: clk: exynos5433: add " Kamil Konieczny
2019-01-19 12:40 ` Chanwoo Choi
2019-01-21 14:51 ` Rob Herring
2019-01-22 0:27 ` Chanwoo Choi
2019-01-22 9:41 ` Sylwester Nawrocki
2019-01-21 14:52 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).