[PATCH 1/1] spi: Remove unused definitions

[PATCH 1/1] spi: Remove unused definitions

[Nick Krause]

Remove unused definition which cause the following warnings

drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
include/linux/fs.h:193:0: note: this is the location of the previous definition
drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
include/linux/fs.h:192:0: note: this is the location of the previous definition

Signed-off-by: Nick Krause <xerofoiffy@gmail.com>
---
 drivers/spi/spi-omap-100k.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/spi/spi-omap-100k.c b/drivers/spi/spi-omap-100k.c
index 5e91858..fb52276 100644
--- a/drivers/spi/spi-omap-100k.c
+++ b/drivers/spi/spi-omap-100k.c
@@ -70,10 +70,6 @@
 #define SPI_STATUS_WE                   (1UL << 1)
 #define SPI_STATUS_RD                   (1UL << 0)
 
-#define WRITE 0
-#define READ  1
-
-
 /* use PIO for small transfers, avoiding DMA setup/teardown overhead and
  * cache operations; better heuristics consider wordsize and bitrate.
  */
-- 
1.9.1

Re: [PATCH 1/1] spi: Remove unused definitions

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

On Wed, 06 Aug 2014 13:53:17 -0400, Nick Krause said:
> Remove unused definition which cause the following warnings
> 
> drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
> include/linux/fs.h:193:0: note: this is the location of the previous definition
> drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
> include/linux/fs.h:192:0: note: this is the location of the previous definition

> -#define WRITE 0
> -#define READ  1

NAK.  Full stop.  These are potentially used in an inner macro someplace, and by
removing these, the conflicting values from fs.h will be used instead.

#define READ                    0
#define WRITE                   RW_MASK

So if there *is* a use in an inner macro, you just screwed the pooch
and introduced a bug in this "clean up" - somebody will be expecting to see
a 0 for a READ, and will receive a 1 instead.  This can't end well.

Nick - how *exactly* did you identify that these are in fact not used?
Given your history of submitting poorly researched patches, you're going to
have to justify the "unused" better than the handwaving you've done here.

[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]

Re: [PATCH 1/1] spi: Remove unused definitions

[Ilia Mirkin]

On Wed, Aug 6, 2014 at 2:27 PM,  <Valdis.Kletnieks@vt.edu> wrote:
> On Wed, 06 Aug 2014 13:53:17 -0400, Nick Krause said:
>> Remove unused definition which cause the following warnings
>>
>> drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
>> include/linux/fs.h:193:0: note: this is the location of the previous definition
>> drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
>> include/linux/fs.h:192:0: note: this is the location of the previous definition
>
>> -#define WRITE 0
>> -#define READ  1
>
> NAK.  Full stop.  These are potentially used in an inner macro someplace, and by
> removing these, the conflicting values from fs.h will be used instead.
>
> #define READ                    0
> #define WRITE                   RW_MASK
>
> So if there *is* a use in an inner macro, you just screwed the pooch
> and introduced a bug in this "clean up" - somebody will be expecting to see
> a 0 for a READ, and will receive a 1 instead.  This can't end well.

I was actually about to send a similar response, but then I noticed I
couldn't actually find any uses of READ/WRITE in the file... But I
could easily have missed them.

>
> Nick - how *exactly* did you identify that these are in fact not used?
> Given your history of submitting poorly researched patches, you're going to
> have to justify the "unused" better than the handwaving you've done here.

Re: [PATCH 1/1] spi: Remove unused definitions

[Geert Uytterhoeven]

On Wed, Aug 6, 2014 at 8:27 PM,  <Valdis.Kletnieks@vt.edu> wrote:
> On Wed, 06 Aug 2014 13:53:17 -0400, Nick Krause said:
>> Remove unused definition which cause the following warnings
>>
>> drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
>> include/linux/fs.h:193:0: note: this is the location of the previous definition
>> drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
>> include/linux/fs.h:192:0: note: this is the location of the previous definition
>
>> -#define WRITE 0
>> -#define READ  1
>
> NAK.  Full stop.  These are potentially used in an inner macro someplace, and by
> removing these, the conflicting values from fs.h will be used instead.
>
> #define READ                    0
> #define WRITE                   RW_MASK
>
> So if there *is* a use in an inner macro, you just screwed the pooch
> and introduced a bug in this "clean up" - somebody will be expecting to see
> a 0 for a READ, and will receive a 1 instead.  This can't end well.
>
> Nick - how *exactly* did you identify that these are in fact not used?
> Given your history of submitting poorly researched patches, you're going to
> have to justify the "unused" better than the handwaving you've done here.

Just looking into this...

It seems READ and WRITE are not used at all in this file.

