[PATCH] ARM: shmobile: r8a7790: add audio clocks

[PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
>> Laurent

Could you please check this patch ?

>> Simon

This patch is based on renesas-devel-v3.13-rc5-20131230 tag

 arch/arm/boot/dts/r8a7790.dtsi         |   29 +++++++++++++++++++++++++++++
 arch/arm/mach-shmobile/clock-r8a7790.c |   16 ++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index f48487c..abe68b9 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -312,6 +312,27 @@
 			clock-frequency = <0>;
 			clock-output-names = "extal";
 		};
+		audio_clk_a: audio_clk_a {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			/* This value can be overriden by the board if exist */
+			clock-frequency = <0>;
+			clock-output-names = "audio_clk_a";
+		};
+		audio_clk_b: audio_clk_b {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			/* This value can be overriden by the board if exist */
+			clock-frequency = <0>;
+			clock-output-names = "audio_clk_b";
+		};
+		audio_clk_c: audio_clk_c {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			/* This value can be overriden by the board if exist */
+			clock-frequency = <0>;
+			clock-output-names = "audio_clk_c";
+		};
 
 		/* Special CPG clocks */
 		cpg_clocks: cpg_clocks@e6150000 {
@@ -522,6 +543,14 @@
 			clock-mult = <1>;
 			clock-output-names = "cp";
 		};
+		audio_clk_internal: audio_clk_internal {
+			compatible = "fixed-factor-clock";
+			clocks = <&m2_clk>;
+			#clock-cells = <0>;
+			clock-div = <1>;
+			clock-mult = <1>;
+			clock-output-names = "audio_clk_internal";
+		};
 
 		/* Gate clocks */
 		mstp0_clks: mstp0_clks@e6150130 {
diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c b/arch/arm/mach-shmobile/clock-r8a7790.c
index c5c60ec..9dd5499b4 100644
--- a/arch/arm/mach-shmobile/clock-r8a7790.c
+++ b/arch/arm/mach-shmobile/clock-r8a7790.c
@@ -82,6 +82,15 @@ static struct clk main_clk = {
 	.ops	= &followparent_clk_ops,
 };
 
+static struct clk audio_clk_a = {
+};
+
+static struct clk audio_clk_b = {
+};
+
+static struct clk audio_clk_c = {
+};
+
 /*
  * clock ratio of these clock will be updated
  * on r8a7790_clock_init()
@@ -115,6 +124,9 @@ SH_FIXED_RATIO_CLK_SET(ddr_clk,			pll3_clk,	1, 8);
 SH_FIXED_RATIO_CLK_SET(mp_clk,			pll1_div2_clk,	1, 15);
 
 static struct clk *main_clks[] = {
+	&audio_clk_a,
+	&audio_clk_b,
+	&audio_clk_c,
 	&extal_clk,
 	&extal_div2_clk,
 	&main_clk,
@@ -246,6 +258,10 @@ static struct clk mstp_clks[MSTP_NR] = {
 static struct clk_lookup lookups[] = {
 
 	/* main clocks */
+	CLKDEV_CON_ID("audio_clk_a",	&audio_clk_a),
+	CLKDEV_CON_ID("audio_clk_b",	&audio_clk_b),
+	CLKDEV_CON_ID("audio_clk_c",	&audio_clk_c),
+	CLKDEV_CON_ID("audio_clk_internal",	&m2_clk),
 	CLKDEV_CON_ID("extal",		&extal_clk),
 	CLKDEV_CON_ID("extal_div2",	&extal_div2_clk),
 	CLKDEV_CON_ID("main",		&main_clk),
-- 
1.7.9.5

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Laurent Pinchart]

Hi Morimoto-san,

Thank you for the patch.

