linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net/dsa: remove unused macros to tame gcc warning
@ 2020-11-06  5:37 Alex Shi
  2020-11-06  6:36 ` Joe Perches
  2020-11-06 14:18 ` Andrew Lunn
  0 siblings, 2 replies; 12+ messages in thread
From: Alex Shi @ 2020-11-06  5:37 UTC (permalink / raw)
  To: andrew
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

There are some macros unused, they causes much gcc warnings. Let's
remove them to tame gcc.

net/dsa/tag_brcm.c:51:0: warning: macro "BRCM_EG_RC_SWITCH" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:53:0: warning: macro "BRCM_EG_RC_MIRROR" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:55:0: warning: macro "BRCM_EG_TC_MASK" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:35:0: warning: macro "BRCM_IG_TS_SHIFT" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:46:0: warning: macro "BRCM_EG_RC_MASK" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:49:0: warning: macro "BRCM_EG_RC_PROT_SNOOP" is not
used [-Wunused-macros]
net/dsa/tag_brcm.c:34:0: warning: macro "BRCM_IG_TE_MASK" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:43:0: warning: macro "BRCM_EG_CID_MASK" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:50:0: warning: macro "BRCM_EG_RC_PROT_TERM" is not
used [-Wunused-macros]
net/dsa/tag_brcm.c:54:0: warning: macro "BRCM_EG_TC_SHIFT" is not used
[-Wunused-macros]
net/dsa/tag_brcm.c:52:0: warning: macro "BRCM_EG_RC_MAC_LEARN" is not
used [-Wunused-macros]
net/dsa/tag_brcm.c:48:0: warning: macro "BRCM_EG_RC_EXCEPTION" is not
used [-Wunused-macros]

Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Cc: Andrew Lunn <andrew@lunn.ch> 
Cc: Vivien Didelot <vivien.didelot@gmail.com> 
Cc: Florian Fainelli <f.fainelli@gmail.com> 
Cc: Vladimir Oltean <olteanv@gmail.com> 
Cc: "David S. Miller" <davem@davemloft.net> 
Cc: Jakub Kicinski <kuba@kernel.org> 
Cc: netdev@vger.kernel.org 
Cc: linux-kernel@vger.kernel.org 
---
 net/dsa/tag_brcm.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index e934dace3922..ce23b5d4c6b8 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -30,29 +30,14 @@
 /* 1st byte in the tag */
 #define BRCM_IG_TC_SHIFT	2
 #define BRCM_IG_TC_MASK		0x7
-/* 2nd byte in the tag */
-#define BRCM_IG_TE_MASK		0x3
-#define BRCM_IG_TS_SHIFT	7
 /* 3rd byte in the tag */
 #define BRCM_IG_DSTMAP2_MASK	1
 #define BRCM_IG_DSTMAP1_MASK	0xff
 
 /* Egress fields */
 
-/* 2nd byte in the tag */
-#define BRCM_EG_CID_MASK	0xff
-
 /* 3rd byte in the tag */
-#define BRCM_EG_RC_MASK		0xff
 #define  BRCM_EG_RC_RSVD	(3 << 6)
-#define  BRCM_EG_RC_EXCEPTION	(1 << 5)
-#define  BRCM_EG_RC_PROT_SNOOP	(1 << 4)
-#define  BRCM_EG_RC_PROT_TERM	(1 << 3)
-#define  BRCM_EG_RC_SWITCH	(1 << 2)
-#define  BRCM_EG_RC_MAC_LEARN	(1 << 1)
-#define  BRCM_EG_RC_MIRROR	(1 << 0)
-#define BRCM_EG_TC_SHIFT	5
-#define BRCM_EG_TC_MASK		0x7
 #define BRCM_EG_PID_MASK	0x1f
 
 #if IS_ENABLED(CONFIG_NET_DSA_TAG_BRCM) || \
-- 
1.8.3.1


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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06  5:37 [PATCH] net/dsa: remove unused macros to tame gcc warning Alex Shi
@ 2020-11-06  6:36 ` Joe Perches
  2020-11-06  8:28   ` Alex Shi
  2020-11-06  9:24   ` David Laight
  2020-11-06 14:18 ` Andrew Lunn
  1 sibling, 2 replies; 12+ messages in thread
From: Joe Perches @ 2020-11-06  6:36 UTC (permalink / raw)
  To: Alex Shi, andrew
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Fri, 2020-11-06 at 13:37 +0800, Alex Shi wrote:
> There are some macros unused, they causes much gcc warnings. Let's
> remove them to tame gcc.

