All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Henriksson <andreas@fatal.se>
To: Fabio Estevam <festevam@gmail.com>
Cc: "Shengjiu Wang" <shengjiu.wang@nxp.com>,
	"Nicolin Chen" <nicoleotsuka@gmail.com>,
	"Xiubo Li" <Xiubo.Lee@gmail.com>,
	"Shengjiu Wang" <shengjiu.wang@gmail.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Takashi Iwai" <tiwai@suse.com>,
	Linux-ALSA <alsa-devel@alsa-project.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"Hans Söderlund" <hans.soderlund@realbit.se>
Subject: Re: [PATCH] ASoC: fsl_sai: Enable MCTL_MCLK_EN bit for master mode
Date: Thu, 6 Jul 2023 10:47:06 +0200	[thread overview]
Message-ID: <20230706084706.bzwsbi3zisx5m5rl@fatal.se> (raw)
In-Reply-To: <CAOMZO5DtpoH0dLDX3=Sv4UUpX_=66VEZPsJUWQNnYviApfMLKQ@mail.gmail.com>

Hello Shengjiu, Fabio,

On Thu, May 19, 2022 at 10:23:06AM -0300, Fabio Estevam wrote:
> Hi Shengjiu,
> 
> On Thu, May 19, 2022 at 9:49 AM Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> 
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index fa950dde5310..dae16a14f177 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -437,6 +437,12 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
> >                                    FSL_SAI_CR2_DIV_MASK | FSL_SAI_CR2_BYP,
> >                                    savediv / 2 - 1);
> >
> > +       if (sai->soc_data->max_register >= FSL_SAI_MCTL) {
> 
> Isn't it a bit fragile to take this decision based on the number of
> SAI registers in the SoC?
> 
> What about adding a specific field in soc_data for such a purpose?

We've been working on an i.MX8MP where MCLK needs to be input and found
that this enables the MCLK as output despite not having set the
`fsl,sai-mclk-direction-output;` devicetree property in our DT.
Reverting the patch fixes the issues for us.

I have to say that the code comment made me a bit confused, but once
I found the commit message I understood why this code existed.
If this is really i.MX8MM specific maybe mention that in the code
comment and please make the code actually only trigger on i.MX8MM.
It seems to me like these all fulfill the current criteria:
imx7ulp, imx8mq, imx8mm, imx8mp, imx8ulp, imx93

Should I report this in bugzilla.kernel.org ?

Regards,
Andreas Henriksson

WARNING: multiple messages have this Message-ID (diff)
From: Andreas Henriksson <andreas@fatal.se>
To: Fabio Estevam <festevam@gmail.com>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>,
	"Xiubo Li" <Xiubo.Lee@gmail.com>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	"Shengjiu Wang" <shengjiu.wang@nxp.com>,
	"Takashi Iwai" <tiwai@suse.com>,
	"Liam Girdwood" <lgirdwood@gmail.com>,
	"Jaroslav Kysela" <perex@perex.cz>,
	"Nicolin Chen" <nicoleotsuka@gmail.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Hans Söderlund" <hans.soderlund@realbit.se>,
	"Shengjiu Wang" <shengjiu.wang@gmail.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ASoC: fsl_sai: Enable MCTL_MCLK_EN bit for master mode
Date: Thu, 6 Jul 2023 10:47:06 +0200	[thread overview]
Message-ID: <20230706084706.bzwsbi3zisx5m5rl@fatal.se> (raw)
In-Reply-To: <CAOMZO5DtpoH0dLDX3=Sv4UUpX_=66VEZPsJUWQNnYviApfMLKQ@mail.gmail.com>

Hello Shengjiu, Fabio,

On Thu, May 19, 2022 at 10:23:06AM -0300, Fabio Estevam wrote:
> Hi Shengjiu,
> 
> On Thu, May 19, 2022 at 9:49 AM Shengjiu Wang <shengjiu.wang@nxp.com> wrote:
> 
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index fa950dde5310..dae16a14f177 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -437,6 +437,12 @@ static int fsl_sai_set_bclk(struct snd_soc_dai *dai, bool tx, u32 freq)
> >                                    FSL_SAI_CR2_DIV_MASK | FSL_SAI_CR2_BYP,
> >                                    savediv / 2 - 1);
> >
> > +       if (sai->soc_data->max_register >= FSL_SAI_MCTL) {
> 
> Isn't it a bit fragile to take this decision based on the number of
> SAI registers in the SoC?
> 
> What about adding a specific field in soc_data for such a purpose?

We've been working on an i.MX8MP where MCLK needs to be input and found
that this enables the MCLK as output despite not having set the
`fsl,sai-mclk-direction-output;` devicetree property in our DT.
Reverting the patch fixes the issues for us.

I have to say that the code comment made me a bit confused, but once
I found the commit message I understood why this code existed.
If this is really i.MX8MM specific maybe mention that in the code
comment and please make the code actually only trigger on i.MX8MM.
It seems to me like these all fulfill the current criteria:
imx7ulp, imx8mq, imx8mm, imx8mp, imx8ulp, imx93

Should I report this in bugzilla.kernel.org ?

Regards,
Andreas Henriksson

  parent reply	other threads:[~2023-07-06  8:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-19 12:36 [PATCH] ASoC: fsl_sai: Enable MCTL_MCLK_EN bit for master mode Shengjiu Wang
2022-05-19 13:23 ` Fabio Estevam
2022-05-19 13:23   ` Fabio Estevam
2022-05-19 13:23   ` Fabio Estevam
2022-05-19 13:41   ` Shengjiu Wang
2022-05-19 13:41     ` Shengjiu Wang
2023-07-06  8:47   ` Andreas Henriksson [this message]
2023-07-06  8:47     ` Andreas Henriksson
2023-07-06 11:06     ` Shengjiu Wang
2023-07-06 11:06       ` Shengjiu Wang
2023-07-06 11:08     ` Fabio Estevam
2023-07-06 11:08       ` Fabio Estevam
2023-07-06 11:19       ` Shengjiu Wang
2023-07-06 11:19         ` Shengjiu Wang
2023-07-06 11:32         ` Fabio Estevam
2023-07-06 11:32           ` Fabio Estevam
2023-07-06 12:34           ` Andreas Henriksson
2023-07-06 12:34             ` Andreas Henriksson
2023-07-06 13:22             ` Fabio Estevam
2023-07-06 13:22               ` Fabio Estevam
2023-07-06 22:00         ` Fabio Estevam
2023-07-06 22:00           ` Fabio Estevam
2022-06-08 20:46 ` Mark Brown
2022-06-08 20:46   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230706084706.bzwsbi3zisx5m5rl@fatal.se \
    --to=andreas@fatal.se \
    --cc=Xiubo.Lee@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=festevam@gmail.com \
    --cc=hans.soderlund@realbit.se \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=nicoleotsuka@gmail.com \
    --cc=perex@perex.cz \
    --cc=shengjiu.wang@gmail.com \
    --cc=shengjiu.wang@nxp.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.