RE: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
> Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add > asoc_simple_card_fmt_master() > to simplify the code. > > On 09/03/2014 05:37 AM, li.xi...@freescale.com wrote: > >> Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add > asoc_simple_card_fmt_master() > ... > >> > >> This won't work. The logic for cpu node needs to be negated for codec node. > >> > > > > Yes, actually it should be. > > > > As my previous patches about this: > > > > Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx' > > mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's > > frame clock is as master/slave. > > > > So these same DAI formats should be informed to CPU and CODE DAIs at > > the same time. For the Codec driver will set the bit clock and frame > > clock as the DAI formats said, but for the CPU driver, if the the > > bit clock or frame clock is as Codec master, so it should be set CPU > > DAI device as bit clock or frame clock as slave, and vice versa. > > > > The old code will cause confusion, and we should be clear that the > > letter 'C' here mean to Codec. > > > > > > For the master format, no matter for CPU or CODEC, it always means Codec > > is master or slave for bit/frame clock, not means the local DAI device's > > bit/frame clock as master or slave. > > > > So your CPU DAI device driver should negate this locally as the existed > > Ones do. > > > > > Yes, but there is double negation in this patch. The switch-case > assignments depend on whether the bitclkmaster and framemaster > DT-node pointers are compared to a cpu-dai-node or > codec-dai-node. When your patch compares the codec-node, it does > the decisions like it was a cpu-node, which produces inverted CBM > and CFM setting. > > However, Kurinori-san's patch fixes this problem because it just > uses the daifmt generated by comparing to codec node for both cpu > and codec nodes. > > The reason why I did the comparison per node basis, was to make > the code more ready for tdm setups with multiple codecs on a same > wire. But writing code for something that is not really needed > yet is usually a bad idea, like it was this time too. > > Kurinori-san's version of the fix should be fine and it cleans up > the code quite nicely. > Yes, agree. So I just removed this patch from my patch series list. Kuninori-san will post his local patch about this later. Thanks, BRs Xiubo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/03/2014 05:37 AM, li.xi...@freescale.com wrote: Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() ... This won't work. The logic for cpu node needs to be negated for codec node. Yes, actually it should be. As my previous patches about this: Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx' mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's frame clock is as master/slave. So these same DAI formats should be informed to CPU and CODE DAIs at the same time. For the Codec driver will set the bit clock and frame clock as the DAI formats said, but for the CPU driver, if the the bit clock or frame clock is as Codec master, so it should be set CPU DAI device as bit clock or frame clock as slave, and vice versa. The old code will cause confusion, and we should be clear that the letter 'C' here mean to Codec. For the master format, no matter for CPU or CODEC, it always means Codec is master or slave for bit/frame clock, not means the local DAI device's bit/frame clock as master or slave. So your CPU DAI device driver should negate this locally as the existed Ones do. Yes, but there is double negation in this patch. The switch-case assignments depend on whether the bitclkmaster and framemaster DT-node pointers are compared to a cpu-dai-node or codec-dai-node. When your patch compares the codec-node, it does the decisions like it was a cpu-node, which produces inverted CBM and CFM setting. However, Kurinori-san's patch fixes this problem because it just uses the daifmt generated by comparing to codec node for both cpu and codec nodes. The reason why I did the comparison per node basis, was to make the code more ready for tdm setups with multiple codecs on a same wire. But writing code for something that is not really needed yet is usually a bad idea, like it was this time too. Kurinori-san's version of the fix should be fine and it cleans up the code quite nicely. Best regards, Jyri -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On Tue, 02 Sep 2014 21:13:52 -0700 (PDT) Kuninori Morimoto wrote: > OK, will do. > To avoid confusion/conflict, I will post it after Mark applied it. > Because many simple-card patches are posted in these days... Yes, I have one more awaiting, about multi-CODECs... -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On Tue, 02 Sep 2014 21:13:52 -0700 (PDT) Kuninori Morimoto kuninori.morimoto...@gmail.com wrote: OK, will do. To avoid confusion/conflict, I will post it after Mark applied it. Because many simple-card patches are posted in these days... Yes, I have one more awaiting, about multi-CODECs... -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/03/2014 05:37 AM, li.xi...@freescale.com wrote: Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() ... This won't work. The logic for cpu node needs to be negated for codec node. Yes, actually it should be. As my previous patches about this: Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx' mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's frame clock is as master/slave. So these same DAI formats should be informed to CPU and CODE DAIs at the same time. For the Codec driver will set the bit clock and frame clock as the DAI formats said, but for the CPU driver, if the the bit clock or frame clock is as Codec master, so it should be set CPU DAI device as bit clock or frame clock as slave, and vice versa. The old code will cause confusion, and we should be clear that the letter 'C' here mean to Codec. For the master format, no matter for CPU or CODEC, it always means Codec is master or slave for bit/frame clock, not means the local DAI device's bit/frame clock as master or slave. So your CPU DAI device driver should negate this locally as the existed Ones do. Yes, but there is double negation in this patch. The switch-case assignments depend on whether the bitclkmaster and framemaster DT-node pointers are compared to a cpu-dai-node or codec-dai-node. When your patch compares the codec-node, it does the decisions like it was a cpu-node, which produces inverted CBM and CFM setting. However, Kurinori-san's patch fixes this problem because it just uses the daifmt generated by comparing to codec node for both cpu and codec nodes. The reason why I did the comparison per node basis, was to make the code more ready for tdm setups with multiple codecs on a same wire. But writing code for something that is not really needed yet is usually a bad idea, like it was this time too. Kurinori-san's version of the fix should be fine and it cleans up the code quite nicely. Best regards, Jyri -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code. On 09/03/2014 05:37 AM, li.xi...@freescale.com wrote: Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() ... This won't work. The logic for cpu node needs to be negated for codec node. Yes, actually it should be. As my previous patches about this: Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx' mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's frame clock is as master/slave. So these same DAI formats should be informed to CPU and CODE DAIs at the same time. For the Codec driver will set the bit clock and frame clock as the DAI formats said, but for the CPU driver, if the the bit clock or frame clock is as Codec master, so it should be set CPU DAI device as bit clock or frame clock as slave, and vice versa. The old code will cause confusion, and we should be clear that the letter 'C' here mean to Codec. For the master format, no matter for CPU or CODEC, it always means Codec is master or slave for bit/frame clock, not means the local DAI device's bit/frame clock as master or slave. So your CPU DAI device driver should negate this locally as the existed Ones do. Yes, but there is double negation in this patch. The switch-case assignments depend on whether the bitclkmaster and framemaster DT-node pointers are compared to a cpu-dai-node or codec-dai-node. When your patch compares the codec-node, it does the decisions like it was a cpu-node, which produces inverted CBM and CFM setting. However, Kurinori-san's patch fixes this problem because it just uses the daifmt generated by comparing to codec node for both cpu and codec nodes. The reason why I did the comparison per node basis, was to make the code more ready for tdm setups with multiple codecs on a same wire. But writing code for something that is not really needed yet is usually a bad idea, like it was this time too. Kurinori-san's version of the fix should be fine and it cleans up the code quite nicely. Yes, agree. So I just removed this patch from my patch series list. Kuninori-san will post his local patch about this later. Thanks, BRs Xiubo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
> Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add > asoc_simple_card_fmt_master() to simplify the code. > > > Hi Xiubo > > > Yes your patch has fixed the bug Jyri has pointed out. > > > > So I has discard my [PATCHv2 1/4] patch. > > > > Please send your patch out to replace this one. > (snip) > > Please send it out of your local patch. > > > > Please also consider the ideas about Jyri, Jean-Francios, Varka and > > Takashi's advice as previous emails about my patch. > > OK, will do. > To avoid confusion/conflict, I will post it after Mark applied it. > Because many simple-card patches are posted in these days... > Yes, Get it. Thanks, BRs Xiubo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo > Yes your patch has fixed the bug Jyri has pointed out. > > So I has discard my [PATCHv2 1/4] patch. > > Please send your patch out to replace this one. (snip) > Please send it out of your local patch. > > Please also consider the ideas about Jyri, Jean-Francios, Varka and > Takashi's advice as previous emails about my patch. OK, will do. To avoid confusion/conflict, I will post it after Mark applied it. Because many simple-card patches are posted in these days... Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
> Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add > asoc_simple_card_fmt_master() to simplify the code. > > > Hi Xiubo > > > Yes, I think it make sense to set all fmt in one function, and will > > Be more readable. > > > > I agree with you, could you please just wait, because there has many > > Replications and good Ideas about this patch, and I will revise it. > > Then you can improve it as your patch blow. > > Thank you for your help > I don't care so much about my local patch. > You can re-use it if you want. > Of course I can do it if you want > Please send it out of your local patch. Please also consider the ideas about Jyri, Jean-Francios, Varka and Takashi's advice as previous emails about my patch. BRs Xiubo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo > Yes, I think it make sense to set all fmt in one function, and will > Be more readable. > > I agree with you, could you please just wait, because there has many > Replications and good Ideas about this patch, and I will revise it. > Then you can improve it as your patch blow. Thank you for your help I don't care so much about my local patch. You can re-use it if you want. Of course I can do it if you want Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Kuninori-san,
Yes your patch has fixed the bug Jyri has pointed out.
So I has discard my [PATCHv2 1/4] patch.
Please send your patch out to replace this one.
Thanks,
BRs
Xiubo
> -Original Message-
> From: Xiubo Li-B47053
> Sent: Wednesday, September 03, 2014 10:22 AM
> To: 'Kuninori Morimoto'
> Cc: broo...@kernel.org; pe...@perex.cz; lgirdw...@gmail.com; ti...@suse.de;
> moin...@free.fr; and...@lunn.ch; kuninori.morimoto...@renesas.com;
> jsa...@ti.com; devicet...@vger.kernel.org; alsa-de...@alsa-project.org;
> robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
> ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; linux-
> ker...@vger.kernel.org
> Subject: RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add
> asoc_simple_card_fmt_master() to simplify the code.
>
> Hi Kuninori-san,
>
> Yes, I think it make sense to set all fmt in one function, and will
> Be more readable.
>
> I agree with you, could you please just wait, because there has many
> Replications and good Ideas about this patch, and I will revise it.
> Then you can improve it as your patch blow.
>
>
> Thanks,
>
> BRs
> Xiubo
>
>
> > Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add
> > asoc_simple_card_fmt_master() to simplify the code.
> >
> >
> > Hi Xiubo
> >
> > I was very surprised about this patch
> > because the idea is same as my local patch
> > (I was planned to send it to ML :)
> >
> > I attached my local patch to sharing idea.
> >
> > > +static inline unsigned int
> > > +asoc_simple_card_fmt_master(struct device_node *np,
> > > + struct device_node *bitclkmaster,
> > > + struct device_node *framemaster)
> > > +{
> > > + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> > > + case 0x11:
> > > + return SND_SOC_DAIFMT_CBS_CFS;
> > > + case 0x10:
> > > + return SND_SOC_DAIFMT_CBS_CFM;
> > > + case 0x01:
> > > + return SND_SOC_DAIFMT_CBM_CFS;
> > > + default:
> > > + return SND_SOC_DAIFMT_CBM_CFM;
> > > + }
> > > +
> > > + /* Shouldn't be here */
> > > + return -EINVAL;
> > > +}
> >
> > I think this concept is nice,
> > but setting all fmt in this function is good for me
> > see my local patch
> >
> > --
> > From 85562eb1587e5c184e4f4e0b183bd7063aaa81b7 Mon Sep 17 00:00:00 2001
> > From: Kuninori Morimoto
> > Date: Thu, 28 Aug 2014 19:20:14 +0900
> > Subject: [PATCH] ASoC: simple-card: add asoc_simple_card_parse_daifmt()
> >
> > Current daifmt setting method in simple-card driver is
> > placed to many places, and using un-readable/confusable method.
> > This patch adds new asoc_simple_card_parse_daifmt()
> > and tidyup code.
> >
> > Signed-off-by: Kuninori Morimoto
> >
> > diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-
> card.c
> > index bea5901..c932103 100644
> > --- a/sound/soc/generic/simple-card.c
> > +++ b/sound/soc/generic/simple-card.c
> > @@ -167,6 +167,64 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
> > return 0;
> > }
> >
> > +static int asoc_simple_card_parse_daifmt(struct device_node *node,
> > +struct simple_card_data *priv,
> > +struct device_node *cpu,
> > +struct device_node *codec,
> > +char *prefix, int idx)
> > +{
> > + struct device *dev = simple_priv_to_dev(priv);
> > + struct device_node *bitclkmaster = NULL;
> > + struct device_node *framemaster = NULL;
> > + struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
> > + struct asoc_simple_dai *cpu_dai = _props->cpu_dai;
> > + struct asoc_simple_dai *codec_dai = _props->codec_dai;
> > + unsigned int daifmt;
> > +
> > + daifmt = snd_soc_of_parse_daifmt(node, prefix,
> > +, );
> > + daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
> > +
> > + if (strlen(prefix) && !bitclkmaster && !framemaster) {
> > + /*
> > +* No dai-link level and master setting was not found from
> > +* sound node level, revert back to legacy DT parsing and
> > +* take the settings from codec node.
> > +*/
> > + dev_dbg(dev, "Revert to legacy daifmt parsing\n&
RE: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
> Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add
> asoc_simple_card_fmt_master()
> to simplify the code.
>
> On 09/02/2014 12:26 PM, Xiubo Li wrote:
> > Signed-off-by: Xiubo Li
> > ---
> > sound/soc/generic/simple-card.c | 61 -
>
> > 1 file changed, 29 insertions(+), 32 deletions(-)
> >
> > diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-
> card.c
> > index 986d2c7..cad2b30 100644
> > --- a/sound/soc/generic/simple-card.c
> > +++ b/sound/soc/generic/simple-card.c
> > @@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
> > return 0;
> > }
> >
> > +static inline unsigned int
> > +asoc_simple_card_fmt_master(struct device_node *np,
> > + struct device_node *bitclkmaster,
> > + struct device_node *framemaster)
> > +{
> > + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> > + case 0x11:
> > + return SND_SOC_DAIFMT_CBS_CFS;
> > + case 0x10:
> > + return SND_SOC_DAIFMT_CBS_CFM;
> > + case 0x01:
> > + return SND_SOC_DAIFMT_CBM_CFS;
> > + default:
> > + return SND_SOC_DAIFMT_CBM_CFM;
> > + }
> > +
> > + /* Shouldn't be here */
> > + return -EINVAL;
> > +}
> > +
>
> > + fmt = asoc_simple_card_fmt_master(np, bitclkmaster, framemaster);
> > + dai_props->cpu_dai.fmt = daifmt | fmt;
> ...
> > + fmt = asoc_simple_card_fmt_master(np, bitclkmaster,
> > + framemaster);
> > + dai_props->codec_dai.fmt = daifmt | fmt;
>
> This won't work. The logic for cpu node needs to be negated for codec node.
>
Yes, actually it should be.
As my previous patches about this:
Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.
So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.
The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.
For the master format, no matter for CPU or CODEC, it always means Codec
is master or slave for bit/frame clock, not means the local DAI device's
bit/frame clock as master or slave.
So your CPU DAI device driver should negate this locally as the existed
Ones do.
Thanks,
BRs
Xiubo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Kuninori-san,
Yes, I think it make sense to set all fmt in one function, and will
Be more readable.
I agree with you, could you please just wait, because there has many
Replications and good Ideas about this patch, and I will revise it.
Then you can improve it as your patch blow.
Thanks,
BRs
Xiubo
> Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add
> asoc_simple_card_fmt_master() to simplify the code.
>
>
> Hi Xiubo
>
> I was very surprised about this patch
> because the idea is same as my local patch
> (I was planned to send it to ML :)
>
> I attached my local patch to sharing idea.
>
> > +static inline unsigned int
> > +asoc_simple_card_fmt_master(struct device_node *np,
> > + struct device_node *bitclkmaster,
> > + struct device_node *framemaster)
> > +{
> > + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> > + case 0x11:
> > + return SND_SOC_DAIFMT_CBS_CFS;
> > + case 0x10:
> > + return SND_SOC_DAIFMT_CBS_CFM;
> > + case 0x01:
> > + return SND_SOC_DAIFMT_CBM_CFS;
> > + default:
> > + return SND_SOC_DAIFMT_CBM_CFM;
> > + }
> > +
> > + /* Shouldn't be here */
> > + return -EINVAL;
> > +}
>
> I think this concept is nice,
> but setting all fmt in this function is good for me
> see my local patch
>
> --
> From 85562eb1587e5c184e4f4e0b183bd7063aaa81b7 Mon Sep 17 00:00:00 2001
> From: Kuninori Morimoto
> Date: Thu, 28 Aug 2014 19:20:14 +0900
> Subject: [PATCH] ASoC: simple-card: add asoc_simple_card_parse_daifmt()
>
> Current daifmt setting method in simple-card driver is
> placed to many places, and using un-readable/confusable method.
> This patch adds new asoc_simple_card_parse_daifmt()
> and tidyup code.
>
> Signed-off-by: Kuninori Morimoto
>
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index bea5901..c932103 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -167,6 +167,64 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
> return 0;
> }
>
> +static int asoc_simple_card_parse_daifmt(struct device_node *node,
> + struct simple_card_data *priv,
> + struct device_node *cpu,
> + struct device_node *codec,
> + char *prefix, int idx)
> +{
> + struct device *dev = simple_priv_to_dev(priv);
> + struct device_node *bitclkmaster = NULL;
> + struct device_node *framemaster = NULL;
> + struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
> + struct asoc_simple_dai *cpu_dai = _props->cpu_dai;
> + struct asoc_simple_dai *codec_dai = _props->codec_dai;
> + unsigned int daifmt;
> +
> + daifmt = snd_soc_of_parse_daifmt(node, prefix,
> + , );
> + daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
> +
> + if (strlen(prefix) && !bitclkmaster && !framemaster) {
> + /*
> + * No dai-link level and master setting was not found from
> + * sound node level, revert back to legacy DT parsing and
> + * take the settings from codec node.
> + */
> + dev_dbg(dev, "Revert to legacy daifmt parsing\n");
> +
> + cpu_dai->fmt = codec_dai->fmt =
> + snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
> + (daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
> + } else {
> +
> + switch (((codec == bitclkmaster) << 4) | (codec == framemaster))
> {
> + case 0x11:
> + daifmt |= SND_SOC_DAIFMT_CBM_CFM;
> + break;
> + case 0x10:
> + daifmt |= SND_SOC_DAIFMT_CBM_CFS;
> + break;
> + case 0x01:
> + daifmt |= SND_SOC_DAIFMT_CBS_CFM;
> + break;
> + default:
> + daifmt |= SND_SOC_DAIFMT_CBS_CFS;
> + break;
> + }
> +
> + cpu_dai->fmt = daifmt;
> + codec_dai->fmt = daifmt;
> + }
> +
> + if (bitclkmaster)
> + of_node_put(bitclkmaster);
> + if (framemaster)
> + of_node_put(framemaster);
> +
> + return 0;
> +}
> +
> static int asoc_simple_card_dai_link_of(struct device_node *node,
>
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo
I was very surprised about this patch
because the idea is same as my local patch
(I was planned to send it to ML :)
I attached my local patch to sharing idea.
> +static inline unsigned int
> +asoc_simple_card_fmt_master(struct device_node *np,
> + struct device_node *bitclkmaster,
> + struct device_node *framemaster)
> +{
> + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> + case 0x11:
> + return SND_SOC_DAIFMT_CBS_CFS;
> + case 0x10:
> + return SND_SOC_DAIFMT_CBS_CFM;
> + case 0x01:
> + return SND_SOC_DAIFMT_CBM_CFS;
> + default:
> + return SND_SOC_DAIFMT_CBM_CFM;
> + }
> +
> + /* Shouldn't be here */
> + return -EINVAL;
> +}
I think this concept is nice,
but setting all fmt in this function is good for me
see my local patch
--
>From 85562eb1587e5c184e4f4e0b183bd7063aaa81b7 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto
Date: Thu, 28 Aug 2014 19:20:14 +0900
Subject: [PATCH] ASoC: simple-card: add asoc_simple_card_parse_daifmt()
Current daifmt setting method in simple-card driver is
placed to many places, and using un-readable/confusable method.
This patch adds new asoc_simple_card_parse_daifmt()
and tidyup code.
Signed-off-by: Kuninori Morimoto
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index bea5901..c932103 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -167,6 +167,64 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static int asoc_simple_card_parse_daifmt(struct device_node *node,
+struct simple_card_data *priv,
+struct device_node *cpu,
+struct device_node *codec,
+char *prefix, int idx)
+{
+ struct device *dev = simple_priv_to_dev(priv);
+ struct device_node *bitclkmaster = NULL;
+ struct device_node *framemaster = NULL;
+ struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
+ struct asoc_simple_dai *cpu_dai = _props->cpu_dai;
+ struct asoc_simple_dai *codec_dai = _props->codec_dai;
+ unsigned int daifmt;
+
+ daifmt = snd_soc_of_parse_daifmt(node, prefix,
+, );
+ daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+
+ if (strlen(prefix) && !bitclkmaster && !framemaster) {
+ /*
+* No dai-link level and master setting was not found from
+* sound node level, revert back to legacy DT parsing and
+* take the settings from codec node.
+*/
+ dev_dbg(dev, "Revert to legacy daifmt parsing\n");
+
+ cpu_dai->fmt = codec_dai->fmt =
+ snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+ (daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
+ } else {
+
+ switch (((codec == bitclkmaster) << 4) | (codec ==
framemaster)) {
+ case 0x11:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFM;
+ break;
+ case 0x10:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFS;
+ break;
+ case 0x01:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFM;
+ break;
+ default:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFS;
+ break;
+ }
+
+ cpu_dai->fmt = daifmt;
+ codec_dai->fmt = daifmt;
+ }
+
+ if (bitclkmaster)
+ of_node_put(bitclkmaster);
+ if (framemaster)
+ of_node_put(framemaster);
+
+ return 0;
+}
+
static int asoc_simple_card_dai_link_of(struct device_node *node,
struct simple_card_data *priv,
int idx,
@@ -175,10 +233,8 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
struct device *dev = simple_priv_to_dev(priv);
struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
- struct device_node *np = NULL;
- struct device_node *bitclkmaster = NULL;
- struct device_node *framemaster = NULL;
- unsigned int daifmt;
+ struct device_node *cpu = NULL;
+ struct device_node *codec = NULL;
char *name;
char prop[128];
char *prefix = "";
@@ -187,82 +243,35 @@ static int asoc_simple_card_dai_link_of(struct
device_node *node,
if (is_top_level_node)
prefix = "simple-audio-card,";
- daifmt = snd_soc_of_parse_daifmt(node, prefix,
-, );
- daifmt &=
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 02:09 PM, Jean-Francois Moine wrote:
On Tue, 02 Sep 2014 16:12:40 +0530
Varka Bhadram wrote:
+ switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
I don't see which macros: the values are just 2 booleans.
I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
names to those...?
#define TRUE_TRUE 0x11
#define TRUE_FALSE 0x10
#define FALSE_TRUE 0x01
or
case ((TRUE << 4) | TRUE:
...
case ((TRUE << 4) | FALSE:
...
case ((FALSE << 4) | TRUE:
...
I would vote for this. Even over the options suggested by Takashi, but
then again this really a matter of taste.
The fact that frame and bit-clock master boolean values are bundled into
a single "enum" field, instead of two dedicated bits, makes all options
bit inconvenient.
Best regards,
Jyri
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 12:26 PM, Xiubo Li wrote:
Signed-off-by: Xiubo Li
---
sound/soc/generic/simple-card.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 986d2c7..cad2b30 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
+
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster, framemaster);
+ dai_props->cpu_dai.fmt = daifmt | fmt;
...
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster,
+ framemaster);
+ dai_props->codec_dai.fmt = daifmt | fmt;
This won't work. The logic for cpu node needs to be negated for codec node.
Best regards,
Jyri
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On Tue, 02 Sep 2014 16:12:40 +0530
Varka Bhadram wrote:
> >>> + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> >>> + case 0x11:
> >>> + return SND_SOC_DAIFMT_CBS_CFS;
> >>> + case 0x10:
> >>> + return SND_SOC_DAIFMT_CBS_CFM;
> >>> + case 0x01:
> >>> + return SND_SOC_DAIFMT_CBM_CFS;
> >>> + default:
> >>> + return SND_SOC_DAIFMT_CBM_CFM;
> >>> + }
> >>> +
> >>> + /* Shouldn't be here */
> >>> + return -EINVAL;
> >>> +}
> >> It will be nice if we declare the switch case numbers as macros (specific
> >> name)...
> > I don't see which macros: the values are just 2 booleans.
> >
> I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
> names to those...?
#define TRUE_TRUE 0x11
#define TRUE_FALSE 0x10
#define FALSE_TRUE 0x01
or
case ((TRUE << 4) | TRUE:
...
case ((TRUE << 4) | FALSE:
...
case ((FALSE << 4) | TRUE:
...
??
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
At Tue, 02 Sep 2014 16:12:40 +0530,
Varka Bhadram wrote:
>
> On 09/02/2014 04:08 PM, Jean-Francois Moine wrote:
> > On Tue, 02 Sep 2014 15:51:41 +0530
> > Varka Bhadram wrote:
> >
> >>> + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> >>> + case 0x11:
> >>> + return SND_SOC_DAIFMT_CBS_CFS;
> >>> + case 0x10:
> >>> + return SND_SOC_DAIFMT_CBS_CFM;
> >>> + case 0x01:
> >>> + return SND_SOC_DAIFMT_CBM_CFS;
> >>> + default:
> >>> + return SND_SOC_DAIFMT_CBM_CFM;
> >>> + }
> >>> +
> >>> + /* Shouldn't be here */
> >>> + return -EINVAL;
> >>> +}
> >> It will be nice if we declare the switch case numbers as macros (specific
> >> name)...
> > I don't see which macros: the values are just 2 booleans.
> >
> I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
> names to those...?
The whole switch block is too hackish, makes unnecessarily complex.
It can be more strightforwardly like:
if (np == bitclkmaster)
return np == framemater ?
SND_SOC_DAIFMT_CBS_CFS : SND_SOC_DAIFMT_CBS_CFM;
else
return np == framemaster ?
SND_SOC_DAIFMT_CBM_CFS : SND_SOC_DAIFMT_CBM_CFM;
Or, if you love efficiency and complexity, something like:
#define SND_SOC_DAIFMT(_np, _clk, _frame) \
_np) != (_clk)) | (((_np) != (_frame)) << 1) << 12) + (1 << 12)
Then
return SND_SOC_DAIFMT(np, blkclkmaster, framemaster);
Takashi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 04:08 PM, Jean-Francois Moine wrote:
On Tue, 02 Sep 2014 15:51:41 +0530
Varka Bhadram wrote:
+ switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
I don't see which macros: the values are just 2 booleans.
I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
names to those...?
--
Regards,
Varka Bhadram.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On Tue, 02 Sep 2014 15:51:41 +0530
Varka Bhadram wrote:
> > + switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
> > + case 0x11:
> > + return SND_SOC_DAIFMT_CBS_CFS;
> > + case 0x10:
> > + return SND_SOC_DAIFMT_CBS_CFM;
> > + case 0x01:
> > + return SND_SOC_DAIFMT_CBM_CFS;
> > + default:
> > + return SND_SOC_DAIFMT_CBM_CFM;
> > + }
> > +
> > + /* Shouldn't be here */
> > + return -EINVAL;
> > +}
>
> It will be nice if we declare the switch case numbers as macros (specific
> name)...
I don't see which macros: the values are just 2 booleans.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 02:56 PM, Xiubo Li wrote:
Signed-off-by: Xiubo Li
---
sound/soc/generic/simple-card.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 986d2c7..cad2b30 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
--
Regards,
Varka Bhadram.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Signed-off-by: Xiubo Li
---
sound/soc/generic/simple-card.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 986d2c7..cad2b30 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
+
static int asoc_simple_card_dai_link_of(struct device_node *node,
struct device *dev,
struct snd_soc_dai_link *dai_link,
@@ -172,7 +192,7 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
struct device_node *np = NULL;
struct device_node *bitclkmaster = NULL;
struct device_node *framemaster = NULL;
- unsigned int daifmt;
+ unsigned int daifmt, fmt;
char *name;
char prop[128];
char *prefix = "";
@@ -185,6 +205,7 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
, );
daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+ /* Parse CPU DAI sub-node */
snprintf(prop, sizeof(prop), "%scpu", prefix);
np = of_get_child_by_name(node, prop);
if (!np) {
@@ -199,23 +220,11 @@ static int asoc_simple_card_dai_link_of(struct
device_node *node,
if (ret < 0)
goto dai_link_of_err;
- dai_props->cpu_dai.fmt = daifmt;
- switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
- case 0x11:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
- break;
- case 0x10:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
- break;
- case 0x01:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
- break;
- default:
- dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
- break;
- }
-
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster, framemaster);
+ dai_props->cpu_dai.fmt = daifmt | fmt;
of_node_put(np);
+
+ /* Parse CODEC DAI sub-node */
snprintf(prop, sizeof(prop), "%scodec", prefix);
np = of_get_child_by_name(node, prop);
if (!np) {
@@ -240,21 +249,9 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
snd_soc_of_parse_daifmt(np, NULL, NULL, NULL) |
(daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
} else {
- dai_props->codec_dai.fmt = daifmt;
- switch (((np == bitclkmaster) << 4) | (np == framemaster)) {
- case 0x11:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
- break;
- case 0x10:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
- break;
- case 0x01:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
- break;
- default:
- dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
- break;
- }
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster,
+ framemaster);
+ dai_props->codec_dai.fmt = daifmt | fmt;
}
if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) {
--
1.8.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Signed-off-by: Xiubo Li li.xi...@freescale.com
---
sound/soc/generic/simple-card.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 986d2c7..cad2b30 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
+
static int asoc_simple_card_dai_link_of(struct device_node *node,
struct device *dev,
struct snd_soc_dai_link *dai_link,
@@ -172,7 +192,7 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
struct device_node *np = NULL;
struct device_node *bitclkmaster = NULL;
struct device_node *framemaster = NULL;
- unsigned int daifmt;
+ unsigned int daifmt, fmt;
char *name;
char prop[128];
char *prefix = ;
@@ -185,6 +205,7 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
bitclkmaster, framemaster);
daifmt = ~SND_SOC_DAIFMT_MASTER_MASK;
+ /* Parse CPU DAI sub-node */
snprintf(prop, sizeof(prop), %scpu, prefix);
np = of_get_child_by_name(node, prop);
if (!np) {
@@ -199,23 +220,11 @@ static int asoc_simple_card_dai_link_of(struct
device_node *node,
if (ret 0)
goto dai_link_of_err;
- dai_props-cpu_dai.fmt = daifmt;
- switch (((np == bitclkmaster) 4) | (np == framemaster)) {
- case 0x11:
- dai_props-cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
- break;
- case 0x10:
- dai_props-cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
- break;
- case 0x01:
- dai_props-cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
- break;
- default:
- dai_props-cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
- break;
- }
-
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster, framemaster);
+ dai_props-cpu_dai.fmt = daifmt | fmt;
of_node_put(np);
+
+ /* Parse CODEC DAI sub-node */
snprintf(prop, sizeof(prop), %scodec, prefix);
np = of_get_child_by_name(node, prop);
if (!np) {
@@ -240,21 +249,9 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
snd_soc_of_parse_daifmt(np, NULL, NULL, NULL) |
(daifmt ~SND_SOC_DAIFMT_CLOCK_MASK);
} else {
- dai_props-codec_dai.fmt = daifmt;
- switch (((np == bitclkmaster) 4) | (np == framemaster)) {
- case 0x11:
- dai_props-codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
- break;
- case 0x10:
- dai_props-codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
- break;
- case 0x01:
- dai_props-codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
- break;
- default:
- dai_props-codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
- break;
- }
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster,
+ framemaster);
+ dai_props-codec_dai.fmt = daifmt | fmt;
}
if (!dai_link-cpu_dai_name || !dai_link-codec_dai_name) {
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 02:56 PM, Xiubo Li wrote:
Signed-off-by: Xiubo Li li.xi...@freescale.com
---
sound/soc/generic/simple-card.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 986d2c7..cad2b30 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
--
Regards,
Varka Bhadram.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On Tue, 02 Sep 2014 15:51:41 +0530
Varka Bhadram varkabhad...@gmail.com wrote:
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
I don't see which macros: the values are just 2 booleans.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 04:08 PM, Jean-Francois Moine wrote:
On Tue, 02 Sep 2014 15:51:41 +0530
Varka Bhadram varkabhad...@gmail.com wrote:
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
I don't see which macros: the values are just 2 booleans.
I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
names to those...?
--
Regards,
Varka Bhadram.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
At Tue, 02 Sep 2014 16:12:40 +0530,
Varka Bhadram wrote:
On 09/02/2014 04:08 PM, Jean-Francois Moine wrote:
On Tue, 02 Sep 2014 15:51:41 +0530
Varka Bhadram varkabhad...@gmail.com wrote:
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
I don't see which macros: the values are just 2 booleans.
I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
names to those...?
The whole switch block is too hackish, makes unnecessarily complex.
It can be more strightforwardly like:
if (np == bitclkmaster)
return np == framemater ?
SND_SOC_DAIFMT_CBS_CFS : SND_SOC_DAIFMT_CBS_CFM;
else
return np == framemaster ?
SND_SOC_DAIFMT_CBM_CFS : SND_SOC_DAIFMT_CBM_CFM;
Or, if you love efficiency and complexity, something like:
#define SND_SOC_DAIFMT(_np, _clk, _frame) \
_np) != (_clk)) | (((_np) != (_frame)) 1) 12) + (1 12)
Then
return SND_SOC_DAIFMT(np, blkclkmaster, framemaster);
Takashi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On Tue, 02 Sep 2014 16:12:40 +0530
Varka Bhadram varkabhad...@gmail.com wrote:
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
I don't see which macros: the values are just 2 booleans.
I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
names to those...?
#define TRUE_TRUE 0x11
#define TRUE_FALSE 0x10
#define FALSE_TRUE 0x01
or
case ((TRUE 4) | TRUE:
...
case ((TRUE 4) | FALSE:
...
case ((FALSE 4) | TRUE:
...
??
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 12:26 PM, Xiubo Li wrote:
Signed-off-by: Xiubo Li li.xi...@freescale.com
---
sound/soc/generic/simple-card.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 986d2c7..cad2b30 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
+
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster, framemaster);
+ dai_props-cpu_dai.fmt = daifmt | fmt;
...
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster,
+ framemaster);
+ dai_props-codec_dai.fmt = daifmt | fmt;
This won't work. The logic for cpu node needs to be negated for codec node.
Best regards,
Jyri
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
On 09/02/2014 02:09 PM, Jean-Francois Moine wrote:
On Tue, 02 Sep 2014 16:12:40 +0530
Varka Bhadram varkabhad...@gmail.com wrote:
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
It will be nice if we declare the switch case numbers as macros (specific
name)...
I don't see which macros: the values are just 2 booleans.
I am talking about 0x11, 0x10, 0x01 values.. We can give any understandable
names to those...?
#define TRUE_TRUE 0x11
#define TRUE_FALSE 0x10
#define FALSE_TRUE 0x01
or
case ((TRUE 4) | TRUE:
...
case ((TRUE 4) | FALSE:
...
case ((FALSE 4) | TRUE:
...
I would vote for this. Even over the options suggested by Takashi, but
then again this really a matter of taste.
The fact that frame and bit-clock master boolean values are bundled into
a single enum field, instead of two dedicated bits, makes all options
bit inconvenient.
Best regards,
Jyri
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo
I was very surprised about this patch
because the idea is same as my local patch
(I was planned to send it to ML :)
I attached my local patch to sharing idea.
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
I think this concept is nice,
but setting all fmt in this function is good for me
see my local patch
--
From 85562eb1587e5c184e4f4e0b183bd7063aaa81b7 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto...@renesas.com
Date: Thu, 28 Aug 2014 19:20:14 +0900
Subject: [PATCH] ASoC: simple-card: add asoc_simple_card_parse_daifmt()
Current daifmt setting method in simple-card driver is
placed to many places, and using un-readable/confusable method.
This patch adds new asoc_simple_card_parse_daifmt()
and tidyup code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto...@renesas.com
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index bea5901..c932103 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -167,6 +167,64 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static int asoc_simple_card_parse_daifmt(struct device_node *node,
+struct simple_card_data *priv,
+struct device_node *cpu,
+struct device_node *codec,
+char *prefix, int idx)
+{
+ struct device *dev = simple_priv_to_dev(priv);
+ struct device_node *bitclkmaster = NULL;
+ struct device_node *framemaster = NULL;
+ struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
+ struct asoc_simple_dai *cpu_dai = dai_props-cpu_dai;
+ struct asoc_simple_dai *codec_dai = dai_props-codec_dai;
+ unsigned int daifmt;
+
+ daifmt = snd_soc_of_parse_daifmt(node, prefix,
+bitclkmaster, framemaster);
+ daifmt = ~SND_SOC_DAIFMT_MASTER_MASK;
+
+ if (strlen(prefix) !bitclkmaster !framemaster) {
+ /*
+* No dai-link level and master setting was not found from
+* sound node level, revert back to legacy DT parsing and
+* take the settings from codec node.
+*/
+ dev_dbg(dev, Revert to legacy daifmt parsing\n);
+
+ cpu_dai-fmt = codec_dai-fmt =
+ snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+ (daifmt ~SND_SOC_DAIFMT_CLOCK_MASK);
+ } else {
+
+ switch (((codec == bitclkmaster) 4) | (codec ==
framemaster)) {
+ case 0x11:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFM;
+ break;
+ case 0x10:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFS;
+ break;
+ case 0x01:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFM;
+ break;
+ default:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFS;
+ break;
+ }
+
+ cpu_dai-fmt = daifmt;
+ codec_dai-fmt = daifmt;
+ }
+
+ if (bitclkmaster)
+ of_node_put(bitclkmaster);
+ if (framemaster)
+ of_node_put(framemaster);
+
+ return 0;
+}
+
static int asoc_simple_card_dai_link_of(struct device_node *node,
struct simple_card_data *priv,
int idx,
@@ -175,10 +233,8 @@ static int asoc_simple_card_dai_link_of(struct device_node
*node,
struct device *dev = simple_priv_to_dev(priv);
struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
- struct device_node *np = NULL;
- struct device_node *bitclkmaster = NULL;
- struct device_node *framemaster = NULL;
- unsigned int daifmt;
+ struct device_node *cpu = NULL;
+ struct device_node *codec = NULL;
char *name;
char prop[128];
char *prefix = ;
@@ -187,82 +243,35 @@ static int asoc_simple_card_dai_link_of(struct
device_node *node,
if (is_top_level_node)
prefix = simple-audio-card,;
- daifmt = snd_soc_of_parse_daifmt(node, prefix,
-
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Kuninori-san,
Yes, I think it make sense to set all fmt in one function, and will
Be more readable.
I agree with you, could you please just wait, because there has many
Replications and good Ideas about this patch, and I will revise it.
Then you can improve it as your patch blow.
Thanks,
BRs
Xiubo
Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add
asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo
I was very surprised about this patch
because the idea is same as my local patch
(I was planned to send it to ML :)
I attached my local patch to sharing idea.
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
I think this concept is nice,
but setting all fmt in this function is good for me
see my local patch
--
From 85562eb1587e5c184e4f4e0b183bd7063aaa81b7 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto...@renesas.com
Date: Thu, 28 Aug 2014 19:20:14 +0900
Subject: [PATCH] ASoC: simple-card: add asoc_simple_card_parse_daifmt()
Current daifmt setting method in simple-card driver is
placed to many places, and using un-readable/confusable method.
This patch adds new asoc_simple_card_parse_daifmt()
and tidyup code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto...@renesas.com
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index bea5901..c932103 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -167,6 +167,64 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static int asoc_simple_card_parse_daifmt(struct device_node *node,
+ struct simple_card_data *priv,
+ struct device_node *cpu,
+ struct device_node *codec,
+ char *prefix, int idx)
+{
+ struct device *dev = simple_priv_to_dev(priv);
+ struct device_node *bitclkmaster = NULL;
+ struct device_node *framemaster = NULL;
+ struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
+ struct asoc_simple_dai *cpu_dai = dai_props-cpu_dai;
+ struct asoc_simple_dai *codec_dai = dai_props-codec_dai;
+ unsigned int daifmt;
+
+ daifmt = snd_soc_of_parse_daifmt(node, prefix,
+ bitclkmaster, framemaster);
+ daifmt = ~SND_SOC_DAIFMT_MASTER_MASK;
+
+ if (strlen(prefix) !bitclkmaster !framemaster) {
+ /*
+ * No dai-link level and master setting was not found from
+ * sound node level, revert back to legacy DT parsing and
+ * take the settings from codec node.
+ */
+ dev_dbg(dev, Revert to legacy daifmt parsing\n);
+
+ cpu_dai-fmt = codec_dai-fmt =
+ snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+ (daifmt ~SND_SOC_DAIFMT_CLOCK_MASK);
+ } else {
+
+ switch (((codec == bitclkmaster) 4) | (codec == framemaster))
{
+ case 0x11:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFM;
+ break;
+ case 0x10:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFS;
+ break;
+ case 0x01:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFM;
+ break;
+ default:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFS;
+ break;
+ }
+
+ cpu_dai-fmt = daifmt;
+ codec_dai-fmt = daifmt;
+ }
+
+ if (bitclkmaster)
+ of_node_put(bitclkmaster);
+ if (framemaster)
+ of_node_put(framemaster);
+
+ return 0;
+}
+
static int asoc_simple_card_dai_link_of(struct device_node *node,
struct simple_card_data *priv,
int idx,
@@ -175,10 +233,8 @@ static int asoc_simple_card_dai_link_of(struct
device_node *node,
struct device *dev = simple_priv_to_dev(priv);
struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
- struct device_node *np = NULL;
- struct device_node *bitclkmaster = NULL;
- struct device_node *framemaster = NULL;
- unsigned int
RE: [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Subject: Re: [PATCHv2 1/4] ASoC: simple-card: add
asoc_simple_card_fmt_master()
to simplify the code.
On 09/02/2014 12:26 PM, Xiubo Li wrote:
Signed-off-by: Xiubo Li li.xi...@freescale.com
---
sound/soc/generic/simple-card.c | 61 -
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-
card.c
index 986d2c7..cad2b30 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -163,6 +163,26 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
+
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster, framemaster);
+ dai_props-cpu_dai.fmt = daifmt | fmt;
...
+ fmt = asoc_simple_card_fmt_master(np, bitclkmaster,
+ framemaster);
+ dai_props-codec_dai.fmt = daifmt | fmt;
This won't work. The logic for cpu node needs to be negated for codec node.
Yes, actually it should be.
As my previous patches about this:
Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.
So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.
The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.
For the master format, no matter for CPU or CODEC, it always means Codec
is master or slave for bit/frame clock, not means the local DAI device's
bit/frame clock as master or slave.
So your CPU DAI device driver should negate this locally as the existed
Ones do.
Thanks,
BRs
Xiubo
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Kuninori-san,
Yes your patch has fixed the bug Jyri has pointed out.
So I has discard my [PATCHv2 1/4] patch.
Please send your patch out to replace this one.
Thanks,
BRs
Xiubo
-Original Message-
From: Xiubo Li-B47053
Sent: Wednesday, September 03, 2014 10:22 AM
To: 'Kuninori Morimoto'
Cc: broo...@kernel.org; pe...@perex.cz; lgirdw...@gmail.com; ti...@suse.de;
moin...@free.fr; and...@lunn.ch; kuninori.morimoto...@renesas.com;
jsa...@ti.com; devicet...@vger.kernel.org; alsa-de...@alsa-project.org;
robh...@kernel.org; pawel.m...@arm.com; mark.rutl...@arm.com;
ijc+devicet...@hellion.org.uk; ga...@codeaurora.org; linux-
ker...@vger.kernel.org
Subject: RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add
asoc_simple_card_fmt_master() to simplify the code.
Hi Kuninori-san,
Yes, I think it make sense to set all fmt in one function, and will
Be more readable.
I agree with you, could you please just wait, because there has many
Replications and good Ideas about this patch, and I will revise it.
Then you can improve it as your patch blow.
Thanks,
BRs
Xiubo
Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add
asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo
I was very surprised about this patch
because the idea is same as my local patch
(I was planned to send it to ML :)
I attached my local patch to sharing idea.
+static inline unsigned int
+asoc_simple_card_fmt_master(struct device_node *np,
+ struct device_node *bitclkmaster,
+ struct device_node *framemaster)
+{
+ switch (((np == bitclkmaster) 4) | (np == framemaster)) {
+ case 0x11:
+ return SND_SOC_DAIFMT_CBS_CFS;
+ case 0x10:
+ return SND_SOC_DAIFMT_CBS_CFM;
+ case 0x01:
+ return SND_SOC_DAIFMT_CBM_CFS;
+ default:
+ return SND_SOC_DAIFMT_CBM_CFM;
+ }
+
+ /* Shouldn't be here */
+ return -EINVAL;
+}
I think this concept is nice,
but setting all fmt in this function is good for me
see my local patch
--
From 85562eb1587e5c184e4f4e0b183bd7063aaa81b7 Mon Sep 17 00:00:00 2001
From: Kuninori Morimoto kuninori.morimoto...@renesas.com
Date: Thu, 28 Aug 2014 19:20:14 +0900
Subject: [PATCH] ASoC: simple-card: add asoc_simple_card_parse_daifmt()
Current daifmt setting method in simple-card driver is
placed to many places, and using un-readable/confusable method.
This patch adds new asoc_simple_card_parse_daifmt()
and tidyup code.
Signed-off-by: Kuninori Morimoto kuninori.morimoto...@renesas.com
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-
card.c
index bea5901..c932103 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -167,6 +167,64 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
return 0;
}
+static int asoc_simple_card_parse_daifmt(struct device_node *node,
+struct simple_card_data *priv,
+struct device_node *cpu,
+struct device_node *codec,
+char *prefix, int idx)
+{
+ struct device *dev = simple_priv_to_dev(priv);
+ struct device_node *bitclkmaster = NULL;
+ struct device_node *framemaster = NULL;
+ struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
+ struct asoc_simple_dai *cpu_dai = dai_props-cpu_dai;
+ struct asoc_simple_dai *codec_dai = dai_props-codec_dai;
+ unsigned int daifmt;
+
+ daifmt = snd_soc_of_parse_daifmt(node, prefix,
+bitclkmaster, framemaster);
+ daifmt = ~SND_SOC_DAIFMT_MASTER_MASK;
+
+ if (strlen(prefix) !bitclkmaster !framemaster) {
+ /*
+* No dai-link level and master setting was not found from
+* sound node level, revert back to legacy DT parsing and
+* take the settings from codec node.
+*/
+ dev_dbg(dev, Revert to legacy daifmt parsing\n);
+
+ cpu_dai-fmt = codec_dai-fmt =
+ snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+ (daifmt ~SND_SOC_DAIFMT_CLOCK_MASK);
+ } else {
+
+ switch (((codec == bitclkmaster) 4) | (codec == framemaster))
{
+ case 0x11:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFM;
+ break;
+ case 0x10:
+ daifmt |= SND_SOC_DAIFMT_CBM_CFS;
+ break;
+ case 0x01:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFM;
+ break;
+ default:
+ daifmt |= SND_SOC_DAIFMT_CBS_CFS;
+ break;
+ }
+
+ cpu_dai-fmt = daifmt;
+ codec_dai-fmt = daifmt
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo Yes, I think it make sense to set all fmt in one function, and will Be more readable. I agree with you, could you please just wait, because there has many Replications and good Ideas about this patch, and I will revise it. Then you can improve it as your patch blow. Thank you for your help I don't care so much about my local patch. You can re-use it if you want. Of course I can do it if you want Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code. Hi Xiubo Yes, I think it make sense to set all fmt in one function, and will Be more readable. I agree with you, could you please just wait, because there has many Replications and good Ideas about this patch, and I will revise it. Then you can improve it as your patch blow. Thank you for your help I don't care so much about my local patch. You can re-use it if you want. Of course I can do it if you want Please send it out of your local patch. Please also consider the ideas about Jyri, Jean-Francios, Varka and Takashi's advice as previous emails about my patch. BRs Xiubo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Hi Xiubo Yes your patch has fixed the bug Jyri has pointed out. So I has discard my [PATCHv2 1/4] patch. Please send your patch out to replace this one. (snip) Please send it out of your local patch. Please also consider the ideas about Jyri, Jean-Francios, Varka and Takashi's advice as previous emails about my patch. OK, will do. To avoid confusion/conflict, I will post it after Mark applied it. Because many simple-card patches are posted in these days... Best regards --- Kuninori Morimoto -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code.
Subject: Re: [alsa-devel] [PATCHv2 1/4] ASoC: simple-card: add asoc_simple_card_fmt_master() to simplify the code. Hi Xiubo Yes your patch has fixed the bug Jyri has pointed out. So I has discard my [PATCHv2 1/4] patch. Please send your patch out to replace this one. (snip) Please send it out of your local patch. Please also consider the ideas about Jyri, Jean-Francios, Varka and Takashi's advice as previous emails about my patch. OK, will do. To avoid confusion/conflict, I will post it after Mark applied it. Because many simple-card patches are posted in these days... Yes, Get it. Thanks, BRs Xiubo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