On Monday 06 January 2014 00:18:31 Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
> 
> >> Laurent
> 
> Could you please check this patch ?
> 
> >> Simon
> 
> This patch is based on renesas-devel-v3.13-rc5-20131230 tag
> 
>  arch/arm/boot/dts/r8a7790.dtsi         |   29 +++++++++++++++++++++++++++++
> arch/arm/mach-shmobile/clock-r8a7790.c |   16 ++++++++++++++++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index f48487c..abe68b9 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -312,6 +312,27 @@
>  			clock-frequency = <0>;
>  			clock-output-names = "extal";
>  		};
> +		audio_clk_a: audio_clk_a {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			/* This value can be overriden by the board if exist */
> +			clock-frequency = <0>;
> +			clock-output-names = "audio_clk_a";
> +		};
> +		audio_clk_b: audio_clk_b {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			/* This value can be overriden by the board if exist */
> +			clock-frequency = <0>;
> +			clock-output-names = "audio_clk_b";
> +		};
> +		audio_clk_c: audio_clk_c {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +			/* This value can be overriden by the board if exist */
> +			clock-frequency = <0>;
> +			clock-output-names = "audio_clk_c";
> +		};

There three clocks could be provided by an external clock generator, so 
"fixed-clock" might not be the right compatible string and might need to be 
overriden by the board as well. I would thus replace the three comments by a 
single one on top of the audio_clk_* nodes that goes like

"The external audio clocks are configured as 0 Hz fixed frequency clocks by 
default. Boards that provide audio clocks should override them."

>  		/* Special CPG clocks */
>  		cpg_clocks: cpg_clocks@e6150000 {
> @@ -522,6 +543,14 @@
>  			clock-mult = <1>;
>  			clock-output-names = "cp";
>  		};
> +		audio_clk_internal: audio_clk_internal {
> +			compatible = "fixed-factor-clock";
> +			clocks = <&m2_clk>;
> +			#clock-cells = <0>;
> +			clock-div = <1>;
> +			clock-mult = <1>;
> +			clock-output-names = "audio_clk_internal";
> +		};

Do you really need an internal clock, can't you reference m2_clk directly 
instead ?