I believe these to be essentially poor warnings.

Aren't these warnings generated only when adding  W=2 to the make
command line?

Perhaps it's better to move the warning to level 3
---
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 95e4cdb94fe9..5c3c220ddb32 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -68,7 +68,6 @@ KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)
 KBUILD_CFLAGS += -Wmissing-field-initializers
 KBUILD_CFLAGS += -Wtype-limits
 KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)
-KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)
 
 KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN2
 
@@ -89,6 +88,7 @@ KBUILD_CFLAGS += -Wredundant-decls
 KBUILD_CFLAGS += -Wsign-compare
 KBUILD_CFLAGS += -Wswitch-default
 KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)
+KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)
 
 KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN3
 



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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06  6:36 ` Joe Perches
@ 2020-11-06  8:28   ` Alex Shi
  2020-11-06  8:52     ` Joe Perches
  2020-11-06  9:24   ` David Laight
  1 sibling, 1 reply; 12+ messages in thread
From: Alex Shi @ 2020-11-06  8:28 UTC (permalink / raw)
  To: Joe Perches, andrew
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel



在 2020/11/6 下午2:36, Joe Perches 写道:
> On Fri, 2020-11-06 at 13:37 +0800, Alex Shi wrote:
>> There are some macros unused, they causes much gcc warnings. Let's
>> remove them to tame gcc.
> 
> I believe these to be essentially poor warnings.
> 
> Aren't these warnings generated only when adding  W=2 to the make
> command line?
> 
> Perhaps it's better to move the warning to level 3
> ---
> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> index 95e4cdb94fe9..5c3c220ddb32 100644
> --- a/scripts/Makefile.extrawarn
> +++ b/scripts/Makefile.extrawarn
> @@ -68,7 +68,6 @@ KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)
>  KBUILD_CFLAGS += -Wmissing-field-initializers
>  KBUILD_CFLAGS += -Wtype-limits
>  KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)
> -KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)

This changed too much, and impact others. May not good. :)

Thanks
Alex
>  
>  KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN2
>  
> @@ -89,6 +88,7 @@ KBUILD_CFLAGS += -Wredundant-decls
>  KBUILD_CFLAGS += -Wsign-compare
>  KBUILD_CFLAGS += -Wswitch-default
>  KBUILD_CFLAGS += $(call cc-option, -Wpacked-bitfield-compat)
> +KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)
>  
>  KBUILD_CPPFLAGS += -DKBUILD_EXTRA_WARN3
>  
> 

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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06  8:28   ` Alex Shi
@ 2020-11-06  8:52     ` Joe Perches
  2020-11-06  8:57       ` Alex Shi
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2020-11-06  8:52 UTC (permalink / raw)
  To: Alex Shi, andrew
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Fri, 2020-11-06 at 16:28 +0800, Alex Shi wrote:
> 
> 在 2020/11/6 下午2:36, Joe Perches 写道:
> > On Fri, 2020-11-06 at 13:37 +0800, Alex Shi wrote:
> > > There are some macros unused, they causes much gcc warnings. Let's
> > > remove them to tame gcc.
> > 
> > I believe these to be essentially poor warnings.
> > 
> > Aren't these warnings generated only when adding  W=2 to the make
> > command line?
> > 
> > Perhaps it's better to move the warning to level 3
> > ---
> > diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
> > index 95e4cdb94fe9..5c3c220ddb32 100644
> > --- a/scripts/Makefile.extrawarn
> > +++ b/scripts/Makefile.extrawarn
> > @@ -68,7 +68,6 @@ KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)
> >  KBUILD_CFLAGS += -Wmissing-field-initializers
> >  KBUILD_CFLAGS += -Wtype-limits
> >  KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)
> > -KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)
> 
> This changed too much, and impact others. May not good. :)

Can you clarify what you mean?



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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06  8:52     ` Joe Perches
@ 2020-11-06  8:57       ` Alex Shi
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Shi @ 2020-11-06  8:57 UTC (permalink / raw)
  To: Joe Perches, andrew
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel



在 2020/11/6 下午4:52, Joe Perches 写道:
> On Fri, 2020-11-06 at 16:28 +0800, Alex Shi wrote:
>>
>> 在 2020/11/6 下午2:36, Joe Perches 写道:
>>> On Fri, 2020-11-06 at 13:37 +0800, Alex Shi wrote:
>>>> There are some macros unused, they causes much gcc warnings. Let's
>>>> remove them to tame gcc.
>>>
>>> I believe these to be essentially poor warnings.
>>>
>>> Aren't these warnings generated only when adding  W=2 to the make
>>> command line?
>>>
>>> Perhaps it's better to move the warning to level 3
>>> ---
>>> diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
>>> index 95e4cdb94fe9..5c3c220ddb32 100644
>>> --- a/scripts/Makefile.extrawarn
>>> +++ b/scripts/Makefile.extrawarn
>>> @@ -68,7 +68,6 @@ KBUILD_CFLAGS += $(call cc-option, -Wlogical-op)
>>>  KBUILD_CFLAGS += -Wmissing-field-initializers
>>>  KBUILD_CFLAGS += -Wtype-limits
>>>  KBUILD_CFLAGS += $(call cc-option, -Wmaybe-uninitialized)
>>> -KBUILD_CFLAGS += $(call cc-option, -Wunused-macros)
>>
>> This changed too much, and impact others. May not good. :)
> 
> Can you clarify what you mean?
> 

Uh, change in this file will impact all kernels not only for net/dsa.
If you want to keep these unused macros, maybe better to put them into comments?

Thanks!
Alex

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

* RE: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06  6:36 ` Joe Perches
  2020-11-06  8:28   ` Alex Shi
@ 2020-11-06  9:24   ` David Laight
  1 sibling, 0 replies; 12+ messages in thread
