[PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Tushar Behera]

I2S driver uses 3 clocks under different conditions. Added two
missing clocks.

Additionally updated pin-control property for this node.

Signed-off-by: Tushar Behera <tushar.b@samsung.com>
---
Based on next-20140606.

Tested on Exynos4210-based Origen boards with private patches for
codec/machine driver.

 arch/arm/boot/dts/exynos4.dtsi |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index fbaf426..10bb081 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -55,11 +55,15 @@
 	i2s0: i2s@03830000 {
 		compatible = "samsung,s5pv210-i2s";
 		reg = <0x03830000 0x100>;
-		clocks = <&clock_audss EXYNOS_I2S_BUS>;
-		clock-names = "iis";
+		clocks = <&clock_audss EXYNOS_I2S_BUS>,
+			<&clock_audss EXYNOS_I2S_BUS>,
+			<&clock_audss EXYNOS_SCLK_I2S>;
+		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
 		dmas = <&pdma0 12>, <&pdma0 11>, <&pdma0 10>;
 		dma-names = "tx", "rx", "tx-sec";
 		samsung,idma-addr = <0x03000000>;
+		pinctrl-0 = <&i2s0_bus>;
+		pinctrl-names = "default";
 		status = "disabled";
 	};
 
-- 
1.7.9.5

Re: [PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Sylwester Nawrocki]

Hi Tushar,

On 07/06/14 11:30, Tushar Behera wrote:
> I2S driver uses 3 clocks under different conditions. Added two
> missing clocks.
> 
> Additionally updated pin-control property for this node.
> 
> Signed-off-by: Tushar Behera <tushar.b@samsung.com>
> ---
> Based on next-20140606.
> 
> Tested on Exynos4210-based Origen boards with private patches for
> codec/machine driver.
> 
>  arch/arm/boot/dts/exynos4.dtsi |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index fbaf426..10bb081 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -55,11 +55,15 @@
>  	i2s0: i2s@03830000 {
>  		compatible = "samsung,s5pv210-i2s";
>  		reg = <0x03830000 0x100>;
> -		clocks = <&clock_audss EXYNOS_I2S_BUS>;
> -		clock-names = "iis";
> +		clocks = <&clock_audss EXYNOS_I2S_BUS>,
> +			<&clock_audss EXYNOS_I2S_BUS>,
> +			<&clock_audss EXYNOS_SCLK_I2S>;
> +		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
>  		dmas = <&pdma0 12>, <&pdma0 11>, <&pdma0 10>;
>  		dma-names = "tx", "rx", "tx-sec";
>  		samsung,idma-addr = <0x03000000>;
> +		pinctrl-0 = <&i2s0_bus>;
> +		pinctrl-names = "default";
>  		status = "disabled";
>  	};

I assumed those properties you're adding here belong to board or
to SoC type specific dts. It doesn't hurt ot have it like this,
but what do you think about moving this to  arch/arm/boot/dts/
exynos4210(4x12).dtsi ? For instance pinctrl definitions are
in SoC specific dts.

--
Thanks,
Sylwester

Re: [PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Tushar Behera]

On 06/09/2014 03:23 PM, Sylwester Nawrocki wrote:
> Hi Tushar,
> 
> On 07/06/14 11:30, Tushar Behera wrote:
>> I2S driver uses 3 clocks under different conditions. Added two
>> missing clocks.
>>
>> Additionally updated pin-control property for this node.
>>
>> Signed-off-by: Tushar Behera <tushar.b@samsung.com>
>> ---
>> Based on next-20140606.
>>
>> Tested on Exynos4210-based Origen boards with private patches for
>> codec/machine driver.
>>
>>  arch/arm/boot/dts/exynos4.dtsi |    8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
>> index fbaf426..10bb081 100644
>> --- a/arch/arm/boot/dts/exynos4.dtsi
>> +++ b/arch/arm/boot/dts/exynos4.dtsi
>> @@ -55,11 +55,15 @@
>>  	i2s0: i2s@03830000 {
>>  		compatible = "samsung,s5pv210-i2s";
>>  		reg = <0x03830000 0x100>;
>> -		clocks = <&clock_audss EXYNOS_I2S_BUS>;
>> -		clock-names = "iis";
>> +		clocks = <&clock_audss EXYNOS_I2S_BUS>,
>> +			<&clock_audss EXYNOS_I2S_BUS>,
>> +			<&clock_audss EXYNOS_SCLK_I2S>;
>> +		clock-names = "iis", "i2s_opclk0", "i2s_opclk1";
>>  		dmas = <&pdma0 12>, <&pdma0 11>, <&pdma0 10>;
>>  		dma-names = "tx", "rx", "tx-sec";
>>  		samsung,idma-addr = <0x03000000>;
>> +		pinctrl-0 = <&i2s0_bus>;
>> +		pinctrl-names = "default";
>>  		status = "disabled";
>>  	};
> 
> I assumed those properties you're adding here belong to board or
> to SoC type specific dts. It doesn't hurt ot have it like this,
> but what do you think about moving this to  arch/arm/boot/dts/
> exynos4210(4x12).dtsi ? For instance pinctrl definitions are
> in SoC specific dts.
> 

IMHO, the clock entries belong here as the audss clock provider
essentially remains same for all Exynos4 based systems.

As for pin-control entry, I believe we can move them to respective SoC
dtsi files.