To be 100% sure, I tried to find with which kernel config this warning shows up.
It doesn't happen for omap1_defconfig with CONFIG_SPI_OMAP_100K,
which was the most likely culprit.

It does happen with sparc/sparc64 allmodconfig. However, changing or
removing the READ and WRITE definitions in drivers/spi/spi-omap-100k.c
doesn't have any influence on the preprocessed source files
("make drivers/spi/spi-omap-100k.i", modulo line number changes),
for both omap1 and sparc builds.

(Nick: I believe the above is what people really want to see)

So I conclude they are really not used, and they can be safely removed.

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/1] spi: Remove unused definitions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1240 bytes --]

On Wed, Aug 06, 2014 at 08:50:15PM +0200, Geert Uytterhoeven wrote:

> To be 100% sure, I tried to find with which kernel config this warning shows up.
> It doesn't happen for omap1_defconfig with CONFIG_SPI_OMAP_100K,
> which was the most likely culprit.

> It does happen with sparc/sparc64 allmodconfig. However, changing or
> removing the READ and WRITE definitions in drivers/spi/spi-omap-100k.c
> doesn't have any influence on the preprocessed source files
> ("make drivers/spi/spi-omap-100k.i", modulo line number changes),
> for both omap1 and sparc builds.

> (Nick: I believe the above is what people really want to see)

> So I conclude they are really not used, and they can be safely removed.

> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Right, I'd already done the same analysis myself with looking for uses
myself (though I didn't go looking for how to trigger).  Nick, please do
look at the analysis Geert provided above - it is indeed exactly the
sort of thing that people would want to see, just saying what's in the
diff isn't enough especially given the well advertised quality problems
with your submissions.  You really need to explain why the change you're
making is a good idea.

I've applied the change.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/1] spi: Remove unused definitions

[Pavel Machek]

On Wed 2014-08-06 14:27:20, Valdis.Kletnieks@vt.edu wrote:
> On Wed, 06 Aug 2014 13:53:17 -0400, Nick Krause said:
> > Remove unused definition which cause the following warnings
> > 
> > drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
> > include/linux/fs.h:193:0: note: this is the location of the previous definition
> > drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
> > include/linux/fs.h:192:0: note: this is the location of the previous definition
> 
> > -#define WRITE 0
> > -#define READ  1
> 
> NAK.  Full stop.  These are potentially used in an inner macro someplace, and by
> removing these, the conflicting values from fs.h will be used instead.
> 
> #define READ                    0
> #define WRITE                   RW_MASK
> 
> So if there *is* a use in an inner macro, you just screwed the pooch
> and introduced a bug in this "clean up" - somebody will be expecting to see
> a 0 for a READ, and will receive a 1 instead.  This can't end well.

Actually.. having macros called READ and WRITE in fs.h is already something I'd say
can't end well. Can we rename those?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/1] spi: Remove unused definitions

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

(Adding Al Viro and linux-fsdevel, dropping Mark Brown and the SPI list, because this is
heading off in a different direction now)

On Wed, 20 Aug 2014 22:26:02 +0200, Pavel Machek said:
> On Wed 2014-08-06 14:27:20, Valdis.Kletnieks@vt.edu wrote:
> > On Wed, 06 Aug 2014 13:53:17 -0400, Nick Krause said:
> > > Remove unused definition which cause the following warnings
> > >
> > > drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
> > > include/linux/fs.h:193:0: note: this is the location of the previous definition
> > > drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
> > > include/linux/fs.h:192:0: note: this is the location of the previous definition
> >
> > > -#define WRITE 0
> > > -#define READ  1
> >
> > NAK.  Full stop.  These are potentially used in an inner macro someplace, and by
> > removing these, the conflicting values from fs.h will be used instead.
> >
> > #define READ                    0
> > #define WRITE                   RW_MASK
> >
> > So if there *is* a use in an inner macro, you just screwed the pooch
> > and introduced a bug in this "clean up" - somebody will be expecting to see
> > a 0 for a READ, and will receive a 1 instead.  This can't end well.
>
> Actually.. having macros called READ and WRITE in fs.h is already something I'd say
> can't end well. Can we rename those?

I had the same thought, but other than a test rename to XYZZY_READ and PLUGH_WRITE
and doing a 'make allmodconfig' and seeing what throws an error, I'm not sure how
to track down all the users.  On my fairly stripped-down .config, I have:

[/usr/src/linux-next] find * -name '.*.cmd' | wc -l
4671
[/usr/src/linux-next] find * -name '.*.cmd' | xargs grep include/linux/fs.h | wc -l
2339

Which is telling me that pretty much half the world ends up including fs.h indirectly.

Now for the mandatory bikeshedding:

What do we want to rename them to? :)

