[PATCH 0/3] clk: Exynos: Register audio subsytem clocks using common clk framework

[PATCH 0/3] clk: Exynos: Register audio subsytem clocks using common clk framework

[Padmavathi Venna]

Samsung Exynos SoC has a separate subsystem for audio. This subsystem
has a internal clock controller which controls i2s0 and pcm0 clocks.
This patch series adds the Samsung Exynos SoC audio subsytem clock code
to the common clock framework and provides the I2S0 clock information in
the dtsi file.

Padmavathi Venna (3):
  clk: exynos: register audio subsystem clocks using common clock
    framework
  ARM: dts: add Exynos audio subsystem clock controller node
  ARM: dts: add clock provider information for i2s0 controller in
    Exynos5250

 arch/arm/boot/dts/exynos5250.dtsi      |    8 ++
 drivers/clk/samsung/Makefile           |    1 +
 drivers/clk/samsung/clk-exynos-audss.c |  139 ++++++++++++++++++++++++++++++++
 3 files changed, 148 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clk/samsung/clk-exynos-audss.c

-- 
1.7.4.4

Re: [PATCH 0/3] clk: Exynos: Register audio subsytem clocks using common clk framework

[Sylwester Nawrocki]

Hi Padmavathi,

On 04/05/2013 08:40 AM, Padmavathi Venna wrote:
> Samsung Exynos SoC has a separate subsystem for audio. This subsystem
> has a internal clock controller which controls i2s0 and pcm0 clocks.
> This patch series adds the Samsung Exynos SoC audio subsytem clock code
> to the common clock framework and provides the I2S0 clock information in
> the dtsi file.
> 
> Padmavathi Venna (3):
>   clk: exynos: register audio subsystem clocks using common clock
>     framework
>   ARM: dts: add Exynos audio subsystem clock controller node
>   ARM: dts: add clock provider information for i2s0 controller in
>     Exynos5250
> 
>  arch/arm/boot/dts/exynos5250.dtsi      |    8 ++
>  drivers/clk/samsung/Makefile           |    1 +
>  drivers/clk/samsung/clk-exynos-audss.c |  139 ++++++++++++++++++++++++++++++++
>  3 files changed, 148 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/clk/samsung/clk-exynos-audss.c

It looks good, it's very similar what we have written recently for Exynos4.
It seems the binding documentation is missing in this patch set. I've included
below content of our .../bindings/clock/exynos4-audss-clock.txt file. Feel free
to reuse any parts of it. 

>From a brief look Exynos4 and Exynos5 Audio Subsystem CLKCON very similar.
I've just found bit 2 of 0x0381_0008 register is not used on Exynos5250.

Additionally the Audio Subsystem Clock controller is present on S5PV210
SoCs and IMO compatible property you used is too generic. I would propose
to use at least:

"samsung,s5pv210-audss-clock"    - for S5PV210 
"samsung,exynos4210-audss-clock" - for Exynos4
"samsung,exynos5250-audss-clock" - for Exynos5


8<---------------------------------------------------------------------------
* Samsung Exynos4 Audio Subsystem Clock Controller

The Exynos4 Audio Subsystem clock controller generates and supplies clocks
to Audio Subsystem block available in the Exynos4 SoCs. The clock binding
described here is applicable to all SoC's in the Exynos4 family.

Required Properties:

- compatible: should be one of the following:
  - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.

- reg: physical base address and length of the controller's register set.

- #clock-cells: should be 1.

The following is the list of clocks generated by the controller. Each clock is
assigned an identifier and client nodes use this identifier to specify the
clock which they consume. Some of the clocks are available only on a particular
Exynos4 SoC and this is specified where applicable.

Provided clocks:

Clock		ID	SoC (if specific)
-----------------------------------------------

mout_audss	0
dout_rp		1
dout_aud_bus	2
mout_i2s	3
dout_i2sclk0	4
clk_i2s0	5
clk_pcm0	6


Example 1: An example of a clock controller node is listed below.

clock_audss: clock-controller@03810000 {
	compatible = "samsung,exynos4-audss-clock";
	reg = <0x03810000 0x0C>;
	#clock-cells = <1>;
};

Example 2: I2S controller node that consumes the clock generated by the clock
	   controller. Refer to the standard clock bindings for information
	   about 'clocks' and 'clock-names' property.

i2s0: i2s@03830000 {
	compatible = "samsung,i2s-v5";
	reg = <0x03830000 0x100>;
	clocks = <&clock_audss 0>, <&clock_audss 3>, <&clock_audss 1>,
		<&clock_audss 2>, <&clock_audss 4>, <&clock_audss 2>;
	clock-names = "mout_audss", "mout_i2s", "dout_srp",
		"dout_bus", "dout_i2s", "i2s_opclk0";
};

8<---------------------------------------------------------------------------

Thanks,
Sylwester

Re: [PATCH 0/3] clk: Exynos: Register audio subsytem clocks using common clk framework

[Padma Venkat]

Hi Sylwester,

