[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

Add proper gate clock for the Analog to Digital Converter (ADC) on
Exynos4x12.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/clk/samsung/clk-exynos4.c   | 3 +++
 include/dt-bindings/clock/exynos4.h | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 714d6ba782c8..5f32410a01f8 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -85,6 +85,7 @@
 #define DIV_PERIL4		0xc560
 #define DIV_PERIL5		0xc564
 #define E4X12_DIV_CAM1		0xc568
+#define E4X12_GATE_BUS_FSYS1	0xc744
 #define GATE_SCLK_CAM		0xc820
 #define GATE_IP_CAM		0xc920
 #define GATE_IP_TV		0xc924
@@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
 		0),
 	GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
 		0),
+	GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
+		0),
 	GATE(CLK_MIPI_HSI, "mipi_hsi", "aclk133", GATE_IP_FSYS, 10, 0, 0),
 	GATE(CLK_CHIPID, "chipid", "aclk100", E4X12_GATE_IP_PERIR, 0, 0, 0),
 	GATE(CLK_SYSREG, "sysreg", "aclk100", E4X12_GATE_IP_PERIR, 1,
diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
index c4b1676ea674..4548531736c1 100644
--- a/include/dt-bindings/clock/exynos4.h
+++ b/include/dt-bindings/clock/exynos4.h
@@ -268,7 +268,10 @@
 #define CLK_DIV_GDL		459
 #define CLK_DIV_GDR		460
 
+/* Exynos4x12 only */
+#define CLK_PCLK_ADC		461
+
 /* must be greater than maximal clock id */
-#define CLK_NR_CLKS		461
+#define CLK_NR_CLKS		462
 
 #endif /* _DT_BINDINGS_CLOCK_EXYNOS_4_H */
-- 
1.9.1

[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

Add proper gate clock for the Analog to Digital Converter (ADC) on
Exynos4x12.

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 drivers/clk/samsung/clk-exynos4.c   | 3 +++
 include/dt-bindings/clock/exynos4.h | 5 ++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 714d6ba782c8..5f32410a01f8 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -85,6 +85,7 @@
 #define DIV_PERIL4		0xc560
 #define DIV_PERIL5		0xc564
 #define E4X12_DIV_CAM1		0xc568
+#define E4X12_GATE_BUS_FSYS1	0xc744
 #define GATE_SCLK_CAM		0xc820
 #define GATE_IP_CAM		0xc920
 #define GATE_IP_TV		0xc924
@@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
 		0),
 	GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
 		0),
