diff mbox series

ARM: dove: fix returnvar.cocci warnings

Message ID YnCXTPrbLhvfRVDm@e3a974050dc4
State New
Headers show
Series ARM: dove: fix returnvar.cocci warnings | expand

Commit Message

kernel test robot May 3, 2022, 2:45 a.m. UTC
From: kernel test robot <lkp@intel.com>

arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161


 Remove unneeded variable used to store return value.

Generated by: scripts/coccinelle/misc/returnvar.cocci

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: kernel test robot <lkp@intel.com>
---

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   9f9b9a2972eb8dcaad09d826c5c6d7488eaca3e6
commit: 09f6b27d5ddd9ad0ec096d1b0f8decdacc70f0f8 [1066/8035] ARM: dove: multiplatform support
:::::: branch date: 15 hours ago
:::::: commit date: 4 weeks ago

 arch/arm/mach-omap2/dma.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Arnd Bergmann May 3, 2022, 7:21 a.m. UTC | #1
On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
>
> From: kernel test robot <lkp@intel.com>
>
> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
>
>
>  Remove unneeded variable used to store return value.
>
> Generated by: scripts/coccinelle/misc/returnvar.cocci
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: kernel test robot <lkp@intel.com>

I checked the patch, and unfortunately it is wrong, the current code
needs to stay.
The problem is the SET_DMA_ERRATA() macro that accesses the
local 'errata' variable.

         Arnd
Tony Lindgren May 3, 2022, 7:30 a.m. UTC | #2
* Arnd Bergmann <arnd@arndb.de> [220503 07:18]:
> On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> >
> > From: kernel test robot <lkp@intel.com>
> >
> > arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> >
> >
> >  Remove unneeded variable used to store return value.
> >
> > Generated by: scripts/coccinelle/misc/returnvar.cocci
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: kernel test robot <lkp@intel.com>
> 
> I checked the patch, and unfortunately it is wrong, the current code
> needs to stay.
> The problem is the SET_DMA_ERRATA() macro that accesses the
> local 'errata' variable.

Yeah this one keeps popping up. Maybe we can make SET_DMA_ERRATA
into a function or have it at least change it to set the errata
value.

Regards,

Tony
Arnd Bergmann May 3, 2022, 7:53 a.m. UTC | #3
On Tue, May 3, 2022 at 9:30 AM Tony Lindgren <tony@atomide.com> wrote:
> * Arnd Bergmann <arnd@arndb.de> [220503 07:18]:
> > On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> > >
> > > From: kernel test robot <lkp@intel.com>
> > >
> > > arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> > >
> > >
> > >  Remove unneeded variable used to store return value.
> > >
> > > Generated by: scripts/coccinelle/misc/returnvar.cocci
> > >
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Signed-off-by: kernel test robot <lkp@intel.com>
> >
> > I checked the patch, and unfortunately it is wrong, the current code
> > needs to stay.
> > The problem is the SET_DMA_ERRATA() macro that accesses the
> > local 'errata' variable.
>
> Yeah this one keeps popping up. Maybe we can make SET_DMA_ERRATA
> into a function or have it at least change it to set the errata
> value.

I would just remove the macro and open-code the assignment, which
I think makes it more readable to both people and tools.

     Arnd
Tony Lindgren May 3, 2022, 8:23 a.m. UTC | #4
* Arnd Bergmann <arnd@arndb.de> [220503 08:12]:
> On Tue, May 3, 2022 at 9:30 AM Tony Lindgren <tony@atomide.com> wrote:
> > * Arnd Bergmann <arnd@arndb.de> [220503 07:18]:
> > > On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> > > >
> > > > From: kernel test robot <lkp@intel.com>
> > > >
> > > > arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> > > >
> > > >
> > > >  Remove unneeded variable used to store return value.
> > > >
> > > > Generated by: scripts/coccinelle/misc/returnvar.cocci
> > > >
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > Signed-off-by: kernel test robot <lkp@intel.com>
> > >
> > > I checked the patch, and unfortunately it is wrong, the current code
> > > needs to stay.
> > > The problem is the SET_DMA_ERRATA() macro that accesses the
> > > local 'errata' variable.
> >
> > Yeah this one keeps popping up. Maybe we can make SET_DMA_ERRATA
> > into a function or have it at least change it to set the errata
> > value.
> 
> I would just remove the macro and open-code the assignment, which
> I think makes it more readable to both people and tools.

