diff mbox

[v2,1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

Message ID 87d25wg804.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kuninori Morimoto Jan. 30, 2015, 5:18 a.m. UTC 
Hi Lars, Simon

> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---

I tested these 3 patches on armadillo800eva

	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

2/3 patch changed behavior for clock inversion on FSI driver.
FSI + wm8978 on armadillo800eva worked without any issues,
but, I don't know how much effect it has for other board.
We used this inversion flags on each board for historical reasons (copy-paste),
but, almost all these were not needed (except some picky board) on FSI.
Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
And unfortunately, Renesas don't use FSI anymore.
So, I think keeping current FSI driver as-is is more safety for old boards.
armadillo800eva works well with below patch (as 2/3 patch)

--------------
--------------

Comments

Simon Horman Feb. 11, 2015, 12:28 a.m. UTC | #1 
On Fri, Jan 30, 2015 at 05:18:56AM +0000, Kuninori Morimoto wrote:
> 
> Hi Lars, Simon
> 
> > The DAI link format should be specified for the whole link rather than just
> > one component on the link. So move the format specification for the HDMI
> > audio link from the CPU component to the link itself.
> > 
> > Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> > in this case there is no functional difference between only specifying the
> > the format for the CPU side or for the whole link, but the later it will
> > allow us to remove support for just specifying the format for one component.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > ---
> 
> I tested these 3 patches on armadillo800eva
> 
> 	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
> 	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
> 	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
> 
> Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> 2/3 patch changed behavior for clock inversion on FSI driver.
> FSI + wm8978 on armadillo800eva worked without any issues,
> but, I don't know how much effect it has for other board.
> We used this inversion flags on each board for historical reasons (copy-paste),
> but, almost all these were not needed (except some picky board) on FSI.
> Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
> And unfortunately, Renesas don't use FSI anymore.
> So, I think keeping current FSI driver as-is is more safety for old boards.
> armadillo800eva works well with below patch (as 2/3 patch)


Thanks, I am inclined to agree that it is best to leave things unchanged.

> 
> --------------
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 75de26c..36aaeb1 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
>         .platform       = "sh_fsi2",
>         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
>         .cpu_dai = {
> -               .fmt    = SND_SOC_DAIFMT_IB_NF,
>                 .name   = "fsia-dai",
>         },
>         .codec_dai = {
> --------------
>
Kuninori Morimoto Feb. 27, 2015, 12:23 a.m. UTC | #2 
Hi Lars, Mark

> > > The DAI link format should be specified for the whole link rather than just
> > > one component on the link. So move the format specification for the HDMI
> > > audio link from the CPU component to the link itself.
> > > 
> > > Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> > > in this case there is no functional difference between only specifying the
> > > the format for the CPU side or for the whole link, but the later it will
> > > allow us to remove support for just specifying the format for one component.
> > > 
> > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > > ---
> > 
> > I tested these 3 patches on armadillo800eva
> > 
> > 	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
> > 	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
> > 	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
> > 
> > Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > 2/3 patch changed behavior for clock inversion on FSI driver.
> > FSI + wm8978 on armadillo800eva worked without any issues,
> > but, I don't know how much effect it has for other board.
> > We used this inversion flags on each board for historical reasons (copy-paste),
> > but, almost all these were not needed (except some picky board) on FSI.
> > Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
> > And unfortunately, Renesas don't use FSI anymore.
> > So, I think keeping current FSI driver as-is is more safety for old boards.
> > armadillo800eva works well with below patch (as 2/3 patch)
> 
> 
> Thanks, I am inclined to agree that it is best to leave things unchanged.
> 
> > 
> > --------------
> > diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > index 75de26c..36aaeb1 100644
> > --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> > +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
> >         .platform       = "sh_fsi2",
> >         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
> >         .cpu_dai = {
> > -               .fmt    = SND_SOC_DAIFMT_IB_NF,
> >                 .name   = "fsia-dai",
> >         },
> >         .codec_dai = {
> > --------------

As I mentioned above (and, Simon agreed), I think we can
agree if [2/3] patch was exchanged

Best regards
---
Kuninori Morimoto
Mark Brown March 22, 2015, 6:52 p.m. UTC | #3 
On Fri, Feb 27, 2015 at 12:23:26AM +0000, Kuninori Morimoto wrote:

> As I mentioned above (and, Simon agreed), I think we can
> agree if [2/3] patch was exchanged

Do you mean that patch 2/3 should be replaced with soemthing else - if
so what should it be replaced by?  I've not seen an updated version of
this series...
Kuninori Morimoto March 23, 2015, 7 a.m. UTC | #4 
Hi Mark

> > As I mentioned above (and, Simon agreed), I think we can
> > agree if [2/3] patch was exchanged
> 
> Do you mean that patch 2/3 should be replaced with soemthing else - if
> so what should it be replaced by?  I've not seen an updated version of
> this series...
> [2 Digital signature <application/pgp-signature (7bit)>]
> No public key for 24D68B725D5487D0 created at 2015-03-23T03:52:37+0900 using RSA

I guess I added my recommend patch on previous mail.
If it was not understandable, I can send full-patch-set instead of Lars.

Best regards
---
Kuninori Morimoto
Mark Brown March 23, 2015, 9:12 p.m. UTC | #5 
On Mon, Mar 23, 2015 at 07:00:11AM +0000, Kuninori Morimoto wrote:

> I guess I added my recommend patch on previous mail.
> If it was not understandable, I can send full-patch-set instead of Lars.

Ah, in a reply in the middle of the thread?  That does get hard to
follow sometimes, if there's general agreement on what should go in if
someone could do a complete resend of the code everyone agrees on that
would be helpful.
Kuninori Morimoto March 24, 2015, 12:07 a.m. UTC | #6 
Hi Mark

> > I guess I added my recommend patch on previous mail.
> > If it was not understandable, I can send full-patch-set instead of Lars.
> 
> Ah, in a reply in the middle of the thread?  That does get hard to
> follow sometimes, if there's general agreement on what should go in if
> someone could do a complete resend of the code everyone agrees on that
> would be helpful.

OK, I will send full-patch-set for it.
diff mbox

Patch

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 75de26c..36aaeb1 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1015,7 +1015,6 @@  static struct asoc_simple_card_info fsi_wm8978_info = {
        .platform       = "sh_fsi2",
        .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
        .cpu_dai = {
-               .fmt    = SND_SOC_DAIFMT_IB_NF,
                .name   = "fsia-dai",
        },
        .codec_dai = {