+	GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
+		0),
 	GATE(CLK_MIPI_HSI, "mipi_hsi", "aclk133", GATE_IP_FSYS, 10, 0, 0),
 	GATE(CLK_CHIPID, "chipid", "aclk100", E4X12_GATE_IP_PERIR, 0, 0, 0),
 	GATE(CLK_SYSREG, "sysreg", "aclk100", E4X12_GATE_IP_PERIR, 1,
diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
index c4b1676ea674..4548531736c1 100644
--- a/include/dt-bindings/clock/exynos4.h
+++ b/include/dt-bindings/clock/exynos4.h
@@ -268,7 +268,10 @@
 #define CLK_DIV_GDL		459
 #define CLK_DIV_GDR		460
 
+/* Exynos4x12 only */
+#define CLK_PCLK_ADC		461
+
 /* must be greater than maximal clock id */
-#define CLK_NR_CLKS		461
+#define CLK_NR_CLKS		462
 
 #endif /* _DT_BINDINGS_CLOCK_EXYNOS_4_H */
-- 
1.9.1

[PATCH 2/2] ARM: dts: Fix wrong clock for Exynos4x12 ADC

[Krzysztof Kozlowski]

The TSADC gate clock is present only in Exynos4210. It should not be
added to exynos4x12.dtsi because the register controlling it is reserved
on Exynos4x12.

Instead, the Analog to Digital Converter of Exynos4x12 uses PCLK_ADC
gate clock from different register.

By using proper clock this effectively enables usage of exynos-adc
driver on Exynos4412 boards, enables accessing sensors connected to it
on Trats2 board (ntc,ncp15wb473 AP and battery thermistors) and fixes
following warnings during boot:
[    2.248247] ERROR: could not get clock /adc@126C0000:adc(0)
[    2.248262] exynos-adc 126c0000.adc: failed getting clock, err = -2
[    2.248293] exynos-adc: probe of 126c0000.adc failed with error -2

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index b77dac61ffb5..d7738dd062b7 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -101,7 +101,7 @@
 		reg = <0x126C0000 0x100>;
 		interrupt-parent = <&combiner>;
 		interrupts = <10 3>;
-		clocks = <&clock CLK_TSADC>;
+		clocks = <&clock CLK_PCLK_ADC>;
 		clock-names = "adc";
 		#io-channel-cells = <1>;
 		io-channel-ranges;
-- 
1.9.1

[PATCH 2/2] ARM: dts: Fix wrong clock for Exynos4x12 ADC

[Krzysztof Kozlowski]

The TSADC gate clock is present only in Exynos4210. It should not be
added to exynos4x12.dtsi because the register controlling it is reserved
on Exynos4x12.

Instead, the Analog to Digital Converter of Exynos4x12 uses PCLK_ADC
gate clock from different register.

By using proper clock this effectively enables usage of exynos-adc
driver on Exynos4412 boards, enables accessing sensors connected to it
on Trats2 board (ntc,ncp15wb473 AP and battery thermistors) and fixes
following warnings during boot:
[    2.248247] ERROR: could not get clock /adc at 126C0000:adc(0)
[    2.248262] exynos-adc 126c0000.adc: failed getting clock, err = -2
[    2.248293] exynos-adc: probe of 126c0000.adc failed with error -2

Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
---
 arch/arm/boot/dts/exynos4x12.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi
index b77dac61ffb5..d7738dd062b7 100644
--- a/arch/arm/boot/dts/exynos4x12.dtsi
+++ b/arch/arm/boot/dts/exynos4x12.dtsi
@@ -101,7 +101,7 @@
 		reg = <0x126C0000 0x100>;
 		interrupt-parent = <&combiner>;
 		interrupts = <10 3>;
-		clocks = <&clock CLK_TSADC>;
+		clocks = <&clock CLK_PCLK_ADC>;
 		clock-names = "adc";
 		#io-channel-cells = <1>;
 		io-channel-ranges;
-- 
1.9.1

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
> Add proper gate clock for the Analog to Digital Converter (ADC) on
> Exynos4x12.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index 714d6ba782c8..5f32410a01f8 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -85,6 +85,7 @@
>  #define DIV_PERIL4		0xc560
>  #define DIV_PERIL5		0xc564
>  #define E4X12_DIV_CAM1		0xc568
> +#define E4X12_GATE_BUS_FSYS1	0xc744
>  #define GATE_SCLK_CAM		0xc820
>  #define GATE_IP_CAM		0xc920
>  #define GATE_IP_TV		0xc924
> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>  		0),
>  	GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>  		0),
> +	GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
> +		0),

Now I have even simpler idea. Don't add new clock id but just define
here the CLK_TSADC as:
GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);

With this change the second patch wouldn't be needed however this does
not reflect the Exynos 4x12 datasheet.

Any comments?

Best regards,
Krzysztof