>  		/* Gate clocks */
>  		mstp0_clks: mstp0_clks@e6150130 {
> diff --git a/arch/arm/mach-shmobile/clock-r8a7790.c
> b/arch/arm/mach-shmobile/clock-r8a7790.c index c5c60ec..9dd5499b4 100644
> --- a/arch/arm/mach-shmobile/clock-r8a7790.c
> +++ b/arch/arm/mach-shmobile/clock-r8a7790.c
> @@ -82,6 +82,15 @@ static struct clk main_clk = {
>  	.ops	= &followparent_clk_ops,
>  };
> 
> +static struct clk audio_clk_a = {
> +};
> +
> +static struct clk audio_clk_b = {
> +};
> +
> +static struct clk audio_clk_c = {
> +};
> +
>  /*
>   * clock ratio of these clock will be updated
>   * on r8a7790_clock_init()
> @@ -115,6 +124,9 @@ SH_FIXED_RATIO_CLK_SET(ddr_clk,			pll3_clk,	1, 8);
>  SH_FIXED_RATIO_CLK_SET(mp_clk,			pll1_div2_clk,	1, 15);
> 
>  static struct clk *main_clks[] = {
> +	&audio_clk_a,
> +	&audio_clk_b,
> +	&audio_clk_c,
>  	&extal_clk,
>  	&extal_div2_clk,
>  	&main_clk,
> @@ -246,6 +258,10 @@ static struct clk mstp_clks[MSTP_NR] = {
>  static struct clk_lookup lookups[] = {
> 
>  	/* main clocks */
> +	CLKDEV_CON_ID("audio_clk_a",	&audio_clk_a),
> +	CLKDEV_CON_ID("audio_clk_b",	&audio_clk_b),
> +	CLKDEV_CON_ID("audio_clk_c",	&audio_clk_c),
> +	CLKDEV_CON_ID("audio_clk_internal",	&m2_clk),
>  	CLKDEV_CON_ID("extal",		&extal_clk),
>  	CLKDEV_CON_ID("extal_div2",	&extal_div2_clk),
>  	CLKDEV_CON_ID("main",		&main_clk),
-- 
Regards,

Laurent Pinchart

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

Hi Laurent

Thank you for your review

> > +		audio_clk_a: audio_clk_a {
> > +			compatible = "fixed-clock";
> > +			#clock-cells = <0>;
> > +			/* This value can be overriden by the board if exist */
> > +			clock-frequency = <0>;
> > +			clock-output-names = "audio_clk_a";
> > +		};
> > +		audio_clk_b: audio_clk_b {
> > +			compatible = "fixed-clock";
> > +			#clock-cells = <0>;
> > +			/* This value can be overriden by the board if exist */
> > +			clock-frequency = <0>;
> > +			clock-output-names = "audio_clk_b";
> > +		};
> > +		audio_clk_c: audio_clk_c {
> > +			compatible = "fixed-clock";
> > +			#clock-cells = <0>;
> > +			/* This value can be overriden by the board if exist */
> > +			clock-frequency = <0>;
> > +			clock-output-names = "audio_clk_c";
> > +		};
> 
> There three clocks could be provided by an external clock generator, so 
> "fixed-clock" might not be the right compatible string and might need to be 
> overriden by the board as well. I would thus replace the three comments by a 
> single one on top of the audio_clk_* nodes that goes like
> 
> "The external audio clocks are configured as 0 Hz fixed frequency clocks by 
> default. Boards that provide audio clocks should override them."

I see. will fix in v2

> > +		audio_clk_internal: audio_clk_internal {
> > +			compatible = "fixed-factor-clock";
> > +			clocks = <&m2_clk>;
> > +			#clock-cells = <0>;
> > +			clock-div = <1>;
> > +			clock-mult = <1>;
> > +			clock-output-names = "audio_clk_internal";
> > +		};
> 
> Do you really need an internal clock, can't you reference m2_clk directly 
> instead ?

Unfortunately, it is impossible.
"audio_clk_internal = m2_clk" is only R-car H2 or Gen2.
It is "clks1" in Gen1 case.
So, sound driver is assuming that
it can get internal clock as "audio_clk_internal"

Best regards
---
Kuninori Morimoto

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Laurent Pinchart]

Hi Morimoto-san,

On Tuesday 07 January 2014 16:22:27 Kuninori Morimoto wrote:
> Hi Laurent
> 
> Thank you for your review

You're welcome.

> > > +		audio_clk_a: audio_clk_a {
> > > +			compatible = "fixed-clock";
> > > +			#clock-cells = <0>;
> > > +			/* This value can be overriden by the board if exist */
> > > +			clock-frequency = <0>;
> > > +			clock-output-names = "audio_clk_a";
> > > +		};
> > > +		audio_clk_b: audio_clk_b {
> > > +			compatible = "fixed-clock";
> > > +			#clock-cells = <0>;
> > > +			/* This value can be overriden by the board if exist */
> > > +			clock-frequency = <0>;
> > > +			clock-output-names = "audio_clk_b";
> > > +		};
> > > +		audio_clk_c: audio_clk_c {
> > > +			compatible = "fixed-clock";
> > > +			#clock-cells = <0>;
> > > +			/* This value can be overriden by the board if exist */
> > > +			clock-frequency = <0>;
> > > +			clock-output-names = "audio_clk_c";
> > > +		};
> > 
> > There three clocks could be provided by an external clock generator, so
> > "fixed-clock" might not be the right compatible string and might need to
> > be overriden by the board as well. I would thus replace the three comments
> > by a single one on top of the audio_clk_* nodes that goes like
> > 
> > "The external audio clocks are configured as 0 Hz fixed frequency clocks
> > by default. Boards that provide audio clocks should override them."
> 
> I see. will fix in v2
> 
> > > +		audio_clk_internal: audio_clk_internal {
> > > +			compatible = "fixed-factor-clock";
> > > +			clocks = <&m2_clk>;
> > > +			#clock-cells = <0>;
> > > +			clock-div = <1>;
> > > +			clock-mult = <1>;
> > > +			clock-output-names = "audio_clk_internal";
> > > +		};
> > 
> > Do you really need an internal clock, can't you reference m2_clk directly
> > instead ?
> 
> Unfortunately, it is impossible.
> "audio_clk_internal = m2_clk" is only R-car H2 or Gen2.
> It is "clks1" in Gen1 case.
> So, sound driver is assuming that
> it can get internal clock as "audio_clk_internal"

