RE: [alsa-devel] [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.

[Xiubo Li-B47053 <B47053@freescale.com>]

> > +static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
> > +		struct snd_pcm_hw_params *params,
> > +		struct snd_soc_dai *cpu_dai)
> > +{
> > +	int ret;
> > +
> > +	ret =3D fsl_sai_hw_params_tr(substream, params, cpu_dai,
> > +				FSL_FMT_TRANSMITTER);
> > +	if (ret) {
> > +		dev_err(cpu_dai->dev,
> > +				"Cannot set sai transmitter hw params: %d\n",
> > +				ret);
> > +		return ret;
> > +	}
> > +
> > +	ret =3D fsl_sai_hw_params_tr(substream, params, cpu_dai,
> > +				FSL_FMT_RECEIVER);
> > +	if (ret) {
> > +		dev_err(cpu_dai->dev,
> > +				"Cannot set sai's receiver hw params: %d\n",
> > +				ret);
> > +		return ret;
> > +	}
>=20
> Shouldn't, depending on the substream direction, either transmit or
> receiver be configured, instead of always configuring both?
>=20

Yes, this can be configed separately. Please see the next version.

> > +
> > +	return 0;
> > +}
> > +
> > +static int fsl_sai_trigger(struct snd_pcm_substream *substream, int
> cmd,
> > +		struct snd_soc_dai *dai)
> > +{
> > +	struct fsl_sai *sai =3D snd_soc_dai_get_drvdata(dai);
> > +	unsigned int tcsr, rcsr;
> > +
> > +	tcsr =3D readl(sai->base + SAI_TCSR);
> > +	rcsr =3D readl(sai->base + SAI_RCSR);
> > +
> > +	switch (cmd) {
> > +	case SNDRV_PCM_TRIGGER_START:
> > +	case SNDRV_PCM_TRIGGER_RESUME:
> > +	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> > +		rcsr |=3D SAI_CSR_TERE | SAI_CSR_FRDE;
> > +		tcsr |=3D SAI_CSR_TERE | SAI_CSR_FRDE;
> > +		writel(rcsr, sai->base + SAI_RCSR);
> > +		udelay(10);
> > +		writel(tcsr, sai->base + SAI_TCSR);
> > +		break;
> > +
> > +	case SNDRV_PCM_TRIGGER_STOP:
> > +	case SNDRV_PCM_TRIGGER_SUSPEND:
> > +	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> > +		tcsr &=3D ~(SAI_CSR_TERE | SAI_CSR_FRDE);
> > +		rcsr &=3D ~(SAI_CSR_TERE | SAI_CSR_FRDE);
> > +		writel(tcsr, sai->base + SAI_TCSR);
> > +		udelay(10);
> > +		writel(rcsr, sai->base + SAI_RCSR);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
>=20
> Same here, shouldn't tx and rx be started independently depending on the
> substream direction?
>=20

But this couldn't, from the SAI's spec we can see that:
------
The SAI transmitter and receiver can be configured to operate with synchron=
ous bit clock
and frame sync.

1),=20
If the transmitter bit clock and frame sync are to be used by both the tran=
smitter and
receiver:
* The transmitter must be configured for asynchronous operation and the rec=
eiver for
synchronous operation.
* In synchronous mode, the receiver is enabled only when both the transmitt=
er and
receiver are enabled.
* It is recommended that the transmitter is the last enabled and the first =
disabled.

2),
If the receiver bit clock and frame sync are to be used by both the transmi=
tter and
receiver:
* The receiver must be configured for asynchronous operation and the transm=
itter for
synchronous operation.
* In synchronous mode, the transmitter is enabled only when both the receiv=
er and
transmitter are both enabled.
* It is recommended that the receiver is the last enabled and the first dis=
abled.
------

The receiver and transmitter should be both enabled at the same time if any=
 of them is alive.


> > +	return 0;
> > +}
> > +
> > +static struct snd_soc_dai_ops fsl_sai_pcm_dai_ops =3D {
>=20
> const
>=20

Please see the next version.