>  	GATE(CLK_MIPI_HSI, "mipi_hsi", "aclk133", GATE_IP_FSYS, 10, 0, 0),
>  	GATE(CLK_CHIPID, "chipid", "aclk100", E4X12_GATE_IP_PERIR, 0, 0, 0),
>  	GATE(CLK_SYSREG, "sysreg", "aclk100", E4X12_GATE_IP_PERIR, 1,
> diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
> index c4b1676ea674..4548531736c1 100644
> --- a/include/dt-bindings/clock/exynos4.h
> +++ b/include/dt-bindings/clock/exynos4.h
> @@ -268,7 +268,10 @@
>  #define CLK_DIV_GDL		459
>  #define CLK_DIV_GDR		460
>  
> +/* Exynos4x12 only */
> +#define CLK_PCLK_ADC		461
> +
>  /* must be greater than maximal clock id */
> -#define CLK_NR_CLKS		461
> +#define CLK_NR_CLKS		462
>  
>  #endif /* _DT_BINDINGS_CLOCK_EXYNOS_4_H */
> 

[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
> Add proper gate clock for the Analog to Digital Converter (ADC) on
> Exynos4x12.
> 
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> ---
>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
> index 714d6ba782c8..5f32410a01f8 100644
> --- a/drivers/clk/samsung/clk-exynos4.c
> +++ b/drivers/clk/samsung/clk-exynos4.c
> @@ -85,6 +85,7 @@
>  #define DIV_PERIL4		0xc560
>  #define DIV_PERIL5		0xc564
>  #define E4X12_DIV_CAM1		0xc568
> +#define E4X12_GATE_BUS_FSYS1	0xc744
>  #define GATE_SCLK_CAM		0xc820
>  #define GATE_IP_CAM		0xc920
>  #define GATE_IP_TV		0xc924
> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>  		0),
>  	GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>  		0),
> +	GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
> +		0),

Now I have even simpler idea. Don't add new clock id but just define
here the CLK_TSADC as:
GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);

With this change the second patch wouldn't be needed however this does
not reflect the Exynos 4x12 datasheet.

Any comments?

Best regards,
Krzysztof


>  	GATE(CLK_MIPI_HSI, "mipi_hsi", "aclk133", GATE_IP_FSYS, 10, 0, 0),
>  	GATE(CLK_CHIPID, "chipid", "aclk100", E4X12_GATE_IP_PERIR, 0, 0, 0),
>  	GATE(CLK_SYSREG, "sysreg", "aclk100", E4X12_GATE_IP_PERIR, 1,
> diff --git a/include/dt-bindings/clock/exynos4.h b/include/dt-bindings/clock/exynos4.h
> index c4b1676ea674..4548531736c1 100644
> --- a/include/dt-bindings/clock/exynos4.h
> +++ b/include/dt-bindings/clock/exynos4.h
> @@ -268,7 +268,10 @@
>  #define CLK_DIV_GDL		459
>  #define CLK_DIV_GDR		460
>  
> +/* Exynos4x12 only */
> +#define CLK_PCLK_ADC		461
> +
>  /* must be greater than maximal clock id */
> -#define CLK_NR_CLKS		461
> +#define CLK_NR_CLKS		462
>  
>  #endif /* _DT_BINDINGS_CLOCK_EXYNOS_4_H */
> 

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Javier Martinez Canillas]

Hello Krzysztof,

On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>> Exynos4x12.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>> index 714d6ba782c8..5f32410a01f8 100644
>> --- a/drivers/clk/samsung/clk-exynos4.c
>> +++ b/drivers/clk/samsung/clk-exynos4.c
>> @@ -85,6 +85,7 @@
>>  #define DIV_PERIL4           0xc560
>>  #define DIV_PERIL5           0xc564
>>  #define E4X12_DIV_CAM1               0xc568
>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>  #define GATE_SCLK_CAM                0xc820
>>  #define GATE_IP_CAM          0xc920
>>  #define GATE_IP_TV           0xc924
>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>               0),
>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>               0),
>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>> +             0),
>
> Now I have even simpler idea. Don't add new clock id but just define
> here the CLK_TSADC as:
> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>
> With this change the second patch wouldn't be needed however this does
> not reflect the Exynos 4x12 datasheet.
>
> Any comments?
>

I think it's better to reflect the datasheet so I prefer your original
patch. Also, wouldn't changing the CLK_TSADC gate definition cause a
regression on an Exynos4210 board that is using the tsadc clock? or
maybe I misunderstood the explanation of your Patch 2/2?

> Best regards,
> Krzysztof
>

Best regards,
Javier

[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Javier Martinez Canillas]

Hello Krzysztof,