I believe this should be fixed in the audio driver. Instead of doing

        adg->clk[CLKA] = clk_get(NULL, "audio_clk_a");
        adg->clk[CLKB] = clk_get(NULL, "audio_clk_b");
        adg->clk[CLKC] = clk_get(NULL, "audio_clk_c");
        adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal");

it should do something like

        adg->clk[CLKA] = clk_get(&pdev->dev, "clk_a");
        adg->clk[CLKB] = clk_get(&pdev->dev, "clk_b");
        adg->clk[CLKC] = clk_get(&pdev->dev, "clk_c");
        adg->clk[CLKI] = clk_get(&pdev->dev, "internal");

The audio device DT node should then reference the clocks

        clocks = <&audio_clk_a &audio_clk_b &audio_clk_c &m2_clk>;
        clock-names = "clk_a", "clk_b", "clk_c", "internal";

On Gen1 that would become

        clocks = <&audio_clk_a &audio_clk_b &audio_clk_c &clks1>;
        clock-names = "clk_a", "clk_b", "clk_c", "internal";

Of course this will require DT bindings for the audio driver. Until you get 
that you can create clock aliases as done for the CMT and SCI devices on Lager 
and Koelsch. See the lager_add_standard_devices() function in arch/arm/mach-
shmobile/board-lager-reference.c for instance.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

Hi Laurent

> I believe this should be fixed in the audio driver. Instead of doing
> 
>         adg->clk[CLKA] = clk_get(NULL, "audio_clk_a");
>         adg->clk[CLKB] = clk_get(NULL, "audio_clk_b");
>         adg->clk[CLKC] = clk_get(NULL, "audio_clk_c");
>         adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal");
> 
> it should do something like
> 
>         adg->clk[CLKA] = clk_get(&pdev->dev, "clk_a");
>         adg->clk[CLKB] = clk_get(&pdev->dev, "clk_b");
>         adg->clk[CLKC] = clk_get(&pdev->dev, "clk_c");
>         adg->clk[CLKI] = clk_get(&pdev->dev, "internal");

Current style was assumed non DT case,
but I noticed that your style has reasonable.

I can fixup driver side, but it is impossible now.
Because there is branch dependency/merge issue between Simon <-> ASoC branch.
Sound driver DT support is stalling by same reason.
I'm waiting branch merging (= next merge window ?).

If you can accept, how about this ?
 - add non-DT "audio_clk_xx" support without common clock settings today
 - merge window
 - fixup driver/platform/clock as above style
 - add common clock setting for DT
 - add sound DT support

Best regards
---
Kuninori Morimoto

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

Hi again

> > I believe this should be fixed in the audio driver. Instead of doing
> > 
> >         adg->clk[CLKA] = clk_get(NULL, "audio_clk_a");
> >         adg->clk[CLKB] = clk_get(NULL, "audio_clk_b");
> >         adg->clk[CLKC] = clk_get(NULL, "audio_clk_c");
> >         adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal");
> > 
> > it should do something like
> > 
> >         adg->clk[CLKA] = clk_get(&pdev->dev, "clk_a");
> >         adg->clk[CLKB] = clk_get(&pdev->dev, "clk_b");
> >         adg->clk[CLKC] = clk_get(&pdev->dev, "clk_c");
> >         adg->clk[CLKI] = clk_get(&pdev->dev, "internal");
> 
> Current style was assumed non DT case,
> but I noticed that your style has reasonable.
> 
> I can fixup driver side, but it is impossible now.
> Because there is branch dependency/merge issue between Simon <-> ASoC branch.
> Sound driver DT support is stalling by same reason.
> I'm waiting branch merging (= next merge window ?).
> 
> If you can accept, how about this ?
>  - add non-DT "audio_clk_xx" support without common clock settings today
>  - merge window
>  - fixup driver/platform/clock as above style
>  - add common clock setting for DT
>  - add sound DT support

