diff mbox

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

Message ID 1434011190-24563-1-git-send-email-k.kozlowski@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Krzysztof Kozlowski June 11, 2015, 8:26 a.m. UTC 
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(-)

Comments

Krzysztof Kozlowski June 11, 2015, 10:43 a.m. UTC | #1 
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 */
>
Javier Martinez Canillas June 11, 2015, 12:15 p.m. UTC | #2 
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
Krzysztof Kozlowski June 11, 2015, 12:40 p.m. UTC | #3 
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
Javier Martinez Canillas June 11, 2015, 12:58 p.m. UTC | #4 
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
Tomasz Figa June 11, 2015, 1:54 p.m. UTC | #5 
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
diff mbox

Patch

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