On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>> Exynos4x12.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>> index 714d6ba782c8..5f32410a01f8 100644
>> --- a/drivers/clk/samsung/clk-exynos4.c
>> +++ b/drivers/clk/samsung/clk-exynos4.c
>> @@ -85,6 +85,7 @@
>>  #define DIV_PERIL4           0xc560
>>  #define DIV_PERIL5           0xc564
>>  #define E4X12_DIV_CAM1               0xc568
>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>  #define GATE_SCLK_CAM                0xc820
>>  #define GATE_IP_CAM          0xc920
>>  #define GATE_IP_TV           0xc924
>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>               0),
>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>               0),
>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>> +             0),
>
> Now I have even simpler idea. Don't add new clock id but just define
> here the CLK_TSADC as:
> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>
> With this change the second patch wouldn't be needed however this does
> not reflect the Exynos 4x12 datasheet.
>
> Any comments?
>

I think it's better to reflect the datasheet so I prefer your original
patch. Also, wouldn't changing the CLK_TSADC gate definition cause a
regression on an Exynos4210 board that is using the tsadc clock? or
maybe I misunderstood the explanation of your Patch 2/2?

> Best regards,
> Krzysztof
>

Best regards,
Javier

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Javier Martinez Canillas]

Hello Krzysztof,

On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>> Exynos4x12.
>>
>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>> ---
>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>> index 714d6ba782c8..5f32410a01f8 100644
>> --- a/drivers/clk/samsung/clk-exynos4.c
>> +++ b/drivers/clk/samsung/clk-exynos4.c
>> @@ -85,6 +85,7 @@
>>  #define DIV_PERIL4           0xc560
>>  #define DIV_PERIL5           0xc564
>>  #define E4X12_DIV_CAM1               0xc568
>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>  #define GATE_SCLK_CAM                0xc820
>>  #define GATE_IP_CAM          0xc920
>>  #define GATE_IP_TV           0xc924
>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>               0),
>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>               0),
>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>> +             0),
>
> Now I have even simpler idea. Don't add new clock id but just define
> here the CLK_TSADC as:
> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>
> With this change the second patch wouldn't be needed however this does
> not reflect the Exynos 4x12 datasheet.
>
> Any comments?
>

I think it's better to reflect the datasheet so I prefer your original
patch. Also, wouldn't changing the CLK_TSADC gate definition cause a
regression on an Exynos4210 board that is using the tsadc clock? or
maybe I misunderstood the explanation of your Patch 2/2?

> Best regards,
> Krzysztof
>

Best regards,
Javier

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
> Hello Krzysztof,
> 
> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>> Exynos4x12.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> ---
>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>> index 714d6ba782c8..5f32410a01f8 100644
>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>> @@ -85,6 +85,7 @@
>>>  #define DIV_PERIL4           0xc560
>>>  #define DIV_PERIL5           0xc564
>>>  #define E4X12_DIV_CAM1               0xc568
>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>  #define GATE_SCLK_CAM                0xc820
>>>  #define GATE_IP_CAM          0xc920
>>>  #define GATE_IP_TV           0xc924
>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>               0),
>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>               0),
>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>> +             0),
>>
>> Now I have even simpler idea. Don't add new clock id but just define
>> here the CLK_TSADC as:
>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>
>> With this change the second patch wouldn't be needed however this does
>> not reflect the Exynos 4x12 datasheet.
>>
>> Any comments?
>>
> 
> I think it's better to reflect the datasheet so I prefer your original
> patch.

Yeah, I also like sticking to datasheet but maybe it is not always worth
to reproduce the datasheet in 100%. It is just thinking out loud.

> Also, wouldn't changing the CLK_TSADC gate definition cause a
> regression on an Exynos4210 board that is using the tsadc clock? or
> maybe I misunderstood the explanation of your Patch 2/2?

No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
register of PCLK_ADC. The id would stay the same as on Exynos4210.

Best regards,
Krzysztof

