[alsa-devel] [PATCH 0/2] Add ipg clock control to sai driver
This series of patches add ipg clock control to fsl_sai driver and updates the vf610.dtsi accordingly.
@Shawn I'm not sure if VF610 currently does full works with SAI audio function. The PATCH-2 is based on broonie/for-next.
Nicolin Chen (2): ASoC: fsl_sai: Add clock control for SAI ARM: dts: Add ipg clock for sai2 on VF610 platform
.../devicetree/bindings/sound/fsl-sai.txt | 7 ++-- arch/arm/boot/dts/vf610.dtsi | 4 +-- sound/soc/fsl/fsl_sai.c | 37 ++++++++++++++++++++-- sound/soc/fsl/fsl_sai.h | 2 ++ 4 files changed, 43 insertions(+), 7 deletions(-)
The SAI mainly has two clocks: ipg_clock -- registers access for SoC or DMA to read and write. sai_clock -- providing DAI format bit clock and frame clock.
Thus this patch adds these two clocks to the driver with their clock controls and replaces the regmap clock 'sai_clock' with 'ipg_clock'.
Signed-off-by: Nicolin Chen Guangyu.Chen@freescale.com --- .../devicetree/bindings/sound/fsl-sai.txt | 7 ++-- sound/soc/fsl/fsl_sai.c | 37 ++++++++++++++++++++-- sound/soc/fsl/fsl_sai.h | 2 ++ 3 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt index 35c09fe..bad4453 100644 --- a/Documentation/devicetree/bindings/sound/fsl-sai.txt +++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt @@ -11,5 +11,6 @@ Required properties: - reg: Offset and length of the register set for the device. - clocks: Must contain an entry for each entry in clock-names. -- clock-names : Must include the "sai" entry. +- clock-names : Must include the "ipg" for register access and "sai" for bit + clock and frame clock providing. - dmas : Generic dma devicetree binding as described in Documentation/devicetree/bindings/dma/dma.txt. @@ -31,6 +32,6 @@ sai2: sai@40031000 { pinctrl-names = "default"; pinctrl-0 = <&pinctrl_sai2_1>; - clocks = <&clks VF610_CLK_SAI2>; - clock-names = "sai"; + clocks = <&clks VF610_CLK_SAI2>, <&clks VF610_CLK_SAI2>; + clock-names = "ipg", "sai"; dma-names = "tx", "rx"; dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>, diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 3847d2a..2d749df 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -428,5 +428,18 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai); - u32 reg; + struct device *dev = &sai->pdev->dev; + u32 reg, ret; + + ret = clk_prepare_enable(sai->ipg_clk); + if (ret) { + dev_err(dev, "failed to prepare and enable ipg clock\n"); + return ret; + } + + ret = clk_prepare_enable(sai->sai_clk); + if (ret) { + dev_err(dev, "failed to prepare and enable sai clock\n"); + goto err; + }
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) @@ -439,4 +452,9 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
return 0; + +err: + clk_disable_unprepare(sai->ipg_clk); + + return ret; }
@@ -454,4 +472,7 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream, regmap_update_bits(sai->regmap, reg, FSL_SAI_CR3_TRCE, ~FSL_SAI_CR3_TRCE); + + clk_disable_unprepare(sai->sai_clk); + clk_disable_unprepare(sai->ipg_clk); }
@@ -609,5 +630,5 @@ static int fsl_sai_probe(struct platform_device *pdev)
sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev, - "sai", base, &fsl_sai_regmap_config); + "ipg", base, &fsl_sai_regmap_config); if (IS_ERR(sai->regmap)) { dev_err(&pdev->dev, "regmap init failed\n"); @@ -615,4 +636,16 @@ static int fsl_sai_probe(struct platform_device *pdev) }
+ sai->ipg_clk = devm_clk_get(&pdev->dev, "ipg"); + if (IS_ERR(sai->ipg_clk)) { + dev_err(&pdev->dev, "failed to get ipg clock\n"); + return PTR_ERR(sai->ipg_clk); + } + + sai->sai_clk = devm_clk_get(&pdev->dev, "sai"); + if (IS_ERR(sai->sai_clk)) { + dev_err(&pdev->dev, "failed to get sai clock\n"); + return PTR_ERR(sai->sai_clk); + } + irq = platform_get_irq(pdev, 0); if (irq < 0) { diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index 677670d..cbaf114 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -127,4 +127,6 @@ struct fsl_sai { struct platform_device *pdev; struct regmap *regmap; + struct clk *ipg_clk; + struct clk *sai_clk;
bool big_endian_regs;
Hi,
I have test this series on my Vybrid-TWR board and it works happily.
[...]
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 3847d2a..2d749df 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -428,5 +428,18 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- u32 reg;
- struct device *dev = &sai->pdev->dev;
- u32 reg, ret;
I'd prefer: + int ret;
- ret = clk_prepare_enable(sai->ipg_clk);
- if (ret) {
dev_err(dev, "failed to prepare and enable ipg clock\n");return ret;- }
[...]
@@ -609,5 +630,5 @@ static int fsl_sai_probe(struct platform_device *pdev)
sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
"sai", base, &fsl_sai_regmap_config);
"ipg", base, &fsl_sai_regmap_config);@@ -615,4 +636,16 @@ static int fsl_sai_probe(struct platform_device *pdev) }
- sai->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
- if (IS_ERR(sai->ipg_clk)) {
dev_err(&pdev->dev, "failed to get ipg clock\n");return PTR_ERR(sai->ipg_clk);- }
Since the 'ipg' clock is just intend to be used for registers accessing and We are using the regmap_init_mmio_clk(), so we can just drop it here and Let the regmap APIs to do the clock options properly.
Otherwise it look good to me.
After this: Acked-by: Xiubo Li Li.Xiubo@freescale.com
Thanks,
Brs, Xiubo
- sai->sai_clk = devm_clk_get(&pdev->dev, "sai");
- if (IS_ERR(sai->sai_clk)) {
dev_err(&pdev->dev, "failed to get sai clock\n");return PTR_ERR(sai->sai_clk);- }
- irq = platform_get_irq(pdev, 0); if (irq < 0) {
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index 677670d..cbaf114 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -127,4 +127,6 @@ struct fsl_sai { struct platform_device *pdev; struct regmap *regmap;
struct clk *ipg_clk;
struct clk *sai_clk;
bool big_endian_regs;
-- 1.8.4
Hi Xiubo,
On Fri, Apr 04, 2014 at 05:24:39PM +0800, Xiubo Li-B47053 wrote:
Hi,
I have test this series on my Vybrid-TWR board and it works happily.
You just checked the wrong version. I've sent a mail to let people disregard this version and a newer v2.
[...]
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 3847d2a..2d749df 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -428,5 +428,18 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- u32 reg;
- struct device *dev = &sai->pdev->dev;
- u32 reg, ret;
I'd prefer:
- int ret;
Just like last time I said, it would be converted to 'int' any way. There's no much difference between them.
- ret = clk_prepare_enable(sai->ipg_clk);
- if (ret) {
dev_err(dev, "failed to prepare and enable ipg clock\n");return ret;- }
[...]
@@ -609,5 +630,5 @@ static int fsl_sai_probe(struct platform_device *pdev)
sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
"sai", base, &fsl_sai_regmap_config);
"ipg", base, &fsl_sai_regmap_config);@@ -615,4 +636,16 @@ static int fsl_sai_probe(struct platform_device *pdev) }
- sai->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
- if (IS_ERR(sai->ipg_clk)) {
dev_err(&pdev->dev, "failed to get ipg clock\n");return PTR_ERR(sai->ipg_clk);- }
Since the 'ipg' clock is just intend to be used for registers accessing and We are using the regmap_init_mmio_clk(), so we can just drop it here and Let the regmap APIs to do the clock options properly.
This 'ipg' clock, which I renamed in v2 to 'bus clock', is not just used in the way you said but also able to drive bit clock as your own code in the fsl_sai_set_dai_sysclk_tr(), and as reference manual describes:
52.3.1.3 Bus clock The bus clock is used by the control and configuration registers and to generate synchronous interrupts and DMA requests.
Thanks, Nicolin
Otherwise it look good to me.
After this: Acked-by: Xiubo Li Li.Xiubo@freescale.com
Thanks,
Brs, Xiubo
- sai->sai_clk = devm_clk_get(&pdev->dev, "sai");
- if (IS_ERR(sai->sai_clk)) {
dev_err(&pdev->dev, "failed to get sai clock\n");return PTR_ERR(sai->sai_clk);- }
- irq = platform_get_irq(pdev, 0); if (irq < 0) {
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index 677670d..cbaf114 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -127,4 +127,6 @@ struct fsl_sai { struct platform_device *pdev; struct regmap *regmap;
struct clk *ipg_clk;
struct clk *sai_clk;
bool big_endian_regs;
-- 1.8.4
Subject: Re: [PATCH 1/2] ASoC: fsl_sai: Add clock control for SAI
Hi Xiubo,
On Fri, Apr 04, 2014 at 05:24:39PM +0800, Xiubo Li-B47053 wrote:
Hi,
I have test this series on my Vybrid-TWR board and it works happily.
You just checked the wrong version. I've sent a mail to let people disregard this version and a newer v2.
Sorry, I didn't receive these, I will search it in the web page.
[...]
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 3847d2a..2d749df 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -428,5 +428,18 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- u32 reg;
- struct device *dev = &sai->pdev->dev;
- u32 reg, ret;
I'd prefer:
- int ret;
Just like last time I said, it would be converted to 'int' any way. There's no much difference between them.
Yes, it is...
I have ever encounter something like this in the feature maybe someone will do the following check:
If (ret < 0) ...
Just one potential problem and one suggestion.
Thanks,
- ret = clk_prepare_enable(sai->ipg_clk);
- if (ret) {
dev_err(dev, "failed to prepare and enable ipg clock\n");return ret;- }
[...]
@@ -609,5 +630,5 @@ static int fsl_sai_probe(struct platform_device *pdev)
sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
"sai", base, &fsl_sai_regmap_config);
"ipg", base, &fsl_sai_regmap_config);@@ -615,4 +636,16 @@ static int fsl_sai_probe(struct platform_device *pdev) }
- sai->ipg_clk = devm_clk_get(&pdev->dev, "ipg");
- if (IS_ERR(sai->ipg_clk)) {
dev_err(&pdev->dev, "failed to get ipg clock\n");return PTR_ERR(sai->ipg_clk);- }
Since the 'ipg' clock is just intend to be used for registers accessing and We are using the regmap_init_mmio_clk(), so we can just drop it here and Let the regmap APIs to do the clock options properly.
This 'ipg' clock, which I renamed in v2 to 'bus clock', is not just used in the way you said but also able to drive bit clock as your own code in the fsl_sai_set_dai_sysclk_tr(), and as reference manual describes:
52.3.1.3 Bus clock The bus clock is used by the control and configuration registers and to generate synchronous interrupts and DMA requests.
Thanks, Nicolin
Otherwise it look good to me.
After this: Acked-by: Xiubo Li Li.Xiubo@freescale.com
Thanks,
Brs, Xiubo
- sai->sai_clk = devm_clk_get(&pdev->dev, "sai");
- if (IS_ERR(sai->sai_clk)) {
dev_err(&pdev->dev, "failed to get sai clock\n");return PTR_ERR(sai->sai_clk);- }
- irq = platform_get_irq(pdev, 0); if (irq < 0) {
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index 677670d..cbaf114 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -127,4 +127,6 @@ struct fsl_sai { struct platform_device *pdev; struct regmap *regmap;
struct clk *ipg_clk;
struct clk *sai_clk;
bool big_endian_regs;
-- 1.8.4
On Fri, Apr 04, 2014 at 05:43:12PM +0800, Xiubo Li-B47053 wrote:
Subject: Re: [PATCH 1/2] ASoC: fsl_sai: Add clock control for SAI
Hi Xiubo,
On Fri, Apr 04, 2014 at 05:24:39PM +0800, Xiubo Li-B47053 wrote:
Hi,
I have test this series on my Vybrid-TWR board and it works happily.
You just checked the wrong version. I've sent a mail to let people disregard this version and a newer v2.
Sorry, I didn't receive these, I will search it in the web page.
[...]
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index 3847d2a..2d749df 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -428,5 +428,18 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream, { struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
- u32 reg;
- struct device *dev = &sai->pdev->dev;
- u32 reg, ret;
I'd prefer:
- int ret;
Just like last time I said, it would be converted to 'int' any way. There's no much difference between them.
Yes, it is...
I have ever encounter something like this in the feature maybe someone will do the following check:
If (ret < 0) ...
Just one potential problem and one suggestion.
Fine...I'll revise it as you wish..after you checked the v2.
Thanks, Nicolin
Since we added ipg clock to the DT binding, we should update the current SAI dts/dtsi so as not to break their functions.
Signed-off-by: Nicolin Chen Guangyu.Chen@freescale.com --- arch/arm/boot/dts/vf610.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi index d31ce1b..493c498 100644 --- a/arch/arm/boot/dts/vf610.dtsi +++ b/arch/arm/boot/dts/vf610.dtsi @@ -140,6 +140,6 @@ reg = <0x40031000 0x1000>; interrupts = <0 86 0x04>; - clocks = <&clks VF610_CLK_SAI2>; - clock-names = "sai"; + clocks = <&clks VF610_CLK_SAI2>, <&clks VF610_CLK_SAI2>; + clock-names = "ipg", "sai"; status = "disabled"; };
Please disregard this version.
SAI actually has three master clocks and one bus clock. So the patch should have added three sai_clks to the driver. I'll later send v2 to append the other two.
Thank you.
On Tue, Apr 1, 2014 at 7:52 PM, Nicolin Chen Guangyu.Chen@freescale.comwrote:
This series of patches add ipg clock control to fsl_sai driver and updates the vf610.dtsi accordingly.
@Shawn I'm not sure if VF610 currently does full works with SAI audio function. The PATCH-2 is based on broonie/for-next.
Nicolin Chen (2): ASoC: fsl_sai: Add clock control for SAI ARM: dts: Add ipg clock for sai2 on VF610 platform
.../devicetree/bindings/sound/fsl-sai.txt | 7 ++-- arch/arm/boot/dts/vf610.dtsi | 4 +-- sound/soc/fsl/fsl_sai.c | 37 ++++++++++++++++++++-- sound/soc/fsl/fsl_sai.h | 2 ++ 4 files changed, 43 insertions(+), 7 deletions(-)
-- 1.8.4
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (3)
-
Li.Xiubo＠freescale.com -
Nicolin Chen -
Nicolin Chen