I can send sound driver support patch
if "audio_clk_xx" patch was accepted.
But it is for non-DT version


Best regards
---
Kuninori Morimoto

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Laurent Pinchart]

Hi Morimoto-san,

On Tuesday 07 January 2014 18:39:54 Kuninori Morimoto wrote:
> Hi again
> 
> > > I believe this should be fixed in the audio driver. Instead of doing
> > > 
> > >         adg->clk[CLKA] = clk_get(NULL, "audio_clk_a");
> > >         adg->clk[CLKB] = clk_get(NULL, "audio_clk_b");
> > >         adg->clk[CLKC] = clk_get(NULL, "audio_clk_c");
> > >         adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal");
> > > 
> > > it should do something like
> > > 
> > >         adg->clk[CLKA] = clk_get(&pdev->dev, "clk_a");
> > >         adg->clk[CLKB] = clk_get(&pdev->dev, "clk_b");
> > >         adg->clk[CLKC] = clk_get(&pdev->dev, "clk_c");
> > >         adg->clk[CLKI] = clk_get(&pdev->dev, "internal");
> > 
> > Current style was assumed non DT case,
> > but I noticed that your style has reasonable.
> > 
> > I can fixup driver side, but it is impossible now.
> > Because there is branch dependency/merge issue between Simon <-> ASoC
> > branch. Sound driver DT support is stalling by same reason.
> > I'm waiting branch merging (= next merge window ?).
> > 
> > If you can accept, how about this ?
> > 
> >  - add non-DT "audio_clk_xx" support without common clock settings today
> >  - merge window
> >  - fixup driver/platform/clock as above style
> >  - add common clock setting for DT
> >  - add sound DT support
> 
> I can send sound driver support patch
> if "audio_clk_xx" patch was accepted.
> But it is for non-DT version

I think you can start by fixing the driver for the non-DT case, DT support can 
come at a later time.

You can first add the following clk_lookup entries to arch/arm/mach-
shmobile/clock-r8a7790.c

        CLKDEV_ICK_ID("rcar_sound", "clk_a",    &audio_clk_a),
        CLKDEV_ICK_ID("rcar_sound", "clk_b",    &audio_clk_b),
        CLKDEV_ICK_ID("rcar_sound", "clk_c",    &audio_clk_c),

and then fix the adg driver with

        adg->clk[CLKA] = clk_get(&pdev->dev, "clk_a");
        adg->clk[CLKB] = clk_get(&pdev->dev, "clk_b");
        adg->clk[CLKC] = clk_get(&pdev->dev, "clk_c");
        adg->clk[CLKI] = clk_get(&pdev->dev, "internal");

-- 
Regards,

Laurent Pinchart

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

Hi Laurent

> > >  - add non-DT "audio_clk_xx" support without common clock settings today
> > >  - merge window
> > >  - fixup driver/platform/clock as above style
> > >  - add common clock setting for DT
> > >  - add sound DT support
> > 
> > I can send sound driver support patch
> > if "audio_clk_xx" patch was accepted.
> > But it is for non-DT version
> 
> I think you can start by fixing the driver for the non-DT case, DT support can 
> come at a later time.
> 
> You can first add the following clk_lookup entries to arch/arm/mach-
> shmobile/clock-r8a7790.c
> 
>         CLKDEV_ICK_ID("rcar_sound", "clk_a",    &audio_clk_a),
>         CLKDEV_ICK_ID("rcar_sound", "clk_b",    &audio_clk_b),
>         CLKDEV_ICK_ID("rcar_sound", "clk_c",    &audio_clk_c),
> 
> and then fix the adg driver with
> 
>         adg->clk[CLKA] = clk_get(&pdev->dev, "clk_a");
>         adg->clk[CLKB] = clk_get(&pdev->dev, "clk_b");
>         adg->clk[CLKC] = clk_get(&pdev->dev, "clk_c");
>         adg->clk[CLKI] = clk_get(&pdev->dev, "internal");

