Re: [PATCH] ASoC: fsl_sai: Enable MCTL_MCLK_EN bit for master mode

[Shengjiu Wang <shengjiu.wang@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1809 bytes --]

On Thu, Jul 6, 2023 at 4:47 PM Andreas Henriksson <andreas@fatal.se> wrote:

> 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.
>
> Good catch. seems there is an issue here.

best regards
wang shengjiu

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
>

[-- Attachment #2: Type: text/html, Size: 2649 bytes --]