[alsa-devel] [patch] ASoC: sun4i-i2s: remove some dead code
"i2s->rst" is either NULL or a valid pointer. We won't probe successfully if it's an error pointer. That means these checks can be removed.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 3635bbc72cbc..d79be3e4361b 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -723,13 +723,11 @@ static int sun4i_i2s_probe(struct platform_device *pdev) } }
- if (!IS_ERR(i2s->rst)) { - ret = reset_control_deassert(i2s->rst); - if (ret) { - dev_err(&pdev->dev, - "Failed to deassert the reset control\n"); - return -EINVAL; - } + ret = reset_control_deassert(i2s->rst); + if (ret) { + dev_err(&pdev->dev, + "Failed to deassert the reset control\n"); + return -EINVAL; }
i2s->playback_dma_data.addr = res->start + SUN4I_I2S_FIFO_TX_REG; @@ -766,8 +764,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev) sun4i_i2s_runtime_suspend(&pdev->dev); err_pm_disable: pm_runtime_disable(&pdev->dev); - if (!IS_ERR(i2s->rst)) - reset_control_assert(i2s->rst); + reset_control_assert(i2s->rst);
return ret; } @@ -782,8 +779,7 @@ static int sun4i_i2s_remove(struct platform_device *pdev) if (!pm_runtime_status_suspended(&pdev->dev)) sun4i_i2s_runtime_suspend(&pdev->dev);
- if (!IS_ERR(i2s->rst)) - reset_control_assert(i2s->rst); + reset_control_assert(i2s->rst);
return 0; }
Hi Dan,
On Tue, Feb 07, 2017 at 04:19:34PM +0300, Dan Carpenter wrote:
"i2s->rst" is either NULL or a valid pointer. We won't probe successfully if it's an error pointer. That means these checks can be removed.
Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c index 3635bbc72cbc..d79be3e4361b 100644 --- a/sound/soc/sunxi/sun4i-i2s.c +++ b/sound/soc/sunxi/sun4i-i2s.c @@ -723,13 +723,11 @@ static int sun4i_i2s_probe(struct platform_device *pdev) } }
- if (!IS_ERR(i2s->rst)) {
ret = reset_control_deassert(i2s->rst);if (ret) {dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;}
- ret = reset_control_deassert(i2s->rst);
- if (ret) {
dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;
In the case where has_reset is false, rst is NULL and will trigger a WARN_ON in reset_control_deassert.
The proper fix would be to move it in the previous if block, or to change the IS_ERR check for a NULL check.
Maxime
On Tue, Feb 07, 2017 at 02:42:15PM +0100, Maxime Ripard wrote:
- if (!IS_ERR(i2s->rst)) {
ret = reset_control_deassert(i2s->rst);if (ret) {dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;}
- ret = reset_control_deassert(i2s->rst);
- if (ret) {
dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;In the case where has_reset is false, rst is NULL and will trigger a WARN_ON in reset_control_deassert.
No it won't. reset_control_deassert(NULL) just returns success immediately.
The proper fix would be to move it in the previous if block, or to change the IS_ERR check for a NULL check.
We could move it as a clean up but the current code works. Checking for NULL is wrong.
regards, dan carpenter
On Tue, Feb 07, 2017 at 04:50:08PM +0300, Dan Carpenter wrote:
On Tue, Feb 07, 2017 at 02:42:15PM +0100, Maxime Ripard wrote:
- if (!IS_ERR(i2s->rst)) {
ret = reset_control_deassert(i2s->rst);if (ret) {dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;}
- ret = reset_control_deassert(i2s->rst);
- if (ret) {
dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;In the case where has_reset is false, rst is NULL and will trigger a WARN_ON in reset_control_deassert.
No it won't. reset_control_deassert(NULL) just returns success immediately.
Not in current Linus' tree at least: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers...
Maxime
On Wed, Feb 8, 2017 at 4:58 PM, Maxime Ripard maxime.ripard@free-electrons.com wrote:
On Tue, Feb 07, 2017 at 04:50:08PM +0300, Dan Carpenter wrote:
On Tue, Feb 07, 2017 at 02:42:15PM +0100, Maxime Ripard wrote:
- if (!IS_ERR(i2s->rst)) {
ret = reset_control_deassert(i2s->rst);if (ret) {dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;}
- ret = reset_control_deassert(i2s->rst);
- if (ret) {
dev_err(&pdev->dev,"Failed to deassert the reset control\n");return -EINVAL;In the case where has_reset is false, rst is NULL and will trigger a WARN_ON in reset_control_deassert.
No it won't. reset_control_deassert(NULL) just returns success immediately.
Not in current Linus' tree at least: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers...
The changes were just pulled into arm-soc. I suggest waiting for -rc1 at least before merging this change.
ChenYu
participants (3)
-
Chen-Yu Tsai -
Dan Carpenter -
Maxime Ripard