2020-05-01 08:22:48

by Shengjiu Wang

[permalink] [raw]
Subject: [PATCH 0/3] ASoC: fsl_esai: Add support for imx8qm

Add support for imx8qm.

Shengjiu Wang (3):
ASoC: fsl_esai: introduce SoC specific data
ASoC: fsl_esai: Add support for imx8qm
ASoC: fsl_esai: Add new compatible string for imx8qm

.../devicetree/bindings/sound/fsl,esai.txt | 1 +
sound/soc/fsl/fsl_esai.c | 65 ++++++++++++++++---
2 files changed, 57 insertions(+), 9 deletions(-)

--
2.21.0


2020-05-01 08:22:59

by Shengjiu Wang

[permalink] [raw]
Subject: [PATCH 1/3] ASoC: fsl_esai: introduce SoC specific data

Introduce a SoC specific data structure which contains the
differences between the different SoCs.
This makes it easier to support more differences without having
to introduce a new if/else each time.

Signed-off-by: Shengjiu Wang <[email protected]>
---
sound/soc/fsl/fsl_esai.c | 46 ++++++++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 9 deletions(-)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 84290be778f0..bac65ba7fbad 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -21,6 +21,17 @@
SNDRV_PCM_FMTBIT_S20_3LE | \
SNDRV_PCM_FMTBIT_S24_LE)

