[alsa-devel] [PATCH v2 1/4] ASoC: imx-sgtl5000: Do not enter the error path on success
From: Fabio Estevam fabio.estevam@freescale.com
Return on success instead of entering the error path.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Keep of_node_put in the success path.
sound/soc/fsl/imx-sgtl5000.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 9584e78..5a6aaa3 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -174,6 +174,11 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) }
platform_set_drvdata(pdev, data); + of_node_put(ssi_np); + of_node_put(codec_np); + + return 0; + clk_fail: clk_put(data->codec_clk); fail:
From: Fabio Estevam fabio.estevam@freescale.com
Currently passing a codec clock is optional.
Make the codec clock to be a required binding in order to simplify codec clock handling in imx-sgtl5000.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - Rebased against Shawn's tree and remove mx53qsb clock fix, as it was already applied on Shawn's tree - Confirmed that audio is working on mx51babbage.
arch/arm/boot/dts/imx51-babbage.dts | 13 ++++++++++++- sound/soc/fsl/imx-sgtl5000.c | 18 ++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts index 6dd9486..5318d26 100644 --- a/arch/arm/boot/dts/imx51-babbage.dts +++ b/arch/arm/boot/dts/imx51-babbage.dts @@ -61,6 +61,16 @@ mux-int-port = <2>; mux-ext-port = <3>; }; + + clocks { + clk_26M: clock { + compatible = "fixed-clock"; + reg=<0>; + #clock-cells = <0>; + clock-frequency = <26000000>; + gpios = <&gpio4 26 1>; + }; + }; };
&esdhc1 { @@ -229,6 +239,7 @@ MX51_PAD_EIM_A27__GPIO2_21 0x5 MX51_PAD_CSPI1_SS0__GPIO4_24 0x85 MX51_PAD_CSPI1_SS1__GPIO4_25 0x85 + MX51_PAD_CSPI1_RDY__GPIO4_26 0x80000000 >; }; }; @@ -255,7 +266,7 @@ sgtl5000: codec@0a { compatible = "fsl,sgtl5000"; reg = <0x0a>; - clock-frequency = <26000000>; + clocks = <&clk_26M>; VDDA-supply = <&vdig_reg>; VDDIO-supply = <&vvideo_reg>; }; diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index fa308b1..6198ca5 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -130,20 +130,14 @@ static int imx_sgtl5000_probe(struct platform_device *pdev)
data->codec_clk = clk_get(&codec_dev->dev, NULL); if (IS_ERR(data->codec_clk)) { - /* assuming clock enabled by default */ - data->codec_clk = NULL; - ret = of_property_read_u32(codec_np, "clock-frequency", - &data->clk_frequency); - if (ret) { - dev_err(&codec_dev->dev, - "clock-frequency missing or invalid\n"); - goto fail; - } - } else { - data->clk_frequency = clk_get_rate(data->codec_clk); - clk_prepare_enable(data->codec_clk); + ret = PTR_ERR(data->codec_clk); + dev_err(&codec_dev->dev, "could not get codec clk: %d\n", ret); + goto fail; }
+ data->clk_frequency = clk_get_rate(data->codec_clk); + clk_prepare_enable(data->codec_clk); + data->dai.name = "HiFi"; data->dai.stream_name = "HiFi"; data->dai.codec_dai_name = "sgtl5000";
On Wed, Apr 24, 2013 at 11:54:44AM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Currently passing a codec clock is optional.
Make the codec clock to be a required binding in order to simplify codec clock handling in imx-sgtl5000.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Changes since v1:
- Rebased against Shawn's tree and remove mx53qsb clock fix, as it was already
applied on Shawn's tree
It seems that you expect the patch to go via my tree? In that case, I need Mark's ACK.
- Confirmed that audio is working on mx51babbage.
arch/arm/boot/dts/imx51-babbage.dts | 13 ++++++++++++- sound/soc/fsl/imx-sgtl5000.c | 18 ++++++------------ 2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts index 6dd9486..5318d26 100644 --- a/arch/arm/boot/dts/imx51-babbage.dts +++ b/arch/arm/boot/dts/imx51-babbage.dts @@ -61,6 +61,16 @@ mux-int-port = <2>; mux-ext-port = <3>; };
- clocks {
clk_26M: clock {
The name "clock" is too generic.
Shawn
compatible = "fixed-clock";
reg=<0>;
#clock-cells = <0>;
clock-frequency = <26000000>;
gpios = <&gpio4 26 1>;
};
- };
};
&esdhc1 { @@ -229,6 +239,7 @@ MX51_PAD_EIM_A27__GPIO2_21 0x5 MX51_PAD_CSPI1_SS0__GPIO4_24 0x85 MX51_PAD_CSPI1_SS1__GPIO4_25 0x85
}; };MX51_PAD_CSPI1_RDY__GPIO4_26 0x80000000 >;
@@ -255,7 +266,7 @@ sgtl5000: codec@0a { compatible = "fsl,sgtl5000"; reg = <0x0a>;
clock-frequency = <26000000>;
VDDA-supply = <&vdig_reg>; VDDIO-supply = <&vvideo_reg>; };clocks = <&clk_26M>;
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index fa308b1..6198ca5 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -130,20 +130,14 @@ static int imx_sgtl5000_probe(struct platform_device *pdev)
data->codec_clk = clk_get(&codec_dev->dev, NULL); if (IS_ERR(data->codec_clk)) {
/* assuming clock enabled by default */
data->codec_clk = NULL;
ret = of_property_read_u32(codec_np, "clock-frequency",
&data->clk_frequency);
if (ret) {
dev_err(&codec_dev->dev,
"clock-frequency missing or invalid\n");
goto fail;
}
- } else {
data->clk_frequency = clk_get_rate(data->codec_clk);
clk_prepare_enable(data->codec_clk);
ret = PTR_ERR(data->codec_clk);
dev_err(&codec_dev->dev, "could not get codec clk: %d\n", ret);
goto fail;
}
data->clk_frequency = clk_get_rate(data->codec_clk);
clk_prepare_enable(data->codec_clk);
data->dai.name = "HiFi"; data->dai.stream_name = "HiFi"; data->dai.codec_dai_name = "sgtl5000";
-- 1.7.9.5
On Thu, Apr 25, 2013 at 01:16:55PM +0800, Shawn Guo wrote:
On Wed, Apr 24, 2013 at 11:54:44AM -0300, Fabio Estevam wrote:
- Rebased against Shawn's tree and remove mx53qsb clock fix, as it was already
applied on Shawn's tree
It seems that you expect the patch to go via my tree? In that case, I need Mark's ACK.
Well, it's a bit late for ARM changes to go in so I guess it'll be sorted out after the merge window anyway and can be applied to the ASoC tree after that?
On Thu, Apr 25, 2013 at 12:01:27PM +0100, Mark Brown wrote:
On Thu, Apr 25, 2013 at 01:16:55PM +0800, Shawn Guo wrote:
On Wed, Apr 24, 2013 at 11:54:44AM -0300, Fabio Estevam wrote:
- Rebased against Shawn's tree and remove mx53qsb clock fix, as it was already
applied on Shawn's tree
It seems that you expect the patch to go via my tree? In that case, I need Mark's ACK.
Well, it's a bit late for ARM changes to go in so I guess it'll be sorted out after the merge window anyway and can be applied to the ASoC tree after that?
Even worse. The dependant patches in my tree are 3.11 material.
Shawn
On Thu, Apr 25, 2013 at 10:35:24PM +0800, Shawn Guo wrote:
On Thu, Apr 25, 2013 at 12:01:27PM +0100, Mark Brown wrote:
On Thu, Apr 25, 2013 at 01:16:55PM +0800, Shawn Guo wrote:
On Wed, Apr 24, 2013 at 11:54:44AM -0300, Fabio Estevam wrote:
- Rebased against Shawn's tree and remove mx53qsb clock fix, as it was already
applied on Shawn's tree
It seems that you expect the patch to go via my tree? In that case, I need Mark's ACK.
Well, it's a bit late for ARM changes to go in so I guess it'll be sorted out after the merge window anyway and can be applied to the ASoC tree after that?
Even worse. The dependant patches in my tree are 3.11 material.
Oh, that's distressing - I guess there's more than just the fix mentioned above? Would it be possible for you to provide a branch I can pull into my tree for v3.11? My workflow is easier if I don't have to worry about having potential collisions with other trees. If it'd be too inconvenient for you then I can cope but it'd be nice.
On Wed, May 01, 2013 at 05:53:01PM +0100, Mark Brown wrote:
On Thu, Apr 25, 2013 at 10:35:24PM +0800, Shawn Guo wrote:
On Thu, Apr 25, 2013 at 12:01:27PM +0100, Mark Brown wrote:
On Thu, Apr 25, 2013 at 01:16:55PM +0800, Shawn Guo wrote:
On Wed, Apr 24, 2013 at 11:54:44AM -0300, Fabio Estevam wrote:
- Rebased against Shawn's tree and remove mx53qsb clock fix, as it was already
applied on Shawn's tree
It seems that you expect the patch to go via my tree? In that case, I need Mark's ACK.
Well, it's a bit late for ARM changes to go in so I guess it'll be sorted out after the merge window anyway and can be applied to the ASoC tree after that?
Even worse. The dependant patches in my tree are 3.11 material.
Oh, that's distressing - I guess there's more than just the fix mentioned above?
Yes, a couple of patches.
Would it be possible for you to provide a branch I can pull into my tree for v3.11?
Ok, I will try to once we have a -rc to be the base.
Shawn
My workflow is easier if I don't have to worry about having potential collisions with other trees. If it'd be too inconvenient for you then I can cope but it'd be nice.
From: Fabio Estevam fabio.estevam@freescale.com
Only turn on the codec clock if it is within the valid range.
Also, disable the codec clock on the clk_fail path.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - None
sound/soc/fsl/imx-sgtl5000.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 9c286e6..18af815 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -56,6 +56,15 @@ static const struct snd_soc_dapm_widget imx_sgtl5000_dapm_widgets[] = { SND_SOC_DAPM_SPK("Ext Spk", NULL), };
+static int sgtl5000_is_valid_sysclk(int freq) +{ + + if (freq < 8000000 || freq > 27000000) + return -EINVAL; + else + return 0; +} + static int imx_sgtl5000_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -135,9 +144,13 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) goto fail; }
- data->clk_frequency = clk_get_rate(data->codec_clk); - clk_prepare_enable(data->codec_clk); + data->clk_frequency = clk_get_rate(data->codec_clk); + ret = sgtl5000_is_valid_sysclk(data->clk_frequency); + if (ret) + goto fail;
+ clk_prepare_enable(data->codec_clk); + data->dai.name = "HiFi"; data->dai.stream_name = "HiFi"; data->dai.codec_dai_name = "sgtl5000"; @@ -172,6 +185,7 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) return 0;
clk_fail: + clk_disable_unprepare(data->codec_clk); clk_put(data->codec_clk); fail: if (ssi_np)
On Wed, Apr 24, 2013 at 11:54:45AM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Only turn on the codec clock if it is within the valid range.
Also, disable the codec clock on the clk_fail path.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Changes since v1:
- None
sound/soc/fsl/imx-sgtl5000.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 9c286e6..18af815 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -56,6 +56,15 @@ static const struct snd_soc_dapm_widget imx_sgtl5000_dapm_widgets[] = { SND_SOC_DAPM_SPK("Ext Spk", NULL), };
+static int sgtl5000_is_valid_sysclk(int freq) +{
- if (freq < 8000000 || freq > 27000000)
return -EINVAL;
- else
return 0;
+}
static int imx_sgtl5000_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; @@ -135,9 +144,13 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) goto fail; }
- data->clk_frequency = clk_get_rate(data->codec_clk);
- clk_prepare_enable(data->codec_clk);
- data->clk_frequency = clk_get_rate(data->codec_clk);
- ret = sgtl5000_is_valid_sysclk(data->clk_frequency);
- if (ret)
goto fail;
In this patch, you haven't converted clk_get to devm_clk_get yet. If you goto "fail" here, clk_put() below will be skipped.
Shawn
- clk_prepare_enable(data->codec_clk);
- data->dai.name = "HiFi"; data->dai.stream_name = "HiFi"; data->dai.codec_dai_name = "sgtl5000";
@@ -172,6 +185,7 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) return 0;
clk_fail:
- clk_disable_unprepare(data->codec_clk); clk_put(data->codec_clk);
fail: if (ssi_np) -- 1.7.9.5
From: Fabio Estevam fabio.estevam@freescale.com
Converting to devm_clk_get() can simplify the code a bit.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Changes since v1: - None
sound/soc/fsl/imx-sgtl5000.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 18af815..c869996 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -137,7 +137,7 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) goto fail; }
- data->codec_clk = clk_get(&codec_dev->dev, NULL); + data->codec_clk = devm_clk_get(&codec_dev->dev, NULL); if (IS_ERR(data->codec_clk)) { ret = PTR_ERR(data->codec_clk); dev_err(&codec_dev->dev, "could not get codec clk: %d\n", ret); @@ -186,7 +186,6 @@ static int imx_sgtl5000_probe(struct platform_device *pdev)
clk_fail: clk_disable_unprepare(data->codec_clk); - clk_put(data->codec_clk); fail: if (ssi_np) of_node_put(ssi_np); @@ -200,10 +199,7 @@ static int imx_sgtl5000_remove(struct platform_device *pdev) { struct imx_sgtl5000_data *data = platform_get_drvdata(pdev);
- if (data->codec_clk) { - clk_disable_unprepare(data->codec_clk); - clk_put(data->codec_clk); - } + clk_disable_unprepare(data->codec_clk); snd_soc_unregister_card(&data->card);
return 0;
On Wed, Apr 24, 2013 at 11:54:46AM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Converting to devm_clk_get() can simplify the code a bit.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Acked-by: Shawn Guo shawn.guo@linaro.org
On Wed, Apr 24, 2013 at 11:54:43AM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Return on success instead of entering the error path.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Acked-by: Shawn Guo shawn.guo@linaro.org
Changes since v1:
- Keep of_node_put in the success path.
sound/soc/fsl/imx-sgtl5000.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c index 9584e78..5a6aaa3 100644 --- a/sound/soc/fsl/imx-sgtl5000.c +++ b/sound/soc/fsl/imx-sgtl5000.c @@ -174,6 +174,11 @@ static int imx_sgtl5000_probe(struct platform_device *pdev) }
platform_set_drvdata(pdev, data);
- of_node_put(ssi_np);
- of_node_put(codec_np);
- return 0;
clk_fail: clk_put(data->codec_clk); fail: -- 1.7.9.5
On Wed, Apr 24, 2013 at 11:54:43AM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
Return on success instead of entering the error path.
Applied, thanks.
participants (3)
-
Fabio Estevam
-
Mark Brown
-
Shawn Guo