[-- Attachment #2: Type: application/pgp-signature, Size: 848 bytes --]

Re: [PATCH 1/1] spi: Remove unused definitions

[Pavel Machek]

On Wed 2014-08-20 17:12:42, Valdis.Kletnieks@vt.edu wrote:
> (Adding Al Viro and linux-fsdevel, dropping Mark Brown and the SPI list, because this is
> heading off in a different direction now)
> 
> On Wed, 20 Aug 2014 22:26:02 +0200, Pavel Machek said:
> > On Wed 2014-08-06 14:27:20, Valdis.Kletnieks@vt.edu wrote:
> > > On Wed, 06 Aug 2014 13:53:17 -0400, Nick Krause said:
> > > > Remove unused definition which cause the following warnings
> > > >
> > > > drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
> > > > include/linux/fs.h:193:0: note: this is the location of the previous definition
> > > > drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
> > > > include/linux/fs.h:192:0: note: this is the location of the previous definition
> > >
> > > > -#define WRITE 0
> > > > -#define READ  1
> > >
> > > NAK.  Full stop.  These are potentially used in an inner macro someplace, and by
> > > removing these, the conflicting values from fs.h will be used instead.
> > >
> > > #define READ                    0
> > > #define WRITE                   RW_MASK
> > >
> > > So if there *is* a use in an inner macro, you just screwed the pooch
> > > and introduced a bug in this "clean up" - somebody will be expecting to see
> > > a 0 for a READ, and will receive a 1 instead.  This can't end well.
> >
> > Actually.. having macros called READ and WRITE in fs.h is already something I'd say
> > can't end well. Can we rename those?
> 
> I had the same thought, but other than a test rename to XYZZY_READ and PLUGH_WRITE
> and doing a 'make allmodconfig' and seeing what throws an error, I'm not sure how
> to track down all the users.  On my fairly stripped-down .config, I have:
> 
> [/usr/src/linux-next] find * -name '.*.cmd' | wc -l
> 4671
> [/usr/src/linux-next] find * -name '.*.cmd' | xargs grep include/linux/fs.h | wc -l
> 2339
> 
> Which is telling me that pretty much half the world ends up including fs.h indirectly.

Yep.

I hope sh math emulator does not include fs.h.

arch/sh/math-emu/math.c:#define READ(d,a)       ({if(get_user(d, (typeof (d)*)a)

> Now for the mandatory bikeshedding:
> 
> What do we want to rename them to? :)

REQ_ prefix would fit there well, I'd say.
									Pavel


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/1] spi: Remove unused definitions

[Greg KH]

On Wed, Aug 06, 2014 at 01:53:17PM -0400, Nick Krause wrote:
> Remove unused definition which cause the following warnings
> 
> drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
> include/linux/fs.h:193:0: note: this is the location of the previous definition
> drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
> include/linux/fs.h:192:0: note: this is the location of the previous definition
> 
> Signed-off-by: Nick Krause <xerofoiffy@gmail.com>
> ---
>  drivers/spi/spi-omap-100k.c | 4 ----
>  1 file changed, 4 deletions(-)

Nick, you were warned about this numerous times in the past.

By changing email addresses you have now ensured that we will never
listen to you, as you obviously are not listening to us.

greg k-h

Re: [PATCH 1/1] spi: Remove unused definitions

[Guenter Roeck]

On Wed, Aug 06, 2014 at 11:33:19AM -0700, Greg KH wrote:
> On Wed, Aug 06, 2014 at 01:53:17PM -0400, Nick Krause wrote:
> > Remove unused definition which cause the following warnings
> > 
> > drivers/spi/spi-omap-100k.c:73:0: warning: "WRITE" redefined [enabled by default]
> > include/linux/fs.h:193:0: note: this is the location of the previous definition
> > drivers/spi/spi-omap-100k.c:74:0: warning: "READ" redefined [enabled by default]
> > include/linux/fs.h:192:0: note: this is the location of the previous definition
> > 
> > Signed-off-by: Nick Krause <xerofoiffy@gmail.com>
> > ---
> >  drivers/spi/spi-omap-100k.c | 4 ----
> >  1 file changed, 4 deletions(-)
> 
> Nick, you were warned about this numerous times in the past.
> 
> By changing email addresses you have now ensured that we will never
> listen to you, as you obviously are not listening to us.
> 
He does manage to get our attention, though. Also, it is not that simple -
for my part I keep looking out for the patches just to make sure that
we don't get any more less than well researched "fixes" into the kernel.
I figure it pays to spend some time now instead of having to clean up
the resulting mess if something slips by.

Guenter