+/**
+ * fsl_esai_soc_data: soc specific data
+ *
+ * @imx: for imx platform
+ * @reset_at_xrun: flags for enable reset operaton
+ */
+struct fsl_esai_soc_data {
+ bool imx;
+ bool reset_at_xrun;
+};
+
/**
* fsl_esai: ESAI private data
*
@@ -33,6 +44,7 @@
* @fsysclk: system clock source to derive HCK, SCK and FS
* @spbaclk: SPBA clock (optional, depending on SoC design)
* @task: tasklet to handle the reset operation
+ * @soc: soc specific data
* @lock: spin lock between hw_reset() and trigger()
* @fifo_depth: depth of tx/rx FIFO
* @slot_width: width of each DAI slot
@@ -44,7 +56,6 @@
* @sck_div: if using PSR/PM dividers for SCKx clock
* @slave_mode: if fully using DAI slave mode
* @synchronous: if using tx/rx synchronous mode
- * @reset_at_xrun: flags for enable reset operaton
* @name: driver name
*/
struct fsl_esai {
@@ -57,6 +68,7 @@ struct fsl_esai {
struct clk *fsysclk;
struct clk *spbaclk;
struct tasklet_struct task;
+ const struct fsl_esai_soc_data *soc;
spinlock_t lock; /* Protect hw_reset and trigger */
u32 fifo_depth;
u32 slot_width;
@@ -70,10 +82,24 @@ struct fsl_esai {
bool sck_div[2];
bool slave_mode;
bool synchronous;
- bool reset_at_xrun;
char name[32];
};

+static struct fsl_esai_soc_data fsl_esai_vf610 = {
+ .imx = false,
+ .reset_at_xrun = true,
+};
+
+static struct fsl_esai_soc_data fsl_esai_imx35 = {
+ .imx = true,
+ .reset_at_xrun = true,
+};
+
+static struct fsl_esai_soc_data fsl_esai_imx6ull = {
+ .imx = true,
+ .reset_at_xrun = false,
+};
+
static irqreturn_t esai_isr(int irq, void *devid)
{
struct fsl_esai *esai_priv = (struct fsl_esai *)devid;
@@ -85,7 +111,7 @@ static irqreturn_t esai_isr(int irq, void *devid)
regmap_read(esai_priv->regmap, REG_ESAI_SAISR, &saisr);

if ((saisr & (ESAI_SAISR_TUE | ESAI_SAISR_ROE)) &&
- esai_priv->reset_at_xrun) {
+ esai_priv->soc->reset_at_xrun) {
dev_dbg(&pdev->dev, "reset module for xrun\n");
regmap_update_bits(esai_priv->regmap, REG_ESAI_TCR,
ESAI_xCR_xEIE_MASK, 0);
@@ -936,9 +962,11 @@ static int fsl_esai_probe(struct platform_device *pdev)
esai_priv->pdev = pdev;
snprintf(esai_priv->name, sizeof(esai_priv->name), "%pOFn", np);

- if (of_device_is_compatible(np, "fsl,vf610-esai") ||
- of_device_is_compatible(np, "fsl,imx35-esai"))
- esai_priv->reset_at_xrun = true;
+ esai_priv->soc = of_device_get_match_data(&pdev->dev);
+ if (!esai_priv->soc) {
+ dev_err(&pdev->dev, "failed to get soc data\n");
+ return -ENODEV;
+ }

/* Get the addresses and IRQ */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1063,9 +1091,9 @@ static int fsl_esai_remove(struct platform_device *pdev)
}

static const struct of_device_id fsl_esai_dt_ids[] = {
- { .compatible = "fsl,imx35-esai", },
- { .compatible = "fsl,vf610-esai", },
- { .compatible = "fsl,imx6ull-esai", },
+ { .compatible = "fsl,imx35-esai", .data = &fsl_esai_imx35 },
+ { .compatible = "fsl,vf610-esai", .data = &fsl_esai_vf610 },
+ { .compatible = "fsl,imx6ull-esai", .data = &fsl_esai_imx6ull },
{}
};
MODULE_DEVICE_TABLE(of, fsl_esai_dt_ids);
--
2.21.0

2020-05-01 08:23:00

by Shengjiu Wang

[permalink] [raw]
Subject: [PATCH 3/3] ASoC: fsl_esai: Add new compatible string for imx8qm

Add new compatible string "fsl,imx8qm-esai" in the binding document.

Signed-off-by: Shengjiu Wang <[email protected]>
---
Documentation/devicetree/bindings/sound/fsl,esai.txt | 1 +
1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,esai.txt b/Documentation/devicetree/bindings/sound/fsl,esai.txt
index 0e6e2166f76c..0a2480aeecf0 100644
--- a/Documentation/devicetree/bindings/sound/fsl,esai.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,esai.txt
@@ -12,6 +12,7 @@ Required properties:
"fsl,imx35-esai",
"fsl,vf610-esai",
"fsl,imx6ull-esai",
+ "fsl,imx8qm-esai",

- reg : Offset and length of the register set for the device.

--
2.21.0

2020-05-01 08:23:04

by Shengjiu Wang

[permalink] [raw]
Subject: [PATCH 2/3] ASoC: fsl_esai: Add support for imx8qm

The difference for esai on imx8qm is that DMA device is EDMA.

EDMA requires the period size to be multiple of maxburst. Otherwise
the remaining bytes are not transferred and thus noise is produced.

We can handle this issue by adding a constraint on
SNDRV_PCM_HW_PARAM_PERIOD_SIZE to be multiple of tx/rx maxburst value.

Signed-off-by: Shengjiu Wang <[email protected]>
---
sound/soc/fsl/fsl_esai.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index bac65ba7fbad..61b5c0bde788 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -26,10 +26,12 @@
*
* @imx: for imx platform
* @reset_at_xrun: flags for enable reset operaton
+ * @use_edma: edma is used.
*/
struct fsl_esai_soc_data {
bool imx;
bool reset_at_xrun;
+ bool use_edma;
};

/**
@@ -88,16 +90,25 @@ struct fsl_esai {
static struct fsl_esai_soc_data fsl_esai_vf610 = {
.imx = false,
.reset_at_xrun = true,
+ .use_edma = false,
};

static struct fsl_esai_soc_data fsl_esai_imx35 = {
.imx = true,
.reset_at_xrun = true,
+ .use_edma = false,
};

static struct fsl_esai_soc_data fsl_esai_imx6ull = {
.imx = true,
.reset_at_xrun = false,
+ .use_edma = false,
+};
+
+static struct fsl_esai_soc_data fsl_esai_imx8qm = {
+ .imx = true,
+ .reset_at_xrun = false,
+ .use_edma = true,
};

static irqreturn_t esai_isr(int irq, void *devid)
@@ -513,6 +524,7 @@ static int fsl_esai_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
struct fsl_esai *esai_priv = snd_soc_dai_get_drvdata(dai);
+ bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;

if (!dai->active) {
/* Set synchronous mode */
@@ -527,6 +539,12 @@ static int fsl_esai_startup(struct snd_pcm_substream *substream,
ESAI_xCCR_xDC_MASK, ESAI_xCCR_xDC(2));
}

+ if (esai_priv->soc->use_edma)
+ snd_pcm_hw_constraint_step(substream->runtime, 0,
+ SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
+ tx ? esai_priv->dma_params_tx.maxburst :
+ esai_priv->dma_params_rx.maxburst);
+
return 0;

}
@@ -1094,6 +1112,7 @@ static const struct of_device_id fsl_esai_dt_ids[] = {
{ .compatible = "fsl,imx35-esai", .data = &fsl_esai_imx35 },
{ .compatible = "fsl,vf610-esai", .data = &fsl_esai_vf610 },
{ .compatible = "fsl,imx6ull-esai", .data = &fsl_esai_imx6ull },
+ { .compatible = "fsl,imx8qm-esai", .data = &fsl_esai_imx8qm },
{}
};
MODULE_DEVICE_TABLE(of, fsl_esai_dt_ids);
--
2.21.0

2020-05-01 10:26:07

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/3] ASoC: fsl_esai: Add support for imx8qm