Hmm...
sorry, but I can't agree.
ASoC branch is based on ALSA branch,
and it doesn't include Simon's branch yet.
OTOH, BockW already has sound support on Simon branch,
and it has audio_clk_b settings.

If I used your idea...

 1) add "rcar_sound" base "clk_x" on Simon branch
    but, sound driver can't care about it in this timing.
    => Sound doesn't work on Lager, I can't send sound support patch.
    => Sound will be supported next-next merge timing

 2) fixup adg driver needs Simon branch merge.
    because it should care BockW in same timing.
    => This fixup will happen after next merge timing

If you can accept "current style" audio_clk_x...

 1) I can send sound support patch immediately for Lager
 2) adg driver fixup happen after next merge,
    then, BockW/Lager are cared in same patch.

There is confusable branch merge timing issue here...
Your idea is nice, but it needs very long time.
This means Lager sound support will be late.

Best regards
---
Kuninori Morimoto

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Laurent Pinchart]

Hi Morimoto-san,

On Wednesday 08 January 2014 01:11:37 Kuninori Morimoto wrote:
> Hi Laurent
> 
> > > >  - add non-DT "audio_clk_xx" support without common clock settings
> > > >  today
> > > >  - merge window
> > > >  - fixup driver/platform/clock as above style
> > > >  - add common clock setting for DT
> > > >  - add sound DT support
> > > 
> > > I can send sound driver support patch
> > > if "audio_clk_xx" patch was accepted.
> > > But it is for non-DT version
> > 
> > I think you can start by fixing the driver for the non-DT case, DT support
> > can come at a later time.
> > 
> > You can first add the following clk_lookup entries to arch/arm/mach-
> > shmobile/clock-r8a7790.c
> > 
> >         CLKDEV_ICK_ID("rcar_sound", "clk_a",    &audio_clk_a),
> >         CLKDEV_ICK_ID("rcar_sound", "clk_b",    &audio_clk_b),
> >         CLKDEV_ICK_ID("rcar_sound", "clk_c",    &audio_clk_c),
> > 
> > and then fix the adg driver with
> > 
> >         adg->clk[CLKA] = clk_get(&pdev->dev, "clk_a");
> >         adg->clk[CLKB] = clk_get(&pdev->dev, "clk_b");
> >         adg->clk[CLKC] = clk_get(&pdev->dev, "clk_c");
> >         adg->clk[CLKI] = clk_get(&pdev->dev, "internal");
> 
> Hmm...
> sorry, but I can't agree.
> ASoC branch is based on ALSA branch,
> and it doesn't include Simon's branch yet.
> OTOH, BockW already has sound support on Simon branch,
> and it has audio_clk_b settings.
> 
> If I used your idea...
> 
>  1) add "rcar_sound" base "clk_x" on Simon branch
>     but, sound driver can't care about it in this timing.
>     => Sound doesn't work on Lager, I can't send sound support patch.
>     => Sound will be supported next-next merge timing
> 
>  2) fixup adg driver needs Simon branch merge.
>     because it should care BockW in same timing.
>     => This fixup will happen after next merge timing
> 
> If you can accept "current style" audio_clk_x...
> 
>  1) I can send sound support patch immediately for Lager
>  2) adg driver fixup happen after next merge,
>     then, BockW/Lager are cared in same patch.
> 
> There is confusable branch merge timing issue here...
> Your idea is nice, but it needs very long time.
> This means Lager sound support will be late.

Right, timing might be affected.