From: David Laight @ 2020-11-06  9:24 UTC (permalink / raw)
  To: 'Joe Perches', Alex Shi, andrew
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

From: Joe Perches
> Sent: 06 November 2020 06:36
> 
> On Fri, 2020-11-06 at 13:37 +0800, Alex Shi wrote:
> > There are some macros unused, they causes much gcc warnings. Let's
> > remove them to tame gcc.
> 
> I believe these to be essentially poor warnings.

Indeed.

One 'solution' is to move the #defines into an included .h file.

> Aren't these warnings generated only when adding  W=2 to the make
> command line?
> 
> Perhaps it's better to move the warning to level 3

I'd move then to level 9999999999.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06  5:37 [PATCH] net/dsa: remove unused macros to tame gcc warning Alex Shi
  2020-11-06  6:36 ` Joe Perches
@ 2020-11-06 14:18 ` Andrew Lunn
  2020-11-06 16:39   ` Florian Fainelli
  1 sibling, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2020-11-06 14:18 UTC (permalink / raw)
  To: Alex Shi
  Cc: Vivien Didelot, Florian Fainelli, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Fri, Nov 06, 2020 at 01:37:30PM +0800, Alex Shi wrote:
> There are some macros unused, they causes much gcc warnings. Let's
> remove them to tame gcc.
> 
> net/dsa/tag_brcm.c:51:0: warning: macro "BRCM_EG_RC_SWITCH" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:53:0: warning: macro "BRCM_EG_RC_MIRROR" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:55:0: warning: macro "BRCM_EG_TC_MASK" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:35:0: warning: macro "BRCM_IG_TS_SHIFT" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:46:0: warning: macro "BRCM_EG_RC_MASK" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:49:0: warning: macro "BRCM_EG_RC_PROT_SNOOP" is not
> used [-Wunused-macros]
> net/dsa/tag_brcm.c:34:0: warning: macro "BRCM_IG_TE_MASK" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:43:0: warning: macro "BRCM_EG_CID_MASK" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:50:0: warning: macro "BRCM_EG_RC_PROT_TERM" is not
> used [-Wunused-macros]
> net/dsa/tag_brcm.c:54:0: warning: macro "BRCM_EG_TC_SHIFT" is not used
> [-Wunused-macros]
> net/dsa/tag_brcm.c:52:0: warning: macro "BRCM_EG_RC_MAC_LEARN" is not
> used [-Wunused-macros]
> net/dsa/tag_brcm.c:48:0: warning: macro "BRCM_EG_RC_EXCEPTION" is not
> used [-Wunused-macros]
> 
> Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
> Cc: Andrew Lunn <andrew@lunn.ch> 
> Cc: Vivien Didelot <vivien.didelot@gmail.com> 
> Cc: Florian Fainelli <f.fainelli@gmail.com> 
> Cc: Vladimir Oltean <olteanv@gmail.com> 
> Cc: "David S. Miller" <davem@davemloft.net> 
> Cc: Jakub Kicinski <kuba@kernel.org> 
> Cc: netdev@vger.kernel.org 
> Cc: linux-kernel@vger.kernel.org 
> ---
>  net/dsa/tag_brcm.c | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
> index e934dace3922..ce23b5d4c6b8 100644
> --- a/net/dsa/tag_brcm.c
> +++ b/net/dsa/tag_brcm.c
> @@ -30,29 +30,14 @@
>  /* 1st byte in the tag */
>  #define BRCM_IG_TC_SHIFT	2
>  #define BRCM_IG_TC_MASK		0x7
> -/* 2nd byte in the tag */
> -#define BRCM_IG_TE_MASK		0x3
> -#define BRCM_IG_TS_SHIFT	7
>  /* 3rd byte in the tag */
>  #define BRCM_IG_DSTMAP2_MASK	1
>  #define BRCM_IG_DSTMAP1_MASK	0xff

Hi Alex

It is good to remember that there are multiple readers of source
files. There is the compiler which generates code from it, and there
is the human trying to understand what is going on, what the hardware
can do, how we could maybe extend the code in the future to make use
of bits are currently don't, etc.

The compiler has no use of these macros, at the moment. But i as a
human do. It is valuable documentation, given that there is no open
datasheet for this hardware.

I would say these warnings are bogus, and the code should be left
alone.

	Andrew

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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06 14:18 ` Andrew Lunn
@ 2020-11-06 16:39   ` Florian Fainelli
  2020-11-07 12:54     ` Alex Shi
  0 siblings, 1 reply; 12+ messages in thread