Yeah agree after looking at the macro :)

Regards,

Tony
Russell King (Oracle) May 5, 2022, 2:08 p.m. UTC | #5
On Tue, May 03, 2022 at 10:45:32AM +0800, kernel test robot wrote:
> From: kernel test robot <lkp@intel.com>
> 
> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> 
> 
>  Remove unneeded variable used to store return value.
> 
> Generated by: scripts/coccinelle/misc/returnvar.cocci
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: kernel test robot <lkp@intel.com>

NAK. The analysis is wrong.
Dave Hansen May 5, 2022, 4:31 p.m. UTC | #6
On 5/3/22 00:21, Arnd Bergmann wrote:
> On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
>> From: kernel test robot <lkp@intel.com>
>>
>> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
>>
>>  Remove unneeded variable used to store return value.
>>
>> Generated by: scripts/coccinelle/misc/returnvar.cocci
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: kernel test robot <lkp@intel.com>
> I checked the patch, and unfortunately it is wrong, the current code
> needs to stay.
> The problem is the SET_DMA_ERRATA() macro that accesses the
> local 'errata' variable.

0day folks, do we have humans looking over these before they're going
out to the list?  If not, can we add some?  If so, can the humans get a
little more discerning? ;)
Philip Li May 6, 2022, 1:09 a.m. UTC | #7
On Thu, May 05, 2022 at 09:31:37AM -0700, Dave Hansen wrote:
> On 5/3/22 00:21, Arnd Bergmann wrote:
> > On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> >> From: kernel test robot <lkp@intel.com>
> >>
> >> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> >>
> >>  Remove unneeded variable used to store return value.
> >>
> >> Generated by: scripts/coccinelle/misc/returnvar.cocci
> >>
> >> Reported-by: kernel test robot <lkp@intel.com>
> >> Signed-off-by: kernel test robot <lkp@intel.com>
> > I checked the patch, and unfortunately it is wrong, the current code
> > needs to stay.
> > The problem is the SET_DMA_ERRATA() macro that accesses the
> > local 'errata' variable.
> 
> 0day folks, do we have humans looking over these before they're going
> out to the list?  If not, can we add some?  If so, can the humans get a
> little more discerning? ;)

Sorry all for the bad patch. So far, we pick up several cocci warnings that
we have confidence based on early result analysis and feedback, for these
warnings, 0day sends out patch automatically.

Thanks for the suggestion Dave, We will change current process to be more
conservative and to avoid false patch by adding human analysis.

Thanks
Philip Li May 6, 2022, 1:09 a.m. UTC | #8
On Thu, May 05, 2022 at 03:08:40PM +0100, Russell King (Oracle) wrote:
> On Tue, May 03, 2022 at 10:45:32AM +0800, kernel test robot wrote:
> > From: kernel test robot <lkp@intel.com>
> > 
> > arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> > 
> > 
> >  Remove unneeded variable used to store return value.
> > 
> > Generated by: scripts/coccinelle/misc/returnvar.cocci
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: kernel test robot <lkp@intel.com>
> 
> NAK. The analysis is wrong.

sorry about the false patch, we will improve this.

> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
> _______________________________________________
> kbuild-all mailing list -- kbuild-all@lists.01.org
> To unsubscribe send an email to kbuild-all-leave@lists.01.org
Arnd Bergmann May 6, 2022, 7:17 a.m. UTC | #9
On Fri, May 6, 2022 at 3:09 AM Philip Li <philip.li@intel.com> wrote:
> On Thu, May 05, 2022 at 09:31:37AM -0700, Dave Hansen wrote:
> > On 5/3/22 00:21, Arnd Bergmann wrote:
> > > On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> > >> From: kernel test robot <lkp@intel.com>
> > >>
> > >> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> > >>
> > >>  Remove unneeded variable used to store return value.
> > >>
> > >> Generated by: scripts/coccinelle/misc/returnvar.cocci
> > >>
> > >> Reported-by: kernel test robot <lkp@intel.com>
> > >> Signed-off-by: kernel test robot <lkp@intel.com>
> > > I checked the patch, and unfortunately it is wrong, the current code
> > > needs to stay.
> > > The problem is the SET_DMA_ERRATA() macro that accesses the
> > > local 'errata' variable.
> >
> > 0day folks, do we have humans looking over these before they're going
> > out to the list?  If not, can we add some?  If so, can the humans get a
> > little more discerning? ;)
>
> Sorry all for the bad patch. So far, we pick up several cocci warnings that
> we have confidence based on early result analysis and feedback, for these
> warnings, 0day sends out patch automatically.
>
> Thanks for the suggestion Dave, We will change current process to be more
> conservative and to avoid false patch by adding human analysis.

For the returnvar.cocci false-positives, I wonder if it's possible to find them
using another coccinelle helper that detects badly formed macros which
access variables out of scope. I can't think of how this would be expressed,
but maybe someone has an idea.

Something else went wrong in this particular patch,  and I can't explain
how this happened: the subject line contains the name of the wrong platform,
"dove" rather than "omap2". My guess is that this was human error copying
the subject line from another patch, but if this came from a script, you
may want to check how this gets generated.

       Arnd
Ard Biesheuvel May 6, 2022, 7:24 a.m. UTC | #10
On Fri, 6 May 2022 at 03:12, Philip Li <philip.li@intel.com> wrote:
>
> On Thu, May 05, 2022 at 09:31:37AM -0700, Dave Hansen wrote:
> > On 5/3/22 00:21, Arnd Bergmann wrote:
> > > On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> > >> From: kernel test robot <lkp@intel.com>
> > >>
> > >> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> > >>
> > >>  Remove unneeded variable used to store return value.
> > >>
> > >> Generated by: scripts/coccinelle/misc/returnvar.cocci
> > >>
> > >> Reported-by: kernel test robot <lkp@intel.com>
> > >> Signed-off-by: kernel test robot <lkp@intel.com>
> > > I checked the patch, and unfortunately it is wrong, the current code
> > > needs to stay.
> > > The problem is the SET_DMA_ERRATA() macro that accesses the
> > > local 'errata' variable.
> >
> > 0day folks, do we have humans looking over these before they're going
> > out to the list?  If not, can we add some?  If so, can the humans get a
> > little more discerning? ;)
>
> Sorry all for the bad patch. So far, we pick up several cocci warnings that
> we have confidence based on early result analysis and feedback, for these
> warnings, 0day sends out patch automatically.
>

Could you please add a special header or something to such emails so I
can filter them out? I am strongly opposed to such automatic spambot
patch generation, as it wastes valuable reviewer bandwidth to save the
bot operator some time, but it think it should be the other way
around.

We expect contributors to carefully prepare their patch submissions
before sending them to the list, and automatically generated patches
simply don't mesh with that. The fact that you use a bot does not mean
you can ignore these rules.
Philip Li May 6, 2022, 7:30 a.m. UTC | #11
On Fri, May 06, 2022 at 09:24:26AM +0200, Ard Biesheuvel wrote:
> On Fri, 6 May 2022 at 03:12, Philip Li <philip.li@intel.com> wrote:
> >
> > On Thu, May 05, 2022 at 09:31:37AM -0700, Dave Hansen wrote:
> > > On 5/3/22 00:21, Arnd Bergmann wrote:
> > > > On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> > > >> From: kernel test robot <lkp@intel.com>
> > > >>
> > > >> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> > > >>
> > > >>  Remove unneeded variable used to store return value.
> > > >>
> > > >> Generated by: scripts/coccinelle/misc/returnvar.cocci
> > > >>
> > > >> Reported-by: kernel test robot <lkp@intel.com>
> > > >> Signed-off-by: kernel test robot <lkp@intel.com>
> > > > I checked the patch, and unfortunately it is wrong, the current code
> > > > needs to stay.
> > > > The problem is the SET_DMA_ERRATA() macro that accesses the
> > > > local 'errata' variable.
> > >
> > > 0day folks, do we have humans looking over these before they're going
> > > out to the list?  If not, can we add some?  If so, can the humans get a
> > > little more discerning? ;)
> >
> > Sorry all for the bad patch. So far, we pick up several cocci warnings that
> > we have confidence based on early result analysis and feedback, for these
> > warnings, 0day sends out patch automatically.
> >
> 
> Could you please add a special header or something to such emails so I
> can filter them out? I am strongly opposed to such automatic spambot
> patch generation, as it wastes valuable reviewer bandwidth to save the
> bot operator some time, but it think it should be the other way
> around.

