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

2013-04-08 Thread Sylwester Nawrocki
Hi,

On 04/06/2013 12:16 PM, Padma Venkat wrote:
> On Fri, Apr 5, 2013 at 6:24 PM, Sylwester Nawrocki
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-06 Thread Padma Venkat
Hi Sylwester,

On Fri, Apr 5, 2013 at 6:24 PM, Sylwester Nawrocki
 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_bus2
> mout_i2s3
> dout_i2sclk04
> clk_i2s05
> clk_pcm06
>
>
> Example 1: An example of a clock controller node is listed below.
>
> clock_audss: clock-controller@0381 {
> compatible = "samsung,exynos4-audss-clock";
> reg = <0x0381 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@0383 {
> compatible = "samsung,i2s-v5";
> reg = <0x0383 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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-05 Thread 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_bus2
mout_i2s3
dout_i2sclk04
clk_i2s05
clk_pcm06


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

clock_audss: clock-controller@0381 {
compatible = "samsung,exynos4-audss-clock";
reg = <0x0381 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@0383 {
compatible = "samsung,i2s-v5";
reg = <0x0383 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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html