From: Florian Fainelli @ 2020-11-06 16:39 UTC (permalink / raw)
  To: Andrew Lunn, Alex Shi
  Cc: Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski,
	netdev, linux-kernel



On 11/6/2020 6:18 AM, Andrew Lunn wrote:
> On Fri, Nov 06, 2020 at 01:37:30PM +0800, Alex Shi wrote:
>> There are some macros unused, they causes much gcc warnings. Let's
>> remove them to tame gcc.
>>
>> net/dsa/tag_brcm.c:51:0: warning: macro "BRCM_EG_RC_SWITCH" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:53:0: warning: macro "BRCM_EG_RC_MIRROR" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:55:0: warning: macro "BRCM_EG_TC_MASK" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:35:0: warning: macro "BRCM_IG_TS_SHIFT" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:46:0: warning: macro "BRCM_EG_RC_MASK" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:49:0: warning: macro "BRCM_EG_RC_PROT_SNOOP" is not
>> used [-Wunused-macros]
>> net/dsa/tag_brcm.c:34:0: warning: macro "BRCM_IG_TE_MASK" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:43:0: warning: macro "BRCM_EG_CID_MASK" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:50:0: warning: macro "BRCM_EG_RC_PROT_TERM" is not
>> used [-Wunused-macros]
>> net/dsa/tag_brcm.c:54:0: warning: macro "BRCM_EG_TC_SHIFT" is not used
>> [-Wunused-macros]
>> net/dsa/tag_brcm.c:52:0: warning: macro "BRCM_EG_RC_MAC_LEARN" is not
>> used [-Wunused-macros]
>> net/dsa/tag_brcm.c:48:0: warning: macro "BRCM_EG_RC_EXCEPTION" is not
>> used [-Wunused-macros]
>>
>> Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
>> Cc: Andrew Lunn <andrew@lunn.ch> 
>> Cc: Vivien Didelot <vivien.didelot@gmail.com> 
>> Cc: Florian Fainelli <f.fainelli@gmail.com> 
>> Cc: Vladimir Oltean <olteanv@gmail.com> 
>> Cc: "David S. Miller" <davem@davemloft.net> 
>> Cc: Jakub Kicinski <kuba@kernel.org> 
>> Cc: netdev@vger.kernel.org 
>> Cc: linux-kernel@vger.kernel.org 
>> ---
>>  net/dsa/tag_brcm.c | 15 ---------------
>>  1 file changed, 15 deletions(-)
>>
>> diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
>> index e934dace3922..ce23b5d4c6b8 100644
>> --- a/net/dsa/tag_brcm.c
>> +++ b/net/dsa/tag_brcm.c
>> @@ -30,29 +30,14 @@
>>  /* 1st byte in the tag */
>>  #define BRCM_IG_TC_SHIFT	2
>>  #define BRCM_IG_TC_MASK		0x7
>> -/* 2nd byte in the tag */
>> -#define BRCM_IG_TE_MASK		0x3
>> -#define BRCM_IG_TS_SHIFT	7
>>  /* 3rd byte in the tag */
>>  #define BRCM_IG_DSTMAP2_MASK	1
>>  #define BRCM_IG_DSTMAP1_MASK	0xff
> 
> Hi Alex
> 
> It is good to remember that there are multiple readers of source
> files. There is the compiler which generates code from it, and there
> is the human trying to understand what is going on, what the hardware
> can do, how we could maybe extend the code in the future to make use
> of bits are currently don't, etc.
> 
> The compiler has no use of these macros, at the moment. But i as a
> human do. It is valuable documentation, given that there is no open
> datasheet for this hardware.
> 
> I would say these warnings are bogus, and the code should be left
> alone.