[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
> Hello Krzysztof,
> 
> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>> Exynos4x12.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> ---
>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>> index 714d6ba782c8..5f32410a01f8 100644
>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>> @@ -85,6 +85,7 @@
>>>  #define DIV_PERIL4           0xc560
>>>  #define DIV_PERIL5           0xc564
>>>  #define E4X12_DIV_CAM1               0xc568
>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>  #define GATE_SCLK_CAM                0xc820
>>>  #define GATE_IP_CAM          0xc920
>>>  #define GATE_IP_TV           0xc924
>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>               0),
>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>               0),
>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>> +             0),
>>
>> Now I have even simpler idea. Don't add new clock id but just define
>> here the CLK_TSADC as:
>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>
>> With this change the second patch wouldn't be needed however this does
>> not reflect the Exynos 4x12 datasheet.
>>
>> Any comments?
>>
> 
> I think it's better to reflect the datasheet so I prefer your original
> patch.

Yeah, I also like sticking to datasheet but maybe it is not always worth
to reproduce the datasheet in 100%. It is just thinking out loud.

> Also, wouldn't changing the CLK_TSADC gate definition cause a
> regression on an Exynos4210 board that is using the tsadc clock? or
> maybe I misunderstood the explanation of your Patch 2/2?

No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
register of PCLK_ADC. The id would stay the same as on Exynos4210.

Best regards,
Krzysztof

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
> Hello Krzysztof,
> 
> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
> <k.kozlowski@samsung.com> wrote:
>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>> Exynos4x12.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> ---
>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>> index 714d6ba782c8..5f32410a01f8 100644
>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>> @@ -85,6 +85,7 @@
>>>  #define DIV_PERIL4           0xc560
>>>  #define DIV_PERIL5           0xc564
>>>  #define E4X12_DIV_CAM1               0xc568
>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>  #define GATE_SCLK_CAM                0xc820
>>>  #define GATE_IP_CAM          0xc920
>>>  #define GATE_IP_TV           0xc924
>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>               0),
>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>               0),
>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>> +             0),
>>
>> Now I have even simpler idea. Don't add new clock id but just define
>> here the CLK_TSADC as:
>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>
>> With this change the second patch wouldn't be needed however this does
>> not reflect the Exynos 4x12 datasheet.
>>
>> Any comments?
>>
> 
> I think it's better to reflect the datasheet so I prefer your original
> patch.

Yeah, I also like sticking to datasheet but maybe it is not always worth
to reproduce the datasheet in 100%. It is just thinking out loud.

> Also, wouldn't changing the CLK_TSADC gate definition cause a
> regression on an Exynos4210 board that is using the tsadc clock? or
> maybe I misunderstood the explanation of your Patch 2/2?

No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
register of PCLK_ADC. The id would stay the same as on Exynos4210.

Best regards,
Krzysztof

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Krzysztof Kozlowski]

W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
> Hello Krzysztof,
> 
> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
> <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>> Exynos4x12.
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>> ---
>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>> index 714d6ba782c8..5f32410a01f8 100644
>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>> @@ -85,6 +85,7 @@
>>>  #define DIV_PERIL4           0xc560
>>>  #define DIV_PERIL5           0xc564
>>>  #define E4X12_DIV_CAM1               0xc568
>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>  #define GATE_SCLK_CAM                0xc820
>>>  #define GATE_IP_CAM          0xc920
>>>  #define GATE_IP_TV           0xc924
>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>               0),
>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>               0),
>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>> +             0),
>>
>> Now I have even simpler idea. Don't add new clock id but just define
>> here the CLK_TSADC as:
>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>
>> With this change the second patch wouldn't be needed however this does
>> not reflect the Exynos 4x12 datasheet.
>>
>> Any comments?
>>
> 
> I think it's better to reflect the datasheet so I prefer your original
> patch.

Yeah, I also like sticking to datasheet but maybe it is not always worth
to reproduce the datasheet in 100%. It is just thinking out loud.

> Also, wouldn't changing the CLK_TSADC gate definition cause a
> regression on an Exynos4210 board that is using the tsadc clock? or
> maybe I misunderstood the explanation of your Patch 2/2?

No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
register of PCLK_ADC. The id would stay the same as on Exynos4210.

Best regards,
Krzysztof


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Javier Martinez Canillas]

Hello Krzysztof,

