All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V1 0/3] fsl esai/spdif/asrc need spba clock
@ 2015-09-17  6:09 Shengjiu Wang
  2015-09-17  6:09 ` [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device Shengjiu Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Shengjiu Wang @ 2015-09-17  6:09 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, timur,
	nicoleotsuka, Xiubo.Lee, lgirdwood, broonie, perex, tiwai
  Cc: alsa-devel

fsl esai/spdif/asrc need spba clock

Shengjiu Wang (3):
  ASoC: fsl_esai: spba clock is needed by esai device
  ASoC: fsl_spdif: spba clk is needed by spdif device
  ASoC: fsl_asrc: spba clock is needed by asrc device

 Documentation/devicetree/bindings/sound/fsl,asrc.txt  |  2 ++
 Documentation/devicetree/bindings/sound/fsl,esai.txt  |  2 ++
 Documentation/devicetree/bindings/sound/fsl,spdif.txt |  2 ++
 sound/soc/fsl/fsl_asrc.c                              | 12 ++++++++++++
 sound/soc/fsl/fsl_asrc.h                              |  1 +
 sound/soc/fsl/fsl_esai.c                              | 13 +++++++++++++
 sound/soc/fsl/fsl_spdif.c                             | 15 +++++++++++++++
 7 files changed, 47 insertions(+)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device
  2015-09-17  6:09 [PATCH V1 0/3] fsl esai/spdif/asrc need spba clock Shengjiu Wang
@ 2015-09-17  6:09 ` Shengjiu Wang
  2015-09-17  6:50   ` Nicolin Chen
  2015-09-17  6:09 ` [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device Shengjiu Wang
  2015-09-17  6:09 ` [PATCH V1 3/3] ASoC: fsl_asrc: spba clock is needed by asrc device Shengjiu Wang
  2 siblings, 1 reply; 13+ messages in thread
From: Shengjiu Wang @ 2015-09-17  6:09 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, timur,
	nicoleotsuka, Xiubo.Lee, lgirdwood, broonie, perex, tiwai
  Cc: alsa-devel

esai need to enable the spba clock, when sdma is using share peripheral
script. If don't enable it, may cause the read/write wrong data from/to
register.

Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
 Documentation/devicetree/bindings/sound/fsl,esai.txt |  2 ++
 sound/soc/fsl/fsl_esai.c                             | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,esai.txt b/Documentation/devicetree/bindings/sound/fsl,esai.txt
index d3b6b5f..d677203 100644
--- a/Documentation/devicetree/bindings/sound/fsl,esai.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,esai.txt
@@ -27,6 +27,8 @@ Required properties:
 			  derive HCK, SCK and FS.
 	"fsys"		  The system clock derived from ahb clock used to
 			  derive HCK, SCK and FS.
+	"dma"		  The spba clock is needed when sdma share peripheral
+			  script is used.
 
   - fsl,fifo-depth	: The number of elements in the transmit and receive
 			  FIFOs. This number is the maximum allowed value for
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c
index 837979e..4e99818 100644
--- a/sound/soc/fsl/fsl_esai.c
+++ b/sound/soc/fsl/fsl_esai.c
@@ -54,6 +54,7 @@ struct fsl_esai {
 	struct clk *coreclk;
 	struct clk *extalclk;
 	struct clk *fsysclk;
+	struct clk *dmaclk;
 	u32 fifo_depth;
 	u32 slot_width;
 	u32 slots;
@@ -469,6 +470,9 @@ static int fsl_esai_startup(struct snd_pcm_substream *substream,
 	ret = clk_prepare_enable(esai_priv->coreclk);
 	if (ret)
 		return ret;
+	ret = clk_prepare_enable(esai_priv->dmaclk);
+	if (ret)
+		return ret;
 	if (!IS_ERR(esai_priv->extalclk)) {
 		ret = clk_prepare_enable(esai_priv->extalclk);
 		if (ret)
@@ -499,6 +503,7 @@ err_fsysclk:
 	if (!IS_ERR(esai_priv->extalclk))
 		clk_disable_unprepare(esai_priv->extalclk);
 err_extalck:
+	clk_disable_unprepare(esai_priv->dmaclk);
 	clk_disable_unprepare(esai_priv->coreclk);
 
 	return ret;
@@ -564,6 +569,7 @@ static void fsl_esai_shutdown(struct snd_pcm_substream *substream,
 		clk_disable_unprepare(esai_priv->fsysclk);
 	if (!IS_ERR(esai_priv->extalclk))
 		clk_disable_unprepare(esai_priv->extalclk);
+	clk_disable_unprepare(esai_priv->dmaclk);
 	clk_disable_unprepare(esai_priv->coreclk);
 }
 
@@ -772,6 +778,13 @@ static int fsl_esai_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "failed to get fsys clock: %ld\n",
 				PTR_ERR(esai_priv->fsysclk));
 
+	esai_priv->dmaclk = devm_clk_get(&pdev->dev, "dma");
+	if (IS_ERR(esai_priv->dmaclk)) {
+		dev_err(&pdev->dev, "Cannot get dma clock: %ld\n",
+				PTR_ERR(esai_priv->dmaclk));
+		return PTR_ERR(esai_priv->dmaclk);
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-09-17  6:09 [PATCH V1 0/3] fsl esai/spdif/asrc need spba clock Shengjiu Wang
  2015-09-17  6:09 ` [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device Shengjiu Wang
@ 2015-09-17  6:09 ` Shengjiu Wang
  2015-09-17 11:01   ` Mark Brown
  2015-09-17  6:09 ` [PATCH V1 3/3] ASoC: fsl_asrc: spba clock is needed by asrc device Shengjiu Wang
  2 siblings, 1 reply; 13+ messages in thread
From: Shengjiu Wang @ 2015-09-17  6:09 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, timur,
	nicoleotsuka, Xiubo.Lee, lgirdwood, broonie, perex, tiwai
  Cc: alsa-devel

spdif need to enable the spba clock, when sdma is using share peripheral
script. If don't enable it, may cause the read/write wrong data from/to
registers.

Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
 Documentation/devicetree/bindings/sound/fsl,spdif.txt |  2 ++
 sound/soc/fsl/fsl_spdif.c                             | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.txt b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
index b5ee32e..9113dc0 100644
--- a/Documentation/devicetree/bindings/sound/fsl,spdif.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
@@ -27,6 +27,8 @@ Required properties:
 			  Transceiver Clock Diagram" of SoC reference manual.
 			  It can also be referred to TxClk_Source bit of
 			  register SPDIF_STC.
+	"dma"		  The spba clock is needed when sdma share peripheral
+			  script is used.
 
    - big-endian		: If this property is absent, the native endian mode
 			  will be in use as default, or the big endian mode
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index ab729f2..2f4cb37 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -106,6 +106,7 @@ struct fsl_spdif_priv {
 	struct clk *rxclk;
 	struct clk *coreclk;
 	struct clk *sysclk;
+	struct clk *dmaclk;
 	struct snd_dmaengine_dai_dma_data dma_params_tx;
 	struct snd_dmaengine_dai_dma_data dma_params_rx;
 };
@@ -466,6 +467,12 @@ static int fsl_spdif_startup(struct snd_pcm_substream *substream,
 			return ret;
 		}
 
+		ret = clk_prepare_enable(spdif_priv->dmaclk);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to enable dma clock\n");
+			return ret;
+		}
+
 		ret = spdif_softreset(spdif_priv);
 		if (ret) {
 			dev_err(&pdev->dev, "failed to soft reset\n");
@@ -540,6 +547,7 @@ static void fsl_spdif_shutdown(struct snd_pcm_substream *substream,
 		spdif_intr_status_clear(spdif_priv);
 		regmap_update_bits(regmap, REG_SPDIF_SCR,
 				SCR_LOW_POWER, SCR_LOW_POWER);
+		clk_disable_unprepare(spdif_priv->dmaclk);
 		clk_disable_unprepare(spdif_priv->coreclk);
 	}
 }
@@ -1221,6 +1229,13 @@ static int fsl_spdif_probe(struct platform_device *pdev)
 		return PTR_ERR(spdif_priv->coreclk);
 	}
 
+	/* Get dma clock for dma script operation */
+	spdif_priv->dmaclk = devm_clk_get(&pdev->dev, "dma");
+	if (IS_ERR(spdif_priv->dmaclk)) {
+		dev_err(&pdev->dev, "no dma clock in devicetree\n");
+		return PTR_ERR(spdif_priv->dmaclk);
+	}
+
 	/* Select clock source for rx/tx clock */
 	spdif_priv->rxclk = devm_clk_get(&pdev->dev, "rxtx1");
 	if (IS_ERR(spdif_priv->rxclk)) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH V1 3/3] ASoC: fsl_asrc: spba clock is needed by asrc device
  2015-09-17  6:09 [PATCH V1 0/3] fsl esai/spdif/asrc need spba clock Shengjiu Wang
  2015-09-17  6:09 ` [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device Shengjiu Wang
  2015-09-17  6:09 ` [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device Shengjiu Wang
@ 2015-09-17  6:09 ` Shengjiu Wang
  2 siblings, 0 replies; 13+ messages in thread
From: Shengjiu Wang @ 2015-09-17  6:09 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, timur,
	nicoleotsuka, Xiubo.Lee, lgirdwood, broonie, perex, tiwai
  Cc: alsa-devel

asrc need to enable the spba clock, when sdma is using share peripheral
script. If don't enable it, may cause the read/write wrong data from/to
register.

Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
 Documentation/devicetree/bindings/sound/fsl,asrc.txt |  2 ++
 sound/soc/fsl/fsl_asrc.c                             | 12 ++++++++++++
 sound/soc/fsl/fsl_asrc.h                             |  1 +
 3 files changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
index b93362a..90a8cd6 100644
--- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt
+++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
@@ -25,6 +25,8 @@ Required properties:
 	"mem"		  Peripheral access clock to access registers.
 	"ipg"		  Peripheral clock to driver module.
 	"asrck_<0-f>"	  Clock sources for input and output clock.
+	"dma"		  The spba clock is needed when sdma share peripheral
+			  script is used.
 
    - big-endian		: If this property is absent, the little endian mode
 			  will be in use as default. Otherwise, the big endian
diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 9f087d4..5ec1eae 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -859,6 +859,12 @@ static int fsl_asrc_probe(struct platform_device *pdev)
 		return PTR_ERR(asrc_priv->ipg_clk);
 	}
 
+	asrc_priv->dma_clk = devm_clk_get(&pdev->dev, "dma");
+	if (IS_ERR(asrc_priv->dma_clk)) {
+		dev_err(&pdev->dev, "failed to get dma script clock\n");
+		return PTR_ERR(asrc_priv->dma_clk);
+	}
+
 	for (i = 0; i < ASRC_CLK_MAX_NUM; i++) {
 		sprintf(tmp, "asrck_%x", i);
 		asrc_priv->asrck_clk[i] = devm_clk_get(&pdev->dev, tmp);
@@ -939,6 +945,9 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 	ret = clk_prepare_enable(asrc_priv->ipg_clk);
 	if (ret)
 		goto disable_mem_clk;
+	ret = clk_prepare_enable(asrc_priv->dma_clk);
+	if (ret)
+		goto disable_ipg_clk;
 	for (i = 0; i < ASRC_CLK_MAX_NUM; i++) {
 		ret = clk_prepare_enable(asrc_priv->asrck_clk[i]);
 		if (ret)
@@ -950,6 +959,8 @@ static int fsl_asrc_runtime_resume(struct device *dev)
 disable_asrck_clk:
 	for (i--; i >= 0; i--)
 		clk_disable_unprepare(asrc_priv->asrck_clk[i]);
+	clk_disable_unprepare(asrc_priv->dma_clk);
+disable_ipg_clk:
 	clk_disable_unprepare(asrc_priv->ipg_clk);
 disable_mem_clk:
 	clk_disable_unprepare(asrc_priv->mem_clk);
@@ -963,6 +974,7 @@ static int fsl_asrc_runtime_suspend(struct device *dev)
 
 	for (i = 0; i < ASRC_CLK_MAX_NUM; i++)
 		clk_disable_unprepare(asrc_priv->asrck_clk[i]);
+	clk_disable_unprepare(asrc_priv->dma_clk);
 	clk_disable_unprepare(asrc_priv->ipg_clk);
 	clk_disable_unprepare(asrc_priv->mem_clk);
 
diff --git a/sound/soc/fsl/fsl_asrc.h b/sound/soc/fsl/fsl_asrc.h
index 4aed63c..1bb13c2 100644
--- a/sound/soc/fsl/fsl_asrc.h
+++ b/sound/soc/fsl/fsl_asrc.h
@@ -442,6 +442,7 @@ struct fsl_asrc {
 	unsigned long paddr;
 	struct clk *mem_clk;
 	struct clk *ipg_clk;
+	struct clk *dma_clk;
 	struct clk *asrck_clk[ASRC_CLK_MAX_NUM];
 	spinlock_t lock;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device
  2015-09-17  6:09 ` [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device Shengjiu Wang
@ 2015-09-17  6:50   ` Nicolin Chen
  0 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2015-09-17  6:50 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: mark.rutland, alsa-devel, pawel.moll, ijc+devicetree, broonie,
	Xiubo.Lee, timur, lgirdwood, robh+dt, galak, tiwai

Hi,

On Thu, Sep 17, 2015 at 02:09:23PM +0800, Shengjiu Wang wrote:
> esai need to enable the spba clock, when sdma is using share peripheral
> script. If don't enable it, may cause the read/write wrong data from/to
> register.

It would be nicer to elaborate the SPBA as well, especially
the reason why it "may cause the read/write wrong data from/to
register".

> Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>

> diff --git a/Documentation/devicetree/bindings/sound/fsl,esai.txt b/Documentation/devicetree/bindings/sound/fsl,esai.txt
> index d3b6b5f..d677203 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,esai.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl,esai.txt
> @@ -27,6 +27,8 @@ Required properties:
>  			  derive HCK, SCK and FS.
>  	"fsys"		  The system clock derived from ahb clock used to
>  			  derive HCK, SCK and FS.
> +	"dma"		  The spba clock is needed when sdma share peripheral
> +			  script is used.

It would be better to treat it as an optional property as ESAI
itself doesn't require it to be enabled while a specific SDMA
script does so as to access the registers successfully via the
arbiter. There is one of existing SoCs that doesn't have SDMA
and SPBA at all so they don't need it.

> @@ -469,6 +470,9 @@ static int fsl_esai_startup(struct snd_pcm_substream *substream,
>  	ret = clk_prepare_enable(esai_priv->coreclk);
>  	if (ret)
>  		return ret;
> +	ret = clk_prepare_enable(esai_priv->dmaclk);
> +	if (ret)
> +		return ret;

Needs a goto for a failed case in order to disable the coreclk.

> @@ -772,6 +778,13 @@ static int fsl_esai_probe(struct platform_device *pdev)
>  		dev_warn(&pdev->dev, "failed to get fsys clock: %ld\n",
>  				PTR_ERR(esai_priv->fsysclk));
>  
> +	esai_priv->dmaclk = devm_clk_get(&pdev->dev, "dma");
> +	if (IS_ERR(esai_priv->dmaclk)) {
> +		dev_err(&pdev->dev, "Cannot get dma clock: %ld\n",
> +				PTR_ERR(esai_priv->dmaclk));
> +		return PTR_ERR(esai_priv->dmaclk);
> +	}

This will break the old DTB -- would be solved by making the clock
optional though. And it would be better to follow the pattern of
previous error message.

Nicolin

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-09-17  6:09 ` [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device Shengjiu Wang
@ 2015-09-17 11:01   ` Mark Brown
  2015-09-17 17:54     ` Nicolin Chen
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2015-09-17 11:01 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: mark.rutland, alsa-devel, pawel.moll, ijc+devicetree, Xiubo.Lee,
	timur, lgirdwood, nicoleotsuka, robh+dt, galak, tiwai


[-- Attachment #1.1: Type: text/plain, Size: 280 bytes --]

On Thu, Sep 17, 2015 at 02:09:24PM +0800, Shengjiu Wang wrote:

> +	"dma"		  The spba clock is needed when sdma share peripheral
> +			  script is used.

It sounds like this clock is used by the DMA controller - shouldn't the
DMA controller driver be responsible for enabling it?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-09-17 11:01   ` Mark Brown
@ 2015-09-17 17:54     ` Nicolin Chen
  2015-09-19 13:50       ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Nicolin Chen @ 2015-09-17 17:54 UTC (permalink / raw)
  To: Mark Brown
  Cc: mark.rutland, Shengjiu Wang, pawel.moll, ijc+devicetree,
	Xiubo.Lee, timur, lgirdwood, alsa-devel, robh+dt, galak, tiwai

On Thu, Sep 17, 2015 at 12:01:54PM +0100, Mark Brown wrote:
> On Thu, Sep 17, 2015 at 02:09:24PM +0800, Shengjiu Wang wrote:
> 
> > +	"dma"		  The spba clock is needed when sdma share peripheral
> > +			  script is used.

> It sounds like this clock is used by the DMA controller - shouldn't the
> DMA controller driver be responsible for enabling it?

I agree but it doesn't seems to be easy to enable and disable it
dynamically along with ASoC open() and close() functions as the
current clocks of SDMA get enabled when its channels are allocated
during the system boot-up via generic DMA bindings.

Maybe we can put in the issue_pending() and terminate_all() which
are regularly called along with ASoC trigger(), or just open it
permanently if the penalty of power efficiency is ignorable.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-09-17 17:54     ` Nicolin Chen
@ 2015-09-19 13:50       ` Mark Brown
  2015-10-10 10:26         ` Shengjiu Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2015-09-19 13:50 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: mark.rutland, Shengjiu Wang, pawel.moll, ijc+devicetree,
	Xiubo.Lee, timur, lgirdwood, alsa-devel, robh+dt, galak, tiwai


[-- Attachment #1.1: Type: text/plain, Size: 1173 bytes --]

On Thu, Sep 17, 2015 at 10:54:08AM -0700, Nicolin Chen wrote:
> On Thu, Sep 17, 2015 at 12:01:54PM +0100, Mark Brown wrote:
> > On Thu, Sep 17, 2015 at 02:09:24PM +0800, Shengjiu Wang wrote:

> > > +	"dma"		  The spba clock is needed when sdma share peripheral
> > > +			  script is used.

> > It sounds like this clock is used by the DMA controller - shouldn't the
> > DMA controller driver be responsible for enabling it?

> I agree but it doesn't seems to be easy to enable and disable it
> dynamically along with ASoC open() and close() functions as the
> current clocks of SDMA get enabled when its channels are allocated
> during the system boot-up via generic DMA bindings.

Do we need to do things along with open() and close() or wouldn't doing
things along with the DMA operatiosn suffice?

> Maybe we can put in the issue_pending() and terminate_all() which
> are regularly called along with ASoC trigger(), or just open it
> permanently if the penalty of power efficiency is ignorable.

Or some kind of runtime PM usage within the DMA driver (possibly at
those points)?  It does seem like something we should solve in the DMA
code rather than in generic code.


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-09-19 13:50       ` Mark Brown
@ 2015-10-10 10:26         ` Shengjiu Wang
  2015-10-10 20:03           ` Fabio Estevam
  0 siblings, 1 reply; 13+ messages in thread
From: Shengjiu Wang @ 2015-10-10 10:26 UTC (permalink / raw)
  To: Mark Brown
  Cc: mark.rutland, alsa-devel, pawel.moll, ijc+devicetree, Xiubo.Lee,
	timur, lgirdwood, Nicolin Chen, robh+dt, galak, tiwai

On Sat, Sep 19, 2015 at 06:50:40AM -0700, Mark Brown wrote:
> On Thu, Sep 17, 2015 at 10:54:08AM -0700, Nicolin Chen wrote:
> > On Thu, Sep 17, 2015 at 12:01:54PM +0100, Mark Brown wrote:
> > > On Thu, Sep 17, 2015 at 02:09:24PM +0800, Shengjiu Wang wrote:
> 
> > > > +	"dma"		  The spba clock is needed when sdma share peripheral
> > > > +			  script is used.
> 
> > > It sounds like this clock is used by the DMA controller - shouldn't the
> > > DMA controller driver be responsible for enabling it?
> 
> > I agree but it doesn't seems to be easy to enable and disable it
> > dynamically along with ASoC open() and close() functions as the
> > current clocks of SDMA get enabled when its channels are allocated
> > during the system boot-up via generic DMA bindings.
> 
> Do we need to do things along with open() and close() or wouldn't doing
> things along with the DMA operatiosn suffice?
> 
> > Maybe we can put in the issue_pending() and terminate_all() which
> > are regularly called along with ASoC trigger(), or just open it
> > permanently if the penalty of power efficiency is ignorable.
> 
> Or some kind of runtime PM usage within the DMA driver (possibly at
> those points)?  It does seem like something we should solve in the DMA
> code rather than in generic code.
> 

I think spba clock should be used by the audio IP not DMA, because in same
situation the FSL_SSI doesn't need this clock.

best regards
wang shengjiu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-10-10 10:26         ` Shengjiu Wang
@ 2015-10-10 20:03           ` Fabio Estevam
  2015-10-10 20:35             ` Nicolin Chen
  0 siblings, 1 reply; 13+ messages in thread
From: Fabio Estevam @ 2015-10-10 20:03 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Mark Rutland, alsa-devel, Pawel Moll, Xiubo Li, Ian Campbell,
	Timur Tabi, robh+dt, Liam Girdwood, Nicolin Chen, Mark Brown,
	Kumar Gala, Takashi Iwai

On Sat, Oct 10, 2015 at 7:26 AM, Shengjiu Wang
<shengjiu.wang@freescale.com> wrote:

> I think spba clock should be used by the audio IP not DMA, because in same
> situation the FSL_SSI doesn't need this clock.

According to this commit from FSL BSP, the SSI2 port on MX53 is
connected to SPBA and needs the SPBA clock:

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/arch/arm/mach-mx5/clock.c?h=imx_2.6.35_maintain&id=2438f48431b7aff00a522502d5a6ea217ea133d9

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-10-10 20:03           ` Fabio Estevam
@ 2015-10-10 20:35             ` Nicolin Chen
  2015-11-04  8:34               ` Shengjiu Wang
  0 siblings, 1 reply; 13+ messages in thread
From: Nicolin Chen @ 2015-10-10 20:35 UTC (permalink / raw)
  To: Fabio Estevam, Shengjiu Wang
  Cc: Mark Rutland, alsa-devel, Pawel Moll, Xiubo Li, Ian Campbell,
	Timur Tabi, robh+dt, Liam Girdwood, Mark Brown, Kumar Gala,
	Takashi Iwai

On Sat, Oct 10, 2015 at 05:03:07PM -0300, Fabio Estevam wrote:
> On Sat, Oct 10, 2015 at 7:26 AM, Shengjiu Wang
> <shengjiu.wang@freescale.com> wrote:
> 
> > I think spba clock should be used by the audio IP not DMA, because in same
> > situation the FSL_SSI doesn't need this clock.
> 
> According to this commit from FSL BSP, the SSI2 port on MX53 is
> connected to SPBA and needs the SPBA clock:
> 
> http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/arch/arm/mach-mx5/clock.c?h=imx_2.6.35_maintain&id=2438f48431b7aff00a522502d5a6ea217ea133d9

Hmm, the detail of an SPBA issue is sometimes complicated due to
different fabric designs between SoCs. We cannot simply say that
a DMA access for an IP under the Shared Peripheral Bus requires
SPBA clock or not unless the hardware team clearly demands that.

However, as an arbiter, the SPBA must be outside any of those IPs
and tied to the SDMA in order to balancing memory access between
the SDMA and a co-existing CPU. The question is whether we treat
it as a part of an IP wrapper or a part of the SDMA.

Therefore, unless a CPU access would fail even *without any SDMA
access being launched*, it sounds fair to me to put it into the
DMA code.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-10-10 20:35             ` Nicolin Chen
@ 2015-11-04  8:34               ` Shengjiu Wang
  2015-11-04 17:31                 ` Nicolin Chen
  0 siblings, 1 reply; 13+ messages in thread
From: Shengjiu Wang @ 2015-11-04  8:34 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: Mark Rutland, alsa-devel, Pawel Moll, Ian Campbell, Xiubo Li,
	Timur Tabi, robh+dt, Liam Girdwood, Mark Brown, Kumar Gala,
	Takashi Iwai, Fabio Estevam

On Sat, Oct 10, 2015 at 01:35:57PM -0700, Nicolin Chen wrote:
> On Sat, Oct 10, 2015 at 05:03:07PM -0300, Fabio Estevam wrote:
> > On Sat, Oct 10, 2015 at 7:26 AM, Shengjiu Wang
> > <shengjiu.wang@freescale.com> wrote:
> > 
> > > I think spba clock should be used by the audio IP not DMA, because in same
> > > situation the FSL_SSI doesn't need this clock.
> > 
> > According to this commit from FSL BSP, the SSI2 port on MX53 is
> > connected to SPBA and needs the SPBA clock:
> > 
> > http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/arch/arm/mach-mx5/clock.c?h=imx_2.6.35_maintain&id=2438f48431b7aff00a522502d5a6ea217ea133d9
> 
> Hmm, the detail of an SPBA issue is sometimes complicated due to
> different fabric designs between SoCs. We cannot simply say that
> a DMA access for an IP under the Shared Peripheral Bus requires
> SPBA clock or not unless the hardware team clearly demands that.
> 
> However, as an arbiter, the SPBA must be outside any of those IPs
> and tied to the SDMA in order to balancing memory access between
> the SDMA and a co-existing CPU. The question is whether we treat
> it as a part of an IP wrapper or a part of the SDMA.
> 
> Therefore, unless a CPU access would fail even *without any SDMA
> access being launched*, it sounds fair to me to put it into the
> DMA code.

I discussed this issue with IC team. The SPBA bus has 3 master port,
one is for SDMA, one is for MCU (ARM core), the third one is for DSP (
currently there is no connection for DSP). Any two master port are used
simultaneously, there will be arbitration issue when SPBA clock is not
enabled.

So I think it is better put SPBA clock in Audio IP, because if there is
DSP master port + MCU master used, there will be issue too. (DSP master 
port is not used in i.MX chip). Anyway I think put it in SDMA driver is
not good choice, if SDMA is removed, there will be still issue. Put it
in Audio IP maybe more safe.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device
  2015-11-04  8:34               ` Shengjiu Wang
@ 2015-11-04 17:31                 ` Nicolin Chen
  0 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2015-11-04 17:31 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Mark Rutland, alsa-devel, Pawel Moll, Ian Campbell, Xiubo Li,
	Timur Tabi, robh+dt, Liam Girdwood, Mark Brown, Kumar Gala,
	Takashi Iwai, Fabio Estevam

On Wed, Nov 04, 2015 at 04:34:44PM +0800, Shengjiu Wang wrote:
> I discussed this issue with IC team. The SPBA bus has 3 master port,
> one is for SDMA, one is for MCU (ARM core), the third one is for DSP (
> currently there is no connection for DSP). Any two master port are used
> simultaneously, there will be arbitration issue when SPBA clock is not
> enabled.
> 
> So I think it is better put SPBA clock in Audio IP, because if there is
> DSP master port + MCU master used, there will be issue too. (DSP master 
> port is not used in i.MX chip). Anyway I think put it in SDMA driver is
> not good choice, if SDMA is removed, there will be still issue. Put it
> in Audio IP maybe more safe.

It's really ambiguous here as it's not easy to say which side is
the best. I'm personally okay if you insist so. However, you need
to refine the patch as I suggested in my first reply and document
it well: Apparently, "dma" is not a good name for this clock. And
it should be an optional property so as not to break the old DTBs.
And only the platforms (check DT compatible) that use the arbiter
can fetch the clock or get a warning when not present in the DTB.

Thanks
Nicolin

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-11-04 17:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-17  6:09 [PATCH V1 0/3] fsl esai/spdif/asrc need spba clock Shengjiu Wang
2015-09-17  6:09 ` [PATCH V1 1/3] ASoC: fsl_esai: spba clock is needed by esai device Shengjiu Wang
2015-09-17  6:50   ` Nicolin Chen
2015-09-17  6:09 ` [PATCH V1 2/3] ASoC: fsl_spdif: spba clk is needed by spdif device Shengjiu Wang
2015-09-17 11:01   ` Mark Brown
2015-09-17 17:54     ` Nicolin Chen
2015-09-19 13:50       ` Mark Brown
2015-10-10 10:26         ` Shengjiu Wang
2015-10-10 20:03           ` Fabio Estevam
2015-10-10 20:35             ` Nicolin Chen
2015-11-04  8:34               ` Shengjiu Wang
2015-11-04 17:31                 ` Nicolin Chen
2015-09-17  6:09 ` [PATCH V1 3/3] ASoC: fsl_asrc: spba clock is needed by asrc device Shengjiu Wang

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.