Agreed, these definitions are intended to document what the hardware
does. These warnings are getting too far.
-- 
Florian

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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-06 16:39   ` Florian Fainelli
@ 2020-11-07 12:54     ` Alex Shi
  2020-11-07 17:39       ` Joe Perches
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Shi @ 2020-11-07 12:54 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski,
	netdev, linux-kernel



在 2020/11/7 上午12:39, Florian Fainelli 写道:
>> Hi Alex
>>
>> It is good to remember that there are multiple readers of source
>> files. There is the compiler which generates code from it, and there
>> is the human trying to understand what is going on, what the hardware
>> can do, how we could maybe extend the code in the future to make use
>> of bits are currently don't, etc.
>>
>> The compiler has no use of these macros, at the moment. But i as a
>> human do. It is valuable documentation, given that there is no open
>> datasheet for this hardware.
>>
>> I would say these warnings are bogus, and the code should be left
>> alone.
> Agreed, these definitions are intended to document what the hardware
> does. These warnings are getting too far.
> -- 

Thanks for all comments! I agree these info are much meaningful.
Is there other way to tame the gcc warning? like put them into a .h file
or covered by comments?

Thanks
Alex

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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-07 12:54     ` Alex Shi
@ 2020-11-07 17:39       ` Joe Perches
  2020-11-07 22:33         ` Andrew Lunn
  0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2020-11-07 17:39 UTC (permalink / raw)
  To: Alex Shi, Florian Fainelli, Andrew Lunn
  Cc: Vivien Didelot, Vladimir Oltean, David S. Miller, Jakub Kicinski,
	netdev, linux-kernel

On Sat, 2020-11-07 at 20:54 +0800, Alex Shi wrote:
> 在 2020/11/7 上午12:39, Florian Fainelli 写道:
> > > It is good to remember that there are multiple readers of source
> > > files. There is the compiler which generates code from it, and there
> > > is the human trying to understand what is going on, what the hardware
> > > can do, how we could maybe extend the code in the future to make use
> > > of bits are currently don't, etc.
> > > 
> > > The compiler has no use of these macros, at the moment. But i as a
> > > human do. It is valuable documentation, given that there is no open
> > > datasheet for this hardware.
> > > 
> > > I would say these warnings are bogus, and the code should be left
> > > alone.
> > Agreed, these definitions are intended to document what the hardware
> > does. These warnings are getting too far.
> 
> Thanks for all comments! I agree these info are much meaningful.
> Is there other way to tame the gcc warning? like put them into a .h file
> or covered by comments?

Does _any_ version of gcc have this warning on by default?

I still think my proposal of moving the warning from W=2 to W=3
quite reasonable.

Another possibility is to turn the warning off altogether.



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