On Fri, May 01, 2020 at 04:12:05PM +0800, Shengjiu Wang wrote:
> The difference for esai on imx8qm is that DMA device is EDMA.
>
> EDMA requires the period size to be multiple of maxburst. Otherwise
> the remaining bytes are not transferred and thus noise is produced.

If this constraint comes from the DMA controller then normally you'd
expect the DMA controller integration to be enforcing this - is there no
information in the DMA API that lets us know that this constraint is
there?


Attachments:
(No filename) (499.00 B)
signature.asc (499.00 B)
Download all attachments

2020-05-04 21:41:50

by Nicolin Chen

[permalink] [raw]
Subject: Re: [PATCH 1/3] ASoC: fsl_esai: introduce SoC specific data

On Fri, May 01, 2020 at 04:12:04PM +0800, Shengjiu Wang wrote:
> Introduce a SoC specific data structure which contains the
> differences between the different SoCs.
> This makes it easier to support more differences without having
> to introduce a new if/else each time.
>
> Signed-off-by: Shengjiu Wang <[email protected]>

Though the 2nd patch is having comments to address, this one
looks fine to me and should be able to merge as long as Mark
is okay with this too:

Acked-by: Nicolin Chen <[email protected]>

2020-05-06 02:47:56

by Shengjiu Wang

[permalink] [raw]
Subject: Re: [PATCH 2/3] ASoC: fsl_esai: Add support for imx8qm

Hi

On Fri, May 1, 2020 at 6:23 PM Mark Brown <[email protected]> wrote:
>
> On Fri, May 01, 2020 at 04:12:05PM +0800, Shengjiu Wang wrote:
> > The difference for esai on imx8qm is that DMA device is EDMA.
> >
> > EDMA requires the period size to be multiple of maxburst. Otherwise
> > the remaining bytes are not transferred and thus noise is produced.
>
> If this constraint comes from the DMA controller then normally you'd
> expect the DMA controller integration to be enforcing this - is there no
> information in the DMA API that lets us know that this constraint is
> there?

No, I can't find one API for this.
Do you have a recommendation?

best regards
wang shengjiu

2020-05-12 02:51:37

