[alsa-devel] [PATCH] ASoC: fsl_sai: Allow the SAI driver to turn on SAI_MCLK

[Nicolin Chen]

Hi Fabio,

On Wed, May 04, 2016 at 09:17:11AM -0300, Fabio Estevam wrote:
> Hi Nicolin,
> 
> On Wed, May 4, 2016 at 4:07 AM, Nicolin Chen <nicoleotsuka at gmail.com> wrote:
> 
> > I found out the mail from Zidan regarding the same GPR bit:
> > http://mailman.alsa-project.org/pipermail/alsa-devel/2015-August/096401.html
> >
> > According to his reply, this bit also controls the clock source of MCLK2
> > for the SAI. Each SAI has 3 MCLKs, but first disregarding MCLK3:
> >
> > When this bit gets set, the MCLK1 and MCLK2 of the corresponding SAI are
> > both getting clock from CCM, and in the meantime outputting the clock via
> > the PAD to external Codec chips. In this case, MCLK1 and MCLK2 have same
> > clock rate, nothing special for bit clock dividing.
> >
> > When this bit gets clear, MCLK1 of the corresponding is still getting its
> > clock from CCM while MCLK2 switches its source from CCM to the PAD. In this
> > case, MCLK1 and MCLK2 can have different clock rates so as to support two
> > sample rate groups: 44.1KHz and 48Khz.
> >
> > So, beside gating the clock output, it's more likely a clock MUX for MCLK2
> > of each SAI to switch between CCM and external input.
> 
> At imx6ul.dtsi the mclk2 and mclk3 are just dummy clocks.

We both know sometimes the source code might not reflect the true
IC design because a part of the design is not documented in detail,
some hidden registers in SSI for example.

> > Because DT property will be hard to change once we define it. I think it
> > would be better to confirm this first before patching it (with Zidan or
> > i.MX IC team). But the driver part, whether putting it to probe() or to
> > set_dai_sysclk(), doesn't matter to me since we can change/improve it
> > later as long as it follows the correct binding.
> 
> Also tried Zidan's NXP email and it also bounced, so not able to contact him.
> 
> Also don't have any contact with the audio folks in the i.MX IC team.
> 
> This binding is all about setting IMX6UL_GPR1_SAIx_MCLK_DIR or not.
>
> So this is a very simple case:
> 
> IMX6UL_GPR1_SAIx_MCLK_DIR = 0 (this is the current behaviour)
> 
> IMX6UL_GPR1_SAIx_MCLK_DIR = 1 (this is what this patch allows)

I have already described the difference between set and clear is
not this simple. Honestly, without reading Zidan's mail above,
I would suggest to set all three bits for SAIs in mach-imx6ul.c
anyway since they are just simply enablers. Since each SAI clock
has a gate in CCM, why bother to set these three bits permanently.

> I cannot see why this proposed binding would possibly break things or
> would need a change in the future.

I have never said anything about breaking things but only accuracy
of the property name.

> It's only purpose is to simply set IMX6UL_GPR1_SAIx_MCLK_DIR.

I understand you want to get this in as soon as possible, that's why
I said I can ignore the place of implementation as long as the name
of the DT property won't create confusion for input clock feature.
So why not just use the name as itself -- direction?

Thanks
Nicolin

> 
> Regards,
> 
> Fabio Estevam