You will have to coordinate the ASoC and shmobile patches. As BockW needs to 
be fixed as well, we have two options. The first one is

1. Add the new style clocks to BockW
2. Modify the ADG driver to use the new style clocks
3. Remove the old style clocks from BockW

while the second one is

1. Add support for the new style clocks to the ADG driver
2. Replace the old style clocks with the new style clocks in BockW
3. Remove support for the old style clocks from the ADG driver

I believe the first option will be easier. Steps 1 and 3 will go through 
Simon's tree, while step 2 will go through the ASoC tree. Given that the arm-
soc tree has been closed for v3.14 we're thus looking at v3.15 for 1 and 2, 
and either v3.15 or v3.16 for 3.

Adding Lager, we could thus do

1. Add the new style clocks to BockW
2. Add the new style clocks to Lager
3. Modify the ADG driver to use the new style clocks
4. Remove the old style clocks from BockW

The first 3 steps would be applied to v3.15 and the last one to either v3.15 
or v3.16.

Starting with the current-style audio clocks for Lager is possible, but I 
don't think it could be applied before v3.15 anyway, unless considered as a 
bug fix. Simon, what's your opinion on that ?

-- 
Regards,

Laurent Pinchart

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

Hi Laurent

Thank you for your help

> 1. Add the new style clocks to BockW
> 2. Modify the ADG driver to use the new style clocks
> 3. Remove the old style clocks from BockW
(snip)
> 1. Add support for the new style clocks to the ADG driver
> 2. Replace the old style clocks with the new style clocks in BockW
> 3. Remove support for the old style clocks from the ADG driver
(snip)
> 1. Add the new style clocks to BockW
> 2. Add the new style clocks to Lager
> 3. Modify the ADG driver to use the new style clocks
> 4. Remove the old style clocks from BockW

Thank you for explaining your opinion.
I'm not sure detail, but
ALSA and ARM branch new feature time limit is different.
(ARM branch time limit is until rc5 ?)
 => different merge timing (?)

And maybe these idea will be break BockW sound or
makes confusable code or etc etc etc...

Anyway, I don't want complex patch/branch magic.
And linus/master is rc7 today.
So, I will re-send these patches
after merge window again.
(lager sound support will be late though...)

Thank you

Best regards
---
Kuninori Morimoto

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Laurent Pinchart]

Hi Morimoto-san,

On Thursday 09 January 2014 16:19:38 Kuninori Morimoto wrote:
> Hi Laurent
> 
> Thank you for your help
> 
> > 1. Add the new style clocks to BockW
> > 2. Modify the ADG driver to use the new style clocks
> > 3. Remove the old style clocks from BockW
> 
> (snip)
> 
> > 1. Add support for the new style clocks to the ADG driver
> > 2. Replace the old style clocks with the new style clocks in BockW
> > 3. Remove support for the old style clocks from the ADG driver
> 
> (snip)
> 
> > 1. Add the new style clocks to BockW
> > 2. Add the new style clocks to Lager
> > 3. Modify the ADG driver to use the new style clocks
> > 4. Remove the old style clocks from BockW
> 
> Thank you for explaining your opinion.
> I'm not sure detail, but
> ALSA and ARM branch new feature time limit is different.
> (ARM branch time limit is until rc5 ?)
>  => different merge timing (?)
> 
> And maybe these idea will be break BockW sound or
> makes confusable code or etc etc etc...
> 
> Anyway, I don't want complex patch/branch magic.
> And linus/master is rc7 today.
> So, I will re-send these patches
> after merge window again.
> (lager sound support will be late though...)

To be clear, I'm fine with

1. Add the old style clocks to Lager
2. Add the new style clocks to BockW
3. Add the new style clocks to Lager
4. Modify the ADG driver to use the new style clocks
5. Remove the old style clocks from BockW
6. Remove the old style clocks from Lager

if that can help getting sound support for Lager merge earlier. If 1 can't get 
merged before 2-4, I then believe we should instead go straight for the new 
style clocks for Lager.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

