Fix the below ignoring return value warning for device_reset.
drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
of function declared with 'warn_unused_result' attribute [-Wunused-result]
device_reset(&pdev->dev);
^~~~~~~~~~~~ ~~~~~~~~~~
drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
of function declared with 'warn_unused_result' attribute [-Wunused-result]
device_reset(&pdev->dev);
^~~~~~~~~~~~ ~~~~~~~~~~
Signed-off-by: Youling Tang <[email protected]>
---
drivers/staging/mt7621-dma/mtk-hsdma.c | 6 +++++-
drivers/staging/ralink-gdma/ralink-gdma.c | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c b/drivers/staging/mt7621-dma/mtk-hsdma.c
index bc4bb43..d4ffa52 100644
--- a/drivers/staging/mt7621-dma/mtk-hsdma.c
+++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
@@ -682,7 +682,11 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
return ret;
}
- device_reset(&pdev->dev);
+ ret = device_reset(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to reset device\n");
+ return ret;
+ }
dd = &hsdma->ddev;
dma_cap_set(DMA_MEMCPY, dd->cap_mask);
diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c b/drivers/staging/ralink-gdma/ralink-gdma.c
index 655df31..df99c47 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -833,7 +833,11 @@ static int gdma_dma_probe(struct platform_device *pdev)
return ret;
}
- device_reset(&pdev->dev);
+ ret = device_reset(&pdev->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to reset device\n");
+ return ret;
+ }
dd = &dma_dev->ddev;
dma_cap_set(DMA_MEMCPY, dd->cap_mask);
--
2.1.0
On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> Fix the below ignoring return value warning for device_reset.
>
> drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(&pdev->dev);
> ^~~~~~~~~~~~ ~~~~~~~~~~
> drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(&pdev->dev);
> ^~~~~~~~~~~~ ~~~~~~~~~~
>
> Signed-off-by: Youling Tang <[email protected]>
> ---
> drivers/staging/mt7621-dma/mtk-hsdma.c | 6 +++++-
> drivers/staging/ralink-gdma/ralink-gdma.c | 6 +++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c b/drivers/staging/mt7621-dma/mtk-hsdma.c
> index bc4bb43..d4ffa52 100644
> --- a/drivers/staging/mt7621-dma/mtk-hsdma.c
> +++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
> @@ -682,7 +682,11 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
> return ret;
> }
>
> - device_reset(&pdev->dev);
> + ret = device_reset(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to reset device\n");
> + return ret;
> + }
>
> dd = &hsdma->ddev;
> dma_cap_set(DMA_MEMCPY, dd->cap_mask);
> diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c b/drivers/staging/ralink-gdma/ralink-gdma.c
> index 655df31..df99c47 100644
> --- a/drivers/staging/ralink-gdma/ralink-gdma.c
> +++ b/drivers/staging/ralink-gdma/ralink-gdma.c
> @@ -833,7 +833,11 @@ static int gdma_dma_probe(struct platform_device *pdev)
> return ret;
> }
>
> - device_reset(&pdev->dev);
> + ret = device_reset(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to reset device\n");
> + return ret;
> + }
>
> dd = &dma_dev->ddev;
> dma_cap_set(DMA_MEMCPY, dd->cap_mask);
> --
> 2.1.0
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- Your patch did not apply to any known trees that Greg is in control
of. Possibly this is because you made it against Linus's tree, not
the linux-next tree, which is where all of the development for the
next version of the kernel is at. Please refresh your patch against
the linux-next tree, or even better yet, the development tree
specified in the MAINTAINERS file for the subsystem you are submitting
a patch for, and resend it.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> Fix the below ignoring return value warning for device_reset.
>
> drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(&pdev->dev);
> ^~~~~~~~~~~~ ~~~~~~~~~~
> drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(&pdev->dev);
> ^~~~~~~~~~~~ ~~~~~~~~~~
>
We can't really do this sort of fix without the hardware to test it.
This could be the correct fix or perhaps switching to device_reset_optional()
is the correct fix. We can't know unless we have the hardware to test.
People think silencing warnings is good, but it's actually bad. The
warning is there to show us a potential bug. If we silence the warning
without fixing the bug, then we it's like when your mom tells you to
clean up the room and you instead just switch off the light. It doesn't
fix the problem, it only makes it harder to find.
regards,
dan carpenter
On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> Fix the below ignoring return value warning for device_reset.
>
> drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(&pdev->dev);
> ^~~~~~~~~~~~ ~~~~~~~~~~
> drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
> of function declared with 'warn_unused_result' attribute [-Wunused-result]
> device_reset(&pdev->dev);
> ^~~~~~~~~~~~ ~~~~~~~~~~
>
> Signed-off-by: Youling Tang <[email protected]>
> ---
> drivers/staging/mt7621-dma/mtk-hsdma.c | 6 +++++-
> drivers/staging/ralink-gdma/ralink-gdma.c | 6 +++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c b/drivers/staging/mt7621-dma/mtk-hsdma.c
> index bc4bb43..d4ffa52 100644
> --- a/drivers/staging/mt7621-dma/mtk-hsdma.c
> +++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
> @@ -682,7 +682,11 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
> return ret;
> }
>
> - device_reset(&pdev->dev);
> + ret = device_reset(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to reset device\n");
> + return ret;
> + }
Normally you don't want to see this error message when -EPROBE_DEFER is
returned because that would mean the reset controller is not yet
available and the driver should probe later again. There is
dev_err_probe() now for exactly this usecase.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Dan,
On Mon, Feb 08, 2021 at 04:45:17PM +0300, Dan Carpenter wrote:
> On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> > Fix the below ignoring return value warning for device_reset.
> >
> > drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > device_reset(&pdev->dev);
> > ^~~~~~~~~~~~ ~~~~~~~~~~
> > drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
> > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > device_reset(&pdev->dev);
> > ^~~~~~~~~~~~ ~~~~~~~~~~
> >
>
> We can't really do this sort of fix without the hardware to test it.
> This could be the correct fix or perhaps switching to device_reset_optional()
> is the correct fix. We can't know unless we have the hardware to test.
When device_reset() is the wrong function then adding a return value
check will turn this into a runtime error for those who have the
hardware which will hopefully trigger them to tell us why reset_device
is wrong for them.
At least for a staging driver I find this procedure opportune.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Mon, Feb 08, 2021 at 04:06:18PM +0100, Sascha Hauer wrote:
> Hi Dan,
>
> On Mon, Feb 08, 2021 at 04:45:17PM +0300, Dan Carpenter wrote:
> > On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> > > Fix the below ignoring return value warning for device_reset.
> > >
> > > drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> > > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > > device_reset(&pdev->dev);
> > > ^~~~~~~~~~~~ ~~~~~~~~~~
> > > drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
> > > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > > device_reset(&pdev->dev);
> > > ^~~~~~~~~~~~ ~~~~~~~~~~
> > >
> >
> > We can't really do this sort of fix without the hardware to test it.
> > This could be the correct fix or perhaps switching to device_reset_optional()
> > is the correct fix. We can't know unless we have the hardware to test.
>
> When device_reset() is the wrong function then adding a return value
> check will turn this into a runtime error for those who have the
> hardware which will hopefully trigger them to tell us why reset_device
> is wrong for them.
> At least for a staging driver I find this procedure opportune.
>
That seems like sort of a jerk move... What's the rush? Someone will
eventually be able to test this if we just wait around for a bit.
Otherwise if no one has the hardware then eventually the driver will be
deleted.
regards,
dan carpenter
Hi, Dan
On 02/09/2021 03:02 AM, Dan Carpenter wrote:
> On Mon, Feb 08, 2021 at 04:06:18PM +0100, Sascha Hauer wrote:
>> Hi Dan,
>>
>> On Mon, Feb 08, 2021 at 04:45:17PM +0300, Dan Carpenter wrote:
>>> On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
>>>> Fix the below ignoring return value warning for device_reset.
>>>>
>>>> drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
>>>> of function declared with 'warn_unused_result' attribute [-Wunused-result]
>>>> device_reset(&pdev->dev);
>>>> ^~~~~~~~~~~~ ~~~~~~~~~~
>>>> drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
>>>> of function declared with 'warn_unused_result' attribute [-Wunused-result]
>>>> device_reset(&pdev->dev);
>>>> ^~~~~~~~~~~~ ~~~~~~~~~~
>>>>
>>> We can't really do this sort of fix without the hardware to test it.
>>> This could be the correct fix or perhaps switching to device_reset_optional()
>>> is the correct fix. We can't know unless we have the hardware to test.
>> When device_reset() is the wrong function then adding a return value
>> check will turn this into a runtime error for those who have the
>> hardware which will hopefully trigger them to tell us why reset_device
>> is wrong for them.
>> At least for a staging driver I find this procedure opportune.
>>
> That seems like sort of a jerk move... What's the rush? Someone will
> eventually be able to test this if we just wait around for a bit.
> Otherwise if no one has the hardware then eventually the driver will be
> deleted.
>
> regards,
> dan carpenter
We do not have the relevant hardware to test, this is just to solve a
compile-time warning.
Thanks,
Youling.
On Tue, Feb 09, 2021 at 09:18:02AM +0800, Youling Tang wrote:
> Hi, Dan
>
>
> On 02/09/2021 03:02 AM, Dan Carpenter wrote:
> > On Mon, Feb 08, 2021 at 04:06:18PM +0100, Sascha Hauer wrote:
> > > Hi Dan,
> > >
> > > On Mon, Feb 08, 2021 at 04:45:17PM +0300, Dan Carpenter wrote:
> > > > On Sun, Feb 07, 2021 at 05:23:28PM +0800, Youling Tang wrote:
> > > > > Fix the below ignoring return value warning for device_reset.
> > > > >
> > > > > drivers/staging/mt7621-dma/mtk-hsdma.c:685:2: warning: ignoring return value
> > > > > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > > > > device_reset(&pdev->dev);
> > > > > ^~~~~~~~~~~~ ~~~~~~~~~~
> > > > > drivers/staging/ralink-gdma/ralink-gdma.c:836:2: warning: ignoring return value
> > > > > of function declared with 'warn_unused_result' attribute [-Wunused-result]
> > > > > device_reset(&pdev->dev);
> > > > > ^~~~~~~~~~~~ ~~~~~~~~~~
> > > > >
> > > > We can't really do this sort of fix without the hardware to test it.
> > > > This could be the correct fix or perhaps switching to device_reset_optional()
> > > > is the correct fix. We can't know unless we have the hardware to test.
> > > When device_reset() is the wrong function then adding a return value
> > > check will turn this into a runtime error for those who have the
> > > hardware which will hopefully trigger them to tell us why reset_device
> > > is wrong for them.
> > > At least for a staging driver I find this procedure opportune.
> > >
> > That seems like sort of a jerk move... What's the rush? Someone will
> > eventually be able to test this if we just wait around for a bit.
> > Otherwise if no one has the hardware then eventually the driver will be
> > deleted.
> >
> > regards,
> > dan carpenter
> We do not have the relevant hardware to test, this is just to solve a
> compile-time warning.
Yeah, I know. Wait for someone who can test it on this. Normally we
don't require testing for staging patches. But in this case, it's
impossible to know which is the proper fix without testing so we should
wait. It's rude to test by breaking people's systems and hope they
report the bug.
regards,
dan carpenter