* [patch] ASoC: sun4i-i2s: remove some dead code
@ 2017-02-07 13:19 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2017-02-07 13:19 UTC (permalink / raw)
To: Liam Girdwood, Mylène Josserand
Cc: alsa-devel, Wei Yongjun, kernel-janitors, Takashi Iwai,
Chen-Yu Tsai, Mark Brown, Maxime Ripard
"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;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread* [patch] ASoC: sun4i-i2s: remove some dead code
@ 2017-02-07 13:19 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2017-02-07 13:19 UTC (permalink / raw)
To: Liam Girdwood, Mylène Josserand
Cc: alsa-devel, Wei Yongjun, kernel-janitors, Takashi Iwai,
Chen-Yu Tsai, Mark Brown, Maxime Ripard
"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;
}
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
2017-02-07 13:19 ` Dan Carpenter
@ 2017-02-07 13:42 ` Maxime Ripard
-1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2017-02-07 13:42 UTC (permalink / raw)
To: Dan Carpenter
Cc: Liam Girdwood, Mylène Josserand, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai, Wei Yongjun,
alsa-devel, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]
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
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
@ 2017-02-07 13:42 ` Maxime Ripard
0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2017-02-07 13:42 UTC (permalink / raw)
To: Dan Carpenter
Cc: Liam Girdwood, Mylène Josserand, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai, Wei Yongjun,
alsa-devel, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]
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
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
2017-02-07 13:42 ` Maxime Ripard
@ 2017-02-07 13:50 ` Dan Carpenter
-1 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2017-02-07 13:50 UTC (permalink / raw)
To: Maxime Ripard
Cc: alsa-devel, Wei Yongjun, kernel-janitors, Takashi Iwai,
Liam Girdwood, Chen-Yu Tsai, Mark Brown, Mylène Josserand
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
@ 2017-02-07 13:50 ` Dan Carpenter
0 siblings, 0 replies; 10+ messages in thread
From: Dan Carpenter @ 2017-02-07 13:50 UTC (permalink / raw)
To: Maxime Ripard
Cc: alsa-devel, Wei Yongjun, kernel-janitors, Takashi Iwai,
Liam Girdwood, Chen-Yu Tsai, Mark Brown, Mylène Josserand
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
2017-02-07 13:50 ` Dan Carpenter
@ 2017-02-08 8:58 ` Maxime Ripard
-1 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2017-02-08 8:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: Liam Girdwood, Mylène Josserand, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai, Wei Yongjun,
alsa-devel, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]
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/reset/core.c#n217
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
@ 2017-02-08 8:58 ` Maxime Ripard
0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2017-02-08 8:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: Liam Girdwood, Mylène Josserand, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai, Wei Yongjun,
alsa-devel, kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]
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/reset/core.c#n217
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
2017-02-08 8:58 ` Maxime Ripard
@ 2017-02-08 9:01 ` Chen-Yu Tsai
-1 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2017-02-08 9:01 UTC (permalink / raw)
To: Maxime Ripard
Cc: Linux-ALSA, Wei Yongjun, kernel-janitors, Takashi Iwai,
Liam Girdwood, Chen-Yu Tsai, Mark Brown, Mylène Josserand,
Dan Carpenter
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/reset/core.c#n217
The changes were just pulled into arm-soc. I suggest waiting for -rc1 at least
before merging this change.
ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [patch] ASoC: sun4i-i2s: remove some dead code
@ 2017-02-08 9:01 ` Chen-Yu Tsai
0 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2017-02-08 9:01 UTC (permalink / raw)
To: Maxime Ripard
Cc: Linux-ALSA, Wei Yongjun, kernel-janitors, Takashi Iwai,
Liam Girdwood, Chen-Yu Tsai, Mark Brown, Mylène Josserand,
Dan Carpenter
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/reset/core.c#n217
The changes were just pulled into arm-soc. I suggest waiting for -rc1 at least
before merging this change.
ChenYu
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-08 9:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-07 13:19 [patch] ASoC: sun4i-i2s: remove some dead code Dan Carpenter
2017-02-07 13:19 ` Dan Carpenter
2017-02-07 13:42 ` Maxime Ripard
2017-02-07 13:42 ` Maxime Ripard
2017-02-07 13:50 ` Dan Carpenter
2017-02-07 13:50 ` Dan Carpenter
2017-02-08 8:58 ` Maxime Ripard
2017-02-08 8:58 ` Maxime Ripard
2017-02-08 9:01 ` Chen-Yu Tsai
2017-02-08 9:01 ` Chen-Yu Tsai
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.