[alsa-devel] [bug report] ASoC: sti: Add CPU DAI driver for playback
Hello Arnaud Pouliquen,
The patch 76c2145ded6b: "ASoC: sti: Add CPU DAI driver for playback" from Jun 22, 2015, leads to the following static checker warning:
sound/soc/sti/uniperif_player.c:1077 uni_player_init() warn: unused return: ret = PTR_ERR()
sound/soc/sti/uniperif_player.c 1047 int uni_player_init(struct platform_device *pdev, 1048 struct uniperif *player) 1049 { 1050 int ret = 0; 1051 1052 player->dev = &pdev->dev; 1053 player->state = UNIPERIF_STATE_STOPPED; 1054 player->dai_ops = &uni_player_dai_ops; 1055 1056 /* Get PCM_CLK_SEL & PCMP_VALID_SEL from audio-glue-ctrl SoC reg */ 1057 ret = uni_player_parse_dt_audio_glue(pdev, player); 1058 1059 if (ret < 0) { 1060 dev_err(player->dev, "Failed to parse DeviceTree\n"); 1061 return ret; 1062 } 1063 1064 /* Underflow recovery is only supported on later ip revisions */ 1065 if (player->ver >= SND_ST_UNIPERIF_VERSION_UNI_PLR_TOP_1_0) 1066 player->underflow_enabled = 1; 1067 1068 if (UNIPERIF_TYPE_IS_TDM(player)) 1069 player->hw = &uni_tdm_hw; 1070 else 1071 player->hw = &uni_player_pcm_hw; 1072 1073 /* Get uniperif resource */ 1074 player->clk = of_clk_get(pdev->dev.of_node, 0); 1075 if (IS_ERR(player->clk)) { 1076 dev_err(player->dev, "Failed to get clock\n"); 1077 ret = PTR_ERR(player->clk); ^^^^^^^^^^^^^^^^^^^^^^^^^^ This should probably be "return PTR_ERR(player->clk);? Can we run without a clk?
1078 } 1079 1080 /* Select the frequency synthesizer clock */ 1081 if (player->clk_sel) { 1082 ret = regmap_field_write(player->clk_sel, 1); 1083 if (ret) { 1084 dev_err(player->dev, 1085 "%s: Failed to select freq synth clock\n", 1086 __func__); 1087 return ret; 1088 } 1089 }
regards, dan carpenter
Hi Dan,
On 04/28/2017 01:28 PM, Dan Carpenter wrote:
Hello Arnaud Pouliquen,
The patch 76c2145ded6b: "ASoC: sti: Add CPU DAI driver for playback" from Jun 22, 2015, leads to the following static checker warning:
sound/soc/sti/uniperif_player.c:1077 uni_player_init() warn: unused return: ret = PTR_ERR()sound/soc/sti/uniperif_player.c 1047 int uni_player_init(struct platform_device *pdev, 1048 struct uniperif *player) 1049 { 1050 int ret = 0; 1051 1052 player->dev = &pdev->dev; 1053 player->state = UNIPERIF_STATE_STOPPED; 1054 player->dai_ops = &uni_player_dai_ops; 1055 1056 /* Get PCM_CLK_SEL & PCMP_VALID_SEL from audio-glue-ctrl SoC reg */ 1057 ret = uni_player_parse_dt_audio_glue(pdev, player); 1058 1059 if (ret < 0) { 1060 dev_err(player->dev, "Failed to parse DeviceTree\n"); 1061 return ret; 1062 } 1063 1064 /* Underflow recovery is only supported on later ip revisions */ 1065 if (player->ver >= SND_ST_UNIPERIF_VERSION_UNI_PLR_TOP_1_0) 1066 player->underflow_enabled = 1; 1067 1068 if (UNIPERIF_TYPE_IS_TDM(player)) 1069 player->hw = &uni_tdm_hw; 1070 else 1071 player->hw = &uni_player_pcm_hw; 1072 1073 /* Get uniperif resource */ 1074 player->clk = of_clk_get(pdev->dev.of_node, 0); 1075 if (IS_ERR(player->clk)) { 1076 dev_err(player->dev, "Failed to get clock\n"); 1077 ret = PTR_ERR(player->clk); ^^^^^^^^^^^^^^^^^^^^^^^^^^ This should probably be "return PTR_ERR(player->clk);? Can we run without a clk?
Yes you are right, thanks for highlighting the issue. the clock is mandatory.
Don't hesitate to propose the patch, i will ack it. Or tell me if you prefer that i fix it on my side...
Regards Arnaud
We intended to return here. The current code has a static checker warning because we set "ret" but don't use it.
Fixes: 76c2145ded6b ("ASoC: sti: Add CPU DAI driver for playback") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c index d7e8dd46d2cc..d8b6936e544e 100644 --- a/sound/soc/sti/uniperif_player.c +++ b/sound/soc/sti/uniperif_player.c @@ -1074,7 +1074,7 @@ int uni_player_init(struct platform_device *pdev, player->clk = of_clk_get(pdev->dev.of_node, 0); if (IS_ERR(player->clk)) { dev_err(player->dev, "Failed to get clock\n"); - ret = PTR_ERR(player->clk); + return PTR_ERR(player->clk); }
/* Select the frequency synthesizer clock */
Hi Dan,
Acked-by: Arnaud POULIQUEN arnaud.pouliquen@st.com
Thanks!
Arnaud
On 04/28/2017 03:22 PM, Dan Carpenter wrote:
We intended to return here. The current code has a static checker warning because we set "ret" but don't use it.
Fixes: 76c2145ded6b ("ASoC: sti: Add CPU DAI driver for playback") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c index d7e8dd46d2cc..d8b6936e544e 100644 --- a/sound/soc/sti/uniperif_player.c +++ b/sound/soc/sti/uniperif_player.c @@ -1074,7 +1074,7 @@ int uni_player_init(struct platform_device *pdev, player->clk = of_clk_get(pdev->dev.of_node, 0); if (IS_ERR(player->clk)) { dev_err(player->dev, "Failed to get clock\n");
ret = PTR_ERR(player->clk);
return PTR_ERR(player->clk);}
/* Select the frequency synthesizer clock */
The patch
ASoC: sti: Fix error handling if of_clk_get() fails
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 65ed0a8d1f24abd79be149253025de8949321900 Mon Sep 17 00:00:00 2001
From: Dan Carpenter dan.carpenter@oracle.com Date: Fri, 28 Apr 2017 16:22:10 +0300 Subject: [PATCH] ASoC: sti: Fix error handling if of_clk_get() fails
We intended to return here. The current code has a static checker warning because we set "ret" but don't use it.
Fixes: 76c2145ded6b ("ASoC: sti: Add CPU DAI driver for playback") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com Acked-by: Arnaud POULIQUEN arnaud.pouliquen@st.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/sti/uniperif_player.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c index d7e8dd46d2cc..d8b6936e544e 100644 --- a/sound/soc/sti/uniperif_player.c +++ b/sound/soc/sti/uniperif_player.c @@ -1074,7 +1074,7 @@ int uni_player_init(struct platform_device *pdev, player->clk = of_clk_get(pdev->dev.of_node, 0); if (IS_ERR(player->clk)) { dev_err(player->dev, "Failed to get clock\n"); - ret = PTR_ERR(player->clk); + return PTR_ERR(player->clk); }
/* Select the frequency synthesizer clock */
participants (3)
-
Arnaud Pouliquen -
Dan Carpenter -
Mark Brown