Sorry for the trouble, we will stop sending the patch automatically and
only send out patch after human confirmed/reviewed.

> 
> We expect contributors to carefully prepare their patch submissions
> before sending them to the list, and automatically generated patches
> simply don't mesh with that. The fact that you use a bot does not mean
> you can ignore these rules.

Got it, we will improve this to follow the right way to send out patches.

Thanks
Philip Li May 6, 2022, 7:32 a.m. UTC | #12
On Fri, May 06, 2022 at 09:17:44AM +0200, Arnd Bergmann wrote:
> On Fri, May 6, 2022 at 3:09 AM Philip Li <philip.li@intel.com> wrote:
> > On Thu, May 05, 2022 at 09:31:37AM -0700, Dave Hansen wrote:
> > > On 5/3/22 00:21, Arnd Bergmann wrote:
> > > > On Tue, May 3, 2022 at 4:45 AM kernel test robot <lkp@intel.com> wrote:
> > > >> From: kernel test robot <lkp@intel.com>
> > > >>
> > > >> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return "0" on line 161
> > > >>
> > > >>  Remove unneeded variable used to store return value.
> > > >>
> > > >> Generated by: scripts/coccinelle/misc/returnvar.cocci
> > > >>
> > > >> Reported-by: kernel test robot <lkp@intel.com>
> > > >> Signed-off-by: kernel test robot <lkp@intel.com>
> > > > I checked the patch, and unfortunately it is wrong, the current code
> > > > needs to stay.
> > > > The problem is the SET_DMA_ERRATA() macro that accesses the
> > > > local 'errata' variable.
> > >
> > > 0day folks, do we have humans looking over these before they're going
> > > out to the list?  If not, can we add some?  If so, can the humans get a
> > > little more discerning? ;)
> >
> > Sorry all for the bad patch. So far, we pick up several cocci warnings that
> > we have confidence based on early result analysis and feedback, for these
> > warnings, 0day sends out patch automatically.
> >
> > Thanks for the suggestion Dave, We will change current process to be more
> > conservative and to avoid false patch by adding human analysis.
> 
> For the returnvar.cocci false-positives, I wonder if it's possible to find them
> using another coccinelle helper that detects badly formed macros which
> access variables out of scope. I can't think of how this would be expressed,
> but maybe someone has an idea.
> 
> Something else went wrong in this particular patch,  and I can't explain
> how this happened: the subject line contains the name of the wrong platform,
> "dove" rather than "omap2". My guess is that this was human error copying
> the subject line from another patch, but if this came from a script, you
> may want to check how this gets generated.

Thanks Arnd, we will investigate this to fix our side issue. And thanks for
taking time to check the detail, as mentioned in other reply, we will not
send out patch unless it is carefully reviewed/acked by members of 0day.

> 
>        Arnd
diff mbox series

Patch

--- a/arch/arm/mach-omap2/dma.c
+++ b/arch/arm/mach-omap2/dma.c
@@ -79,8 +79,6 @@  static const struct omap_dma_reg reg_map
 
 static unsigned configure_dma_errata(void)
 {
-	unsigned errata = 0;
-
 	/*
 	 * Errata applicable for OMAP2430ES1.0 and all omap2420
 	 *
@@ -158,7 +156,7 @@  static unsigned configure_dma_errata(voi
 	if (cpu_is_omap34xx() && (omap_type() != OMAP2_DEVICE_TYPE_GP))
 		SET_DMA_ERRATA(DMA_ROMCODE_BUG);
 
-	return errata;
+	return 0;
 }
 
 static const struct dma_slave_map omap24xx_sdma_dt_map[] = {