On Thu, Jun 11, 2015 at 2:40 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
>> Hello Krzysztof,
>>
>> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>>> Exynos4x12.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> ---
>>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>>> index 714d6ba782c8..5f32410a01f8 100644
>>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>>> @@ -85,6 +85,7 @@
>>>>  #define DIV_PERIL4           0xc560
>>>>  #define DIV_PERIL5           0xc564
>>>>  #define E4X12_DIV_CAM1               0xc568
>>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>>  #define GATE_SCLK_CAM                0xc820
>>>>  #define GATE_IP_CAM          0xc920
>>>>  #define GATE_IP_TV           0xc924
>>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>>               0),
>>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>>               0),
>>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>>> +             0),
>>>
>>> Now I have even simpler idea. Don't add new clock id but just define
>>> here the CLK_TSADC as:
>>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>>
>>> With this change the second patch wouldn't be needed however this does
>>> not reflect the Exynos 4x12 datasheet.
>>>
>>> Any comments?
>>>
>>
>> I think it's better to reflect the datasheet so I prefer your original
>> patch.
>
> Yeah, I also like sticking to datasheet but maybe it is not always worth
> to reproduce the datasheet in 100%. It is just thinking out loud.
>

Agreed.

>> Also, wouldn't changing the CLK_TSADC gate definition cause a
>> regression on an Exynos4210 board that is using the tsadc clock? or
>> maybe I misunderstood the explanation of your Patch 2/2?
>
> No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
> in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
> 1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
> 2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
> register of PCLK_ADC. The id would stay the same as on Exynos4210.
>

I see, thanks for the explanation. Both approaches seems sensible for
me then, I don't have a strong opinion on either though.

> Best regards,
> Krzysztof
>

Best regards,
Javier

[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Javier Martinez Canillas]

Hello Krzysztof,

On Thu, Jun 11, 2015 at 2:40 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
>> Hello Krzysztof,
>>
>> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>>> Exynos4x12.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> ---
>>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>>> index 714d6ba782c8..5f32410a01f8 100644
>>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>>> @@ -85,6 +85,7 @@
>>>>  #define DIV_PERIL4           0xc560
>>>>  #define DIV_PERIL5           0xc564
>>>>  #define E4X12_DIV_CAM1               0xc568
>>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>>  #define GATE_SCLK_CAM                0xc820
>>>>  #define GATE_IP_CAM          0xc920
>>>>  #define GATE_IP_TV           0xc924
>>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>>               0),
>>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>>               0),
>>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>>> +             0),
>>>
>>> Now I have even simpler idea. Don't add new clock id but just define
>>> here the CLK_TSADC as:
>>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>>
>>> With this change the second patch wouldn't be needed however this does
>>> not reflect the Exynos 4x12 datasheet.
>>>
>>> Any comments?
>>>
>>
>> I think it's better to reflect the datasheet so I prefer your original
>> patch.
>
> Yeah, I also like sticking to datasheet but maybe it is not always worth
> to reproduce the datasheet in 100%. It is just thinking out loud.
>

Agreed.

>> Also, wouldn't changing the CLK_TSADC gate definition cause a
>> regression on an Exynos4210 board that is using the tsadc clock? or
>> maybe I misunderstood the explanation of your Patch 2/2?
>
> No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
> in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
> 1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
> 2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
> register of PCLK_ADC. The id would stay the same as on Exynos4210.
>

I see, thanks for the explanation. Both approaches seems sensible for
me then, I don't have a strong opinion on either though.

> Best regards,
> Krzysztof
>

Best regards,
Javier

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Javier Martinez Canillas]

Hello Krzysztof,

On Thu, Jun 11, 2015 at 2:40 PM, Krzysztof Kozlowski
<k.kozlowski@samsung.com> wrote:
> W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
>> Hello Krzysztof,
>>
>> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>>> Exynos4x12.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> ---
>>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>>> index 714d6ba782c8..5f32410a01f8 100644
>>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>>> @@ -85,6 +85,7 @@
>>>>  #define DIV_PERIL4           0xc560
>>>>  #define DIV_PERIL5           0xc564
>>>>  #define E4X12_DIV_CAM1               0xc568
>>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>>  #define GATE_SCLK_CAM                0xc820
>>>>  #define GATE_IP_CAM          0xc920
>>>>  #define GATE_IP_TV           0xc924
>>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>>               0),
>>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>>               0),
>>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>>> +             0),
>>>
>>> Now I have even simpler idea. Don't add new clock id but just define
>>> here the CLK_TSADC as:
>>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>>
>>> With this change the second patch wouldn't be needed however this does
>>> not reflect the Exynos 4x12 datasheet.
>>>
>>> Any comments?
>>>
>>
>> I think it's better to reflect the datasheet so I prefer your original
>> patch.
>
> Yeah, I also like sticking to datasheet but maybe it is not always worth
> to reproduce the datasheet in 100%. It is just thinking out loud.
>

