All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 17:53 ` Nick Krause
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Krause @ 2014-08-06 17:53 UTC (permalink / raw)
  To: Richard Weinberger, Mark Brown, open list:SPI SUBSYSTEM, open list

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 17:53 ` Nick Krause
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Krause @ 2014-08-06 17:53 UTC (permalink / raw)
  To: Richard Weinberger, Mark Brown, open list:SPI SUBSYSTEM, open list

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
  2014-08-06 17:53 ` Nick Krause
  (?)
@ 2014-08-06 18:27 ` Valdis.Kletnieks
  2014-08-06 18:35     ` Ilia Mirkin
                     ` (2 more replies)
  -1 siblings, 3 replies; 16+ messages in thread
From: Valdis.Kletnieks @ 2014-08-06 18:27 UTC (permalink / raw)
  To: Nick Krause
  Cc: Richard Weinberger, Mark Brown, open list:SPI SUBSYSTEM, open list

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 18:33   ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2014-08-06 18:33 UTC (permalink / raw)
  To: Nick Krause
  Cc: Richard Weinberger, Mark Brown, open list:SPI SUBSYSTEM, open list

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 18:33   ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2014-08-06 18:33 UTC (permalink / raw)
  To: Nick Krause
  Cc: Richard Weinberger, Mark Brown, open list:SPI SUBSYSTEM, open list

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 18:35     ` Ilia Mirkin
  0 siblings, 0 replies; 16+ messages in thread
From: Ilia Mirkin @ 2014-08-06 18:35 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 18:35     ` Ilia Mirkin
  0 siblings, 0 replies; 16+ messages in thread
From: Ilia Mirkin @ 2014-08-06 18:35 UTC (permalink / raw)
  To: Valdis.Kletnieks-PjAqaU27lzQ
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

On Wed, Aug 6, 2014 at 2:27 PM,  <Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 18:50     ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2014-08-06 18:50 UTC (permalink / raw)
  To: Valdis Kletnieks
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 18:50     ` Geert Uytterhoeven
  0 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2014-08-06 18:50 UTC (permalink / raw)
  To: Valdis Kletnieks
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

On Wed, Aug 6, 2014 at 8:27 PM,  <Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org> 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-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 19:14     ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2014-08-06 19:14 UTC (permalink / raw)
  To: Greg KH
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-06 19:14     ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2014-08-06 19:14 UTC (permalink / raw)
  To: Greg KH
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
  2014-08-06 18:50     ` Geert Uytterhoeven
  (?)
@ 2014-08-06 19:34     ` Mark Brown
  -1 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2014-08-06 19:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Valdis Kletnieks, Nick Krause, Richard Weinberger,
	open list:SPI SUBSYSTEM, open list, Greg Kroah-Hartman

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-20 20:26     ` Pavel Machek
  0 siblings, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2014-08-20 20:26 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
@ 2014-08-20 20:26     ` Pavel Machek
  0 siblings, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2014-08-20 20:26 UTC (permalink / raw)
  To: Valdis.Kletnieks-PjAqaU27lzQ
  Cc: Nick Krause, Richard Weinberger, Mark Brown,
	open list:SPI SUBSYSTEM, open list

On Wed 2014-08-06 14:27:20, Valdis.Kletnieks-PjAqaU27lzQ@public.gmane.org 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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
  2014-08-20 20:26     ` Pavel Machek
  (?)
@ 2014-08-20 21:12     ` Valdis.Kletnieks
  2014-08-20 21:56       ` Pavel Machek
  -1 siblings, 1 reply; 16+ messages in thread
From: Valdis.Kletnieks @ 2014-08-20 21:12 UTC (permalink / raw)
  To: Pavel Machek, linux-fsdevel, Alexander Viro
  Cc: Nick Krause, Richard Weinberger, open list

[-- 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 --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] spi: Remove unused definitions
  2014-08-20 21:12     ` Valdis.Kletnieks
@ 2014-08-20 21:56       ` Pavel Machek
  0 siblings, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2014-08-20 21:56 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: linux-fsdevel, Alexander Viro, Nick Krause, Richard Weinberger,
	open list

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-08-20 21:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-06 17:53 [PATCH 1/1] spi: Remove unused definitions Nick Krause
2014-08-06 17:53 ` Nick Krause
2014-08-06 18:27 ` Valdis.Kletnieks
2014-08-06 18:35   ` Ilia Mirkin
2014-08-06 18:35     ` Ilia Mirkin
2014-08-06 18:50   ` Geert Uytterhoeven
2014-08-06 18:50     ` Geert Uytterhoeven
2014-08-06 19:34     ` Mark Brown
2014-08-20 20:26   ` Pavel Machek
2014-08-20 20:26     ` Pavel Machek
2014-08-20 21:12     ` Valdis.Kletnieks
2014-08-20 21:56       ` Pavel Machek
2014-08-06 18:33 ` Greg KH
2014-08-06 18:33   ` Greg KH
2014-08-06 19:14   ` Guenter Roeck
2014-08-06 19:14     ` Guenter Roeck

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.