2015-06-11 08:26:57

by Krzysztof Kozlowski

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

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

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
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


2015-06-11 08:27:15

by Krzysztof Kozlowski

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

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 <[email protected]>
---
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

2015-06-11 10:44:00

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> 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 */
>

2015-06-11 12:15:49

by Javier Martinez Canillas

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

Hello Krzysztof,

On Thu, Jun 11, 2015 at 12:43 PM, Krzysztof Kozlowski
<[email protected]> 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 <[email protected]>
>> ---
>> 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

2015-06-11 12:40:37

by Krzysztof Kozlowski

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

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
> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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

2015-06-11 12:58:47

by Javier Martinez Canillas

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

Hello Krzysztof,

On Thu, Jun 11, 2015 at 2:40 PM, Krzysztof Kozlowski
<[email protected]> 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
>> <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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

2015-06-11 13:55:09

by Tomasz Figa

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

2015-06-11 21:40 GMT+09:00 Krzysztof Kozlowski <[email protected]>:
> 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
>> <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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