Agreed.

>> Also, wouldn't changing the CLK_TSADC gate definition cause a
>> regression on an Exynos4210 board that is using the tsadc clock? or
>> maybe I misunderstood the explanation of your Patch 2/2?
>
> No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
> in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
> 1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
> 2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
> register of PCLK_ADC. The id would stay the same as on Exynos4210.
>

I see, thanks for the explanation. Both approaches seems sensible for
me then, I don't have a strong opinion on either though.

> Best regards,
> Krzysztof
>

Best regards,
Javier

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Tomasz Figa]

2015-06-11 21:40 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
>> Hello Krzysztof,
>>
>> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>>> Exynos4x12.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> ---
>>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>>> index 714d6ba782c8..5f32410a01f8 100644
>>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>>> @@ -85,6 +85,7 @@
>>>>  #define DIV_PERIL4           0xc560
>>>>  #define DIV_PERIL5           0xc564
>>>>  #define E4X12_DIV_CAM1               0xc568
>>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>>  #define GATE_SCLK_CAM                0xc820
>>>>  #define GATE_IP_CAM          0xc920
>>>>  #define GATE_IP_TV           0xc924
>>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>>               0),
>>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>>               0),
>>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>>> +             0),
>>>
>>> Now I have even simpler idea. Don't add new clock id but just define
>>> here the CLK_TSADC as:
>>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>>
>>> With this change the second patch wouldn't be needed however this does
>>> not reflect the Exynos 4x12 datasheet.
>>>
>>> Any comments?
>>>
>>
>> I think it's better to reflect the datasheet so I prefer your original
>> patch.
>
> Yeah, I also like sticking to datasheet but maybe it is not always worth
> to reproduce the datasheet in 100%. It is just thinking out loud.
>
>> Also, wouldn't changing the CLK_TSADC gate definition cause a
>> regression on an Exynos4210 board that is using the tsadc clock? or
>> maybe I misunderstood the explanation of your Patch 2/2?
>
> No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
> in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
> 1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
> 2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
> register of PCLK_ADC. The id would stay the same as on Exynos4210.

Or we can:
3. Add new CLK_PCLK_ADC macro equal to current CLK_TSADC. Then drivers
and dtsi would be able to use the name matching the datasheet, but the
ID (and so DT ABI) would be preserved.

Best regards,
Tomasz

[PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Tomasz Figa]

2015-06-11 21:40 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
>> Hello Krzysztof,
>>
>> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>>> Exynos4x12.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> ---
>>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>>> index 714d6ba782c8..5f32410a01f8 100644
>>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>>> @@ -85,6 +85,7 @@
>>>>  #define DIV_PERIL4           0xc560
>>>>  #define DIV_PERIL5           0xc564
>>>>  #define E4X12_DIV_CAM1               0xc568
>>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>>  #define GATE_SCLK_CAM                0xc820
>>>>  #define GATE_IP_CAM          0xc920
>>>>  #define GATE_IP_TV           0xc924
>>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>>               0),
>>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>>               0),
>>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>>> +             0),
>>>
>>> Now I have even simpler idea. Don't add new clock id but just define
>>> here the CLK_TSADC as:
>>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>>
>>> With this change the second patch wouldn't be needed however this does
>>> not reflect the Exynos 4x12 datasheet.
>>>
>>> Any comments?
>>>
>>
>> I think it's better to reflect the datasheet so I prefer your original
>> patch.
>
> Yeah, I also like sticking to datasheet but maybe it is not always worth
> to reproduce the datasheet in 100%. It is just thinking out loud.
>
>> Also, wouldn't changing the CLK_TSADC gate definition cause a
>> regression on an Exynos4210 board that is using the tsadc clock? or
>> maybe I misunderstood the explanation of your Patch 2/2?
>
> No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
> in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
> 1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
> 2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
> register of PCLK_ADC. The id would stay the same as on Exynos4210.

Or we can:
3. Add new CLK_PCLK_ADC macro equal to current CLK_TSADC. Then drivers
and dtsi would be able to use the name matching the datasheet, but the
ID (and so DT ABI) would be preserved.

Best regards,
Tomasz

Re: [PATCH 1/2] clk: exynos4: Add PCLK_ADC gate clock on Exynos4x12

[Tomasz Figa]

2015-06-11 21:40 GMT+09:00 Krzysztof Kozlowski <k.kozlowski@samsung.com>:
> W dniu 11.06.2015 o 21:15, Javier Martinez Canillas pisze:
>> Hello Krzysztof,
>>
>> On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
>> <k.kozlowski@samsung.com> wrote:
>>> W dniu 11.06.2015 o 17:26, Krzysztof Kozlowski pisze:
>>>> Add proper gate clock for the Analog to Digital Converter (ADC) on
>>>> Exynos4x12.
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> ---
>>>>  drivers/clk/samsung/clk-exynos4.c   | 3 +++
>>>>  include/dt-bindings/clock/exynos4.h | 5 ++++-
>>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
>>>> index 714d6ba782c8..5f32410a01f8 100644
>>>> --- a/drivers/clk/samsung/clk-exynos4.c
>>>> +++ b/drivers/clk/samsung/clk-exynos4.c
>>>> @@ -85,6 +85,7 @@
>>>>  #define DIV_PERIL4           0xc560
>>>>  #define DIV_PERIL5           0xc564
>>>>  #define E4X12_DIV_CAM1               0xc568
>>>> +#define E4X12_GATE_BUS_FSYS1 0xc744
>>>>  #define GATE_SCLK_CAM                0xc820
>>>>  #define GATE_IP_CAM          0xc920
>>>>  #define GATE_IP_TV           0xc924
>>>> @@ -1095,6 +1096,8 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
>>>>               0),
>>>>       GATE(CLK_PPMUIMAGE, "ppmuimage", "aclk200", E4X12_GATE_IP_IMAGE, 9, 0,
>>>>               0),
>>>> +     GATE(CLK_PCLK_ADC, "pclk_adc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0,
>>>> +             0),
>>>
>>> Now I have even simpler idea. Don't add new clock id but just define
>>> here the CLK_TSADC as:
>>> GATE(CLK_TSADC, "tsadc", "aclk133", E4X12_GATE_BUS_FSYS1, 16, 0, 0);
>>>
>>> With this change the second patch wouldn't be needed however this does
>>> not reflect the Exynos 4x12 datasheet.
>>>
>>> Any comments?
>>>
>>
>> I think it's better to reflect the datasheet so I prefer your original
>> patch.
>
> Yeah, I also like sticking to datasheet but maybe it is not always worth
> to reproduce the datasheet in 100%. It is just thinking out loud.
>
>> Also, wouldn't changing the CLK_TSADC gate definition cause a
>> regression on an Exynos4210 board that is using the tsadc clock? or
>> maybe I misunderstood the explanation of your Patch 2/2?
>
> No, no. The Exynos4210 would be unchanged. It has the CLK_TSADC - both
> in hardware and in kernel driver. The Exynos4x12 SoCs don't have so we can:
> 1. Add new CLK_PCLK_ADC (id and clock) reflecting datasheet.
> 2. Add only CLK_TSADC clock  on Exynos4x12 which will be using the
> register of PCLK_ADC. The id would stay the same as on Exynos4210.

Or we can:
3. Add new CLK_PCLK_ADC macro equal to current CLK_TSADC. Then drivers
and dtsi would be able to use the name matching the datasheet, but the
ID (and so DT ABI) would be preserved.

Best regards,
Tomasz