* Re: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-07 17:39       ` Joe Perches
@ 2020-11-07 22:33         ` Andrew Lunn
  2020-11-08 12:39           ` David Laight
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Lunn @ 2020-11-07 22:33 UTC (permalink / raw)
  To: Joe Perches
  Cc: Alex Shi, Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

On Sat, Nov 07, 2020 at 09:39:42AM -0800, Joe Perches wrote:
> On Sat, 2020-11-07 at 20:54 +0800, Alex Shi wrote:
> > 在 2020/11/7 上午12:39, Florian Fainelli 写道:
> > > > It is good to remember that there are multiple readers of source
> > > > files. There is the compiler which generates code from it, and there
> > > > is the human trying to understand what is going on, what the hardware
> > > > can do, how we could maybe extend the code in the future to make use
> > > > of bits are currently don't, etc.
> > > > 
> > > > The compiler has no use of these macros, at the moment. But i as a
> > > > human do. It is valuable documentation, given that there is no open
> > > > datasheet for this hardware.
> > > > 
> > > > I would say these warnings are bogus, and the code should be left
> > > > alone.
> > > Agreed, these definitions are intended to document what the hardware
> > > does. These warnings are getting too far.
> > 
> > Thanks for all comments! I agree these info are much meaningful.
> > Is there other way to tame the gcc warning? like put them into a .h file
> > or covered by comments?
> 
> Does _any_ version of gcc have this warning on by default?
> 
> I still think my proposal of moving the warning from W=2 to W=3
> quite reasonable.
> 
> Another possibility is to turn the warning off altogether.

Lets tern the question around first. How many real bugs have you found
with this warning? Places where the #define should of been used, but
was not? Then we can get an idea of the value of this warning. My
guess would be, its value is ~ 0 for the kernel. If so, we should just
turn it off.

     Andrew

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

* RE: [PATCH] net/dsa: remove unused macros to tame gcc warning
  2020-11-07 22:33         ` Andrew Lunn
@ 2020-11-08 12:39           ` David Laight
  0 siblings, 0 replies; 12+ messages in thread
From: David Laight @ 2020-11-08 12:39 UTC (permalink / raw)
  To: 'Andrew Lunn', Joe Perches
  Cc: Alex Shi, Florian Fainelli, Vivien Didelot, Vladimir Oltean,
	David S. Miller, Jakub Kicinski, netdev, linux-kernel

From: Andrew Lunn
> Sent: 07 November 2020 22:33
> 
> On Sat, Nov 07, 2020 at 09:39:42AM -0800, Joe Perches wrote:
> > On Sat, 2020-11-07 at 20:54 +0800, Alex Shi wrote:
> > > 在 2020/11/7 上午12:39, Florian Fainelli 写道:
> > > > > It is good to remember that there are multiple readers of source
> > > > > files. There is the compiler which generates code from it, and there
> > > > > is the human trying to understand what is going on, what the hardware
> > > > > can do, how we could maybe extend the code in the future to make use
> > > > > of bits are currently don't, etc.
> > > > >
> > > > > The compiler has no use of these macros, at the moment. But i as a
> > > > > human do. It is valuable documentation, given that there is no open
> > > > > datasheet for this hardware.
> > > > >
> > > > > I would say these warnings are bogus, and the code should be left
> > > > > alone.
> > > > Agreed, these definitions are intended to document what the hardware
> > > > does. These warnings are getting too far.
> > >
> > > Thanks for all comments! I agree these info are much meaningful.
> > > Is there other way to tame the gcc warning? like put them into a .h file
> > > or covered by comments?
> >
> > Does _any_ version of gcc have this warning on by default?
> >
> > I still think my proposal of moving the warning from W=2 to W=3
> > quite reasonable.
> >
> > Another possibility is to turn the warning off altogether.
> 
> Lets tern the question around first. How many real bugs have you found
> with this warning? Places where the #define should of been used, but
> was not? Then we can get an idea of the value of this warning. My
> guess would be, its value is ~ 0 for the kernel. If so, we should just
> turn it off.

Sometimes there are defines for 0, 1 and 2 (enumeration or bitmap)
and the one for 0 is never used.
Rather than delete the define for 0 (which has been proposed at
least once in this series) the 'fix' would be to find the
initialisation and replace the literal 0 with the define.
OTOH it is probably implicit from a memset().

Even if never actually used the define for 0 helps the
human reading the code.

The same is true when there are defines for hardware register bits.
It is useful to know what they all mean - even if some aren't used.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

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

end of thread, other threads:[~2020-11-08 12:44 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-06  5:37 [PATCH] net/dsa: remove unused macros to tame gcc warning Alex Shi
2020-11-06  6:36 ` Joe Perches
2020-11-06  8:28   ` Alex Shi
2020-11-06  8:52     ` Joe Perches
2020-11-06  8:57       ` Alex Shi
2020-11-06  9:24   ` David Laight
2020-11-06 14:18 ` Andrew Lunn
2020-11-06 16:39   ` Florian Fainelli
2020-11-07 12:54     ` Alex Shi
2020-11-07 17:39       ` Joe Perches
2020-11-07 22:33         ` Andrew Lunn
2020-11-08 12:39           ` David Laight

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).