Hi Laurent

Thank you for your help

> 1. Add the old style clocks to Lager
> 2. Add the new style clocks to BockW
> 3. Add the new style clocks to Lager
> 4. Modify the ADG driver to use the new style clocks
> 5. Remove the old style clocks from BockW
> 6. Remove the old style clocks from Lager

Yes it become clean.
But hmm, 
I guess #5, #6 (#4 too ?) are happen after merge anyway ?
If so, this is more clean for me.

1. Add the old style clocks to Lager
2. Modify the ADG driver to use the new style clocks.
   BockW/Lager clocls are fixed in same patch.

Here, #2 will be done on ALSA branch side after merge,
and I guess Simon can use it for topic branch for us ?

# This kind of merge timing something
# is headache for us :P

Best regards
---
Kuninori Morimoto

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Simon Horman]

On Thu, Jan 09, 2014 at 04:51:52PM -0800, Kuninori Morimoto wrote:
> 
> Hi Laurent
> 
> Thank you for your help
> 
> > 1. Add the old style clocks to Lager
> > 2. Add the new style clocks to BockW
> > 3. Add the new style clocks to Lager
> > 4. Modify the ADG driver to use the new style clocks
> > 5. Remove the old style clocks from BockW
> > 6. Remove the old style clocks from Lager
> 
> Yes it become clean.
> But hmm, 
> I guess #5, #6 (#4 too ?) are happen after merge anyway ?
> If so, this is more clean for me.
> 
> 1. Add the old style clocks to Lager
> 2. Modify the ADG driver to use the new style clocks.
>    BockW/Lager clocls are fixed in same patch.
> 
> Here, #2 will be done on ALSA branch side after merge,
> and I guess Simon can use it for topic branch for us ?

Sure, let me know what you need.

> # This kind of merge timing something
> # is headache for us :P

Yes!

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Laurent Pinchart]

Hi Morimoto-san,

On Thursday 09 January 2014 16:51:52 Kuninori Morimoto wrote:
> Hi Laurent
> 
> Thank you for your help
> 
> > 1. Add the old style clocks to Lager
> > 2. Add the new style clocks to BockW
> > 3. Add the new style clocks to Lager
> > 4. Modify the ADG driver to use the new style clocks
> > 5. Remove the old style clocks from BockW
> > 6. Remove the old style clocks from Lager
> 
> Yes it become clean.
> But hmm,
> I guess #5, #6 (#4 too ?) are happen after merge anyway ?
> If so, this is more clean for me.
> 
> 1. Add the old style clocks to Lager
> 2. Modify the ADG driver to use the new style clocks.
>    BockW/Lager clocls are fixed in same patch.

My point is that you need to add the new style clocks to platform code before 
you modify the ADG driver. If adding old style clocks will get merged during 
the same merge window as the new style clocks and the ADG driver changes, I 
don't think there's a point to add them.

> Here, #2 will be done on ALSA branch side after merge,
> and I guess Simon can use it for topic branch for us ?
> 
> # This kind of merge timing something
> # is headache for us :P

-- 
Regards,

Laurent Pinchart

Re: [PATCH] ARM: shmobile: r8a7790: add audio clocks

[Kuninori Morimoto]

Hi Laurent

> > > 1. Add the old style clocks to Lager
> > > 2. Add the new style clocks to BockW
> > > 3. Add the new style clocks to Lager
> > > 4. Modify the ADG driver to use the new style clocks
> > > 5. Remove the old style clocks from BockW
> > > 6. Remove the old style clocks from Lager
(snip)
> My point is that you need to add the new style clocks to platform code before 
> you modify the ADG driver. If adding old style clocks will get merged during 
> the same merge window as the new style clocks and the ADG driver changes, I 
> don't think there's a point to add them.

OK
I send v2 patch-set which has above old/new style clocks soon.
Thank you for your help

Best regards
---
Kuninori Morimoto