On Fri, Apr 5, 2013 at 6:24 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Hi Padmavathi,
>
> On 04/05/2013 08:40 AM, Padmavathi Venna wrote:
>> Samsung Exynos SoC has a separate subsystem for audio. This subsystem
>> has a internal clock controller which controls i2s0 and pcm0 clocks.
>> This patch series adds the Samsung Exynos SoC audio subsytem clock code
>> to the common clock framework and provides the I2S0 clock information in
>> the dtsi file.
>>
>> Padmavathi Venna (3):
>>   clk: exynos: register audio subsystem clocks using common clock
>>     framework
>>   ARM: dts: add Exynos audio subsystem clock controller node
>>   ARM: dts: add clock provider information for i2s0 controller in
>>     Exynos5250
>>
>>  arch/arm/boot/dts/exynos5250.dtsi      |    8 ++
>>  drivers/clk/samsung/Makefile           |    1 +
>>  drivers/clk/samsung/clk-exynos-audss.c |  139 ++++++++++++++++++++++++++++++++
>>  3 files changed, 148 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/clk/samsung/clk-exynos-audss.c
>
> It looks good, it's very similar what we have written recently for Exynos4.
> It seems the binding documentation is missing in this patch set. I've included
> below content of our .../bindings/clock/exynos4-audss-clock.txt file. Feel free
> to reuse any parts of it.

Thanks for your review and help. I forgot to include documentation
file. I will reuse your's.

>
> From a brief look Exynos4 and Exynos5 Audio Subsystem CLKCON very similar.
> I've just found bit 2 of 0x0381_0008 register is not used on Exynos5250.

I added bit 2 as i2s_bus gate clock or you are pointing to something else?

>
> Additionally the Audio Subsystem Clock controller is present on S5PV210
> SoCs and IMO compatible property you used is too generic. I would propose
> to use at least:
>
> "samsung,s5pv210-audss-clock"    - for S5PV210
> "samsung,exynos4210-audss-clock" - for Exynos4
> "samsung,exynos5250-audss-clock" - for Exynos5

Different compatible names means different driver files for exynos4 and 5??
I haven't seen any difference between Exynos4 and Exynos5 audio subsystem.
Can't we maintain one for both? If any extra clock instance is added
on newer SoCs
anyway it gets added at the end of the list of clocks. Please correct
me if I am wrong.

>
>
> 8<---------------------------------------------------------------------------
> * Samsung Exynos4 Audio Subsystem Clock Controller
>
> The Exynos4 Audio Subsystem clock controller generates and supplies clocks
> to Audio Subsystem block available in the Exynos4 SoCs. The clock binding
> described here is applicable to all SoC's in the Exynos4 family.
>
> Required Properties:
>
> - compatible: should be one of the following:
>   - "samsung,exynos4210-audss-clock" - controller compatible with all Exynos4 SoCs.
>
> - reg: physical base address and length of the controller's register set.
>
> - #clock-cells: should be 1.
>
> The following is the list of clocks generated by the controller. Each clock is
> assigned an identifier and client nodes use this identifier to specify the
> clock which they consume. Some of the clocks are available only on a particular
> Exynos4 SoC and this is specified where applicable.
>
> Provided clocks:
>
> Clock           ID      SoC (if specific)
> -----------------------------------------------
>
> mout_audss      0
> dout_rp         1
> dout_aud_bus    2
> mout_i2s        3
> dout_i2sclk0    4
> clk_i2s0        5
> clk_pcm0        6
>
>
> Example 1: An example of a clock controller node is listed below.
>
> clock_audss: clock-controller@03810000 {
>         compatible = "samsung,exynos4-audss-clock";
>         reg = <0x03810000 0x0C>;
>         #clock-cells = <1>;
> };
>
> Example 2: I2S controller node that consumes the clock generated by the clock
>            controller. Refer to the standard clock bindings for information
>            about 'clocks' and 'clock-names' property.
>
> i2s0: i2s@03830000 {
>         compatible = "samsung,i2s-v5";
>         reg = <0x03830000 0x100>;
>         clocks = <&clock_audss 0>, <&clock_audss 3>, <&clock_audss 1>,
>                 <&clock_audss 2>, <&clock_audss 4>, <&clock_audss 2>;
>         clock-names = "mout_audss", "mout_i2s", "dout_srp",
>                 "dout_bus", "dout_i2s", "i2s_opclk0";
> };
>
> 8<---------------------------------------------------------------------------
>
> Thanks,
> Sylwester

Thanks
Padma

Re: [PATCH 0/3] clk: Exynos: Register audio subsytem clocks using common clk framework

[Sylwester Nawrocki]

Hi,

On 04/06/2013 12:16 PM, Padma Venkat wrote:
> On Fri, Apr 5, 2013 at 6:24 PM, Sylwester Nawrocki
> <s.nawrocki@samsung.com> wrote:
>>
>> From a brief look Exynos4 and Exynos5 Audio Subsystem CLKCON very similar.
>> I've just found bit 2 of 0x0381_0008 register is not used on Exynos5250.
> 
> I added bit 2 as i2s_bus gate clock or you are pointing to something else?

Sorry, I should have written "bit 1".

>> Additionally the Audio Subsystem Clock controller is present on S5PV210
>> SoCs and IMO compatible property you used is too generic. I would propose
>> to use at least:
>>
>> "samsung,s5pv210-audss-clock"    - for S5PV210
>> "samsung,exynos4210-audss-clock" - for Exynos4
>> "samsung,exynos5250-audss-clock" - for Exynos5
> 
> Different compatible names means different driver files for exynos4 and 5??

No, it would be same driver but with multiple entries in the of_match_table.

> I haven't seen any difference between Exynos4 and Exynos5 audio subsystem.
> Can't we maintain one for both? If any extra clock instance is added
> on newer SoCs
> anyway it gets added at the end of the list of clocks. Please correct
> me if I am wrong.

Sure, the differences between Exynos4 and Exynos5 audio subsystem are very
minor ones. So we could easily use one clock driver for those. Probably same
driver can be used for S5PV210, an any differences could be handled in the
driver based on the compatible property.


Thanks,
Sylwester

-- 
Sylwester Nawrocki
Samsung Poland R&D Center