by Shengjiu Wang

[permalink] [raw]
Subject: Re: [PATCH 2/3] ASoC: fsl_esai: Add support for imx8qm

Hi Mark, Nicolin

On Wed, May 6, 2020 at 10:33 AM Shengjiu Wang <[email protected]> wrote:
>
> Hi
>
> On Fri, May 1, 2020 at 6:23 PM Mark Brown <[email protected]> wrote:
> >
> > On Fri, May 01, 2020 at 04:12:05PM +0800, Shengjiu Wang wrote:
> > > The difference for esai on imx8qm is that DMA device is EDMA.
> > >
> > > EDMA requires the period size to be multiple of maxburst. Otherwise
> > > the remaining bytes are not transferred and thus noise is produced.
> >
> > If this constraint comes from the DMA controller then normally you'd
> > expect the DMA controller integration to be enforcing this - is there no
> > information in the DMA API that lets us know that this constraint is
> > there?
>
> No, I can't find one API for this.
> Do you have a recommendation?
>
could you please recommend which DMA API can I use?

best regards
wang shengjiu

2020-05-12 12:42:25

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 2/3] ASoC: fsl_esai: Add support for imx8qm

On Tue, May 12, 2020 at 10:48:41AM +0800, Shengjiu Wang wrote:
> On Wed, May 6, 2020 at 10:33 AM Shengjiu Wang <[email protected]> wrote:
> > On Fri, May 1, 2020 at 6:23 PM Mark Brown <[email protected]> wrote:

> > > > EDMA requires the period size to be multiple of maxburst. Otherwise
> > > > the remaining bytes are not transferred and thus noise is produced.

> > > If this constraint comes from the DMA controller then normally you'd
> > > expect the DMA controller integration to be enforcing this - is there no
> > > information in the DMA API that lets us know that this constraint is
> > > there?

> > No, I can't find one API for this.
> > Do you have a recommendation?

> could you please recommend which DMA API can I use?

Not off-hand, you'd probably need to extend the API to export the
information.


Attachments:
(No filename) (842.00 B)
signature.asc (499.00 B)
Download all attachments

2020-05-12 16:40:04

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 3/3] ASoC: fsl_esai: Add new compatible string for imx8qm

On Fri, 1 May 2020 16:12:06 +0800, Shengjiu Wang wrote:
> Add new compatible string "fsl,imx8qm-esai" in the binding document.
>
> Signed-off-by: Shengjiu Wang <[email protected]>
> ---
> Documentation/devicetree/bindings/sound/fsl,esai.txt | 1 +
> 1 file changed, 1 insertion(+)
>

Acked-by: Rob Herring <[email protected]>

2020-05-15 10:06:11

by Shengjiu Wang

[permalink] [raw]
Subject: Re: [PATCH 2/3] ASoC: fsl_esai: Add support for imx8qm

On Tue, May 12, 2020 at 8:38 PM Mark Brown <[email protected]> wrote:
>
> On Tue, May 12, 2020 at 10:48:41AM +0800, Shengjiu Wang wrote:
> > On Wed, May 6, 2020 at 10:33 AM Shengjiu Wang <[email protected]> wrote:
> > > On Fri, May 1, 2020 at 6:23 PM Mark Brown <[email protected]> wrote:
>
> > > > > EDMA requires the period size to be multiple of maxburst. Otherwise
> > > > > the remaining bytes are not transferred and thus noise is produced.
>
> > > > If this constraint comes from the DMA controller then normally you'd
> > > > expect the DMA controller integration to be enforcing this - is there no
> > > > information in the DMA API that lets us know that this constraint is
> > > > there?
>
> > > No, I can't find one API for this.
> > > Do you have a recommendation?
>
> > could you please recommend which DMA API can I use?
>
> Not off-hand, you'd probably need to extend the API to export the
> information.

Thanks. I will think about if I can find a better solution.
And I will drop this change and send v2 of this patch-set.