Does that sound okay to you?

> --
> Thanks,
> Sylwester
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Tushar Behera

Re: [PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Sylwester Nawrocki]

On 09/06/14 12:44, Tushar Behera wrote:
> IMHO, the clock entries belong here as the audss clock provider
> essentially remains same for all Exynos4 based systems.
> 
> As for pin-control entry, I believe we can move them to respective SoC
> dtsi files.
> 
> Does that sound okay to you?

I guess that's fine, I tend to agree with you about the clocks.

--
Thanks,
Sylwester

Re: [PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Tushar Behera]

On 06/09/2014 04:36 PM, Sylwester Nawrocki wrote:
> On 09/06/14 12:44, Tushar Behera wrote:
>> IMHO, the clock entries belong here as the audss clock provider
>> essentially remains same for all Exynos4 based systems.
>>
>> As for pin-control entry, I believe we can move them to respective SoC
>> dtsi files.
>>
>> Does that sound okay to you?
> 
> I guess that's fine, I tend to agree with you about the clocks.
> 

As I was going ahead with the implementation, it felt like repetition
with the same code. Would it be acceptable to you if we go ahead with
the earlier approach of adding both clock and pin-control entry to
exynos4.dtsi?

I can see i2c_n and spi_n adding pin-control entries in exynos4.dtsi itself.

> --
> Thanks,
> Sylwester
> 


-- 
Tushar Behera

Re: [PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Sylwester Nawrocki]

On 16/06/14 13:43, Tushar Behera wrote:
> On 06/09/2014 04:36 PM, Sylwester Nawrocki wrote:
>> > On 09/06/14 12:44, Tushar Behera wrote:
>>> >> IMHO, the clock entries belong here as the audss clock provider
>>> >> essentially remains same for all Exynos4 based systems.
>>> >>
>>> >> As for pin-control entry, I believe we can move them to respective SoC
>>> >> dtsi files.
>>> >>
>>> >> Does that sound okay to you?
>> > 
>> > I guess that's fine, I tend to agree with you about the clocks.
>> > 
> As I was going ahead with the implementation, it felt like repetition
> with the same code. Would it be acceptable to you if we go ahead with
> the earlier approach of adding both clock and pin-control entry to
> exynos4.dtsi?
> 
> I can see i2c_n and spi_n adding pin-control entries in exynos4.dtsi itself.

OK, I don't mind, I'll leave it up to the maintainers.

--
Regards,
Sylwester

Re: [PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Tushar Behera]

On Mon, Jun 16, 2014 at 5:20 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> On 16/06/14 13:43, Tushar Behera wrote:
>> On 06/09/2014 04:36 PM, Sylwester Nawrocki wrote:
>>> > On 09/06/14 12:44, Tushar Behera wrote:
>>>> >> IMHO, the clock entries belong here as the audss clock provider
>>>> >> essentially remains same for all Exynos4 based systems.
>>>> >>
>>>> >> As for pin-control entry, I believe we can move them to respective SoC
>>>> >> dtsi files.
>>>> >>
>>>> >> Does that sound okay to you?
>>> >
>>> > I guess that's fine, I tend to agree with you about the clocks.
>>> >
>> As I was going ahead with the implementation, it felt like repetition
>> with the same code. Would it be acceptable to you if we go ahead with
>> the earlier approach of adding both clock and pin-control entry to
>> exynos4.dtsi?
>>
>> I can see i2c_n and spi_n adding pin-control entries in exynos4.dtsi itself.
>
> OK, I don't mind, I'll leave it up to the maintainers.
>
> --
> Regards,
> Sylwester

Kukjin,

Any comments on this? If not, I will resend this patch based on v3.16-rc1.

-- 
Tushar Behera

Re: [PATCH] ARM: dts: Add missing clock/pin-control entries for I2S0 on Exynos4

[Tomasz Figa]

Hi Tushar, Sylwester,

On 16.06.2014 14:03, Tushar Behera wrote:
> On Mon, Jun 16, 2014 at 5:20 PM, Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>> On 16/06/14 13:43, Tushar Behera wrote:
>>> On 06/09/2014 04:36 PM, Sylwester Nawrocki wrote:
>>>>> On 09/06/14 12:44, Tushar Behera wrote:
>>>>>>> IMHO, the clock entries belong here as the audss clock provider
>>>>>>> essentially remains same for all Exynos4 based systems.
>>>>>>>
>>>>>>> As for pin-control entry, I believe we can move them to respective SoC
>>>>>>> dtsi files.
>>>>>>>
>>>>>>> Does that sound okay to you?
>>>>>
>>>>> I guess that's fine, I tend to agree with you about the clocks.
>>>>>
>>> As I was going ahead with the implementation, it felt like repetition
>>> with the same code. Would it be acceptable to you if we go ahead with
>>> the earlier approach of adding both clock and pin-control entry to
>>> exynos4.dtsi?
>>>
>>> I can see i2c_n and spi_n adding pin-control entries in exynos4.dtsi itself.
>>
>> OK, I don't mind, I'll leave it up to the maintainers.
>>
>> --
>> Regards,
>> Sylwester
> 
> Kukjin,
> 
> Any comments on this? If not, I will resend this patch based on v3.16-rc1.
> 

I'd just keep them all in exynos4.dtsi if they are all the same. I don't
see any reason why those names should ever change to something platform
specific.

Best regards,
Tomasz