net: systemport: avoid build warnings due to unused I/O helpers

[PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers

Posted by Vladimir Oltean 1 month, 1 week ago

A clang-16 W=1 build emits the following (abridged):

warning: unused function 'txchk_readl' [-Wunused-function]
BCM_SYSPORT_IO_MACRO(txchk, SYS_PORT_TXCHK_OFFSET);
note: expanded from macro 'BCM_SYSPORT_IO_MACRO'

warning: unused function 'txchk_writel' [-Wunused-function]
note: expanded from macro 'BCM_SYSPORT_IO_MACRO'

warning: unused function 'tbuf_readl' [-Wunused-function]
BCM_SYSPORT_IO_MACRO(tbuf, SYS_PORT_TBUF_OFFSET);
note: expanded from macro 'BCM_SYSPORT_IO_MACRO'

warning: unused function 'tbuf_writel' [-Wunused-function]
note: expanded from macro 'BCM_SYSPORT_IO_MACRO'

Annotate the functions with the __maybe_unused attribute to tell the
compiler it's fine to do dead code elimination, and suppress the
warnings.

Also, remove the "inline" keyword from C files, since the compiler is
free anyway to inline or not.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index b45ed7cd2921..7d6e2c2ee445 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -29,13 +29,15 @@
 
 /* I/O accessors register helpers */
 #define BCM_SYSPORT_IO_MACRO(name, offset) \
-static inline u32 name##_readl(struct bcm_sysport_priv *priv, u32 off)	\
+static u32 __maybe_unused						\
+name##_readl(struct bcm_sysport_priv *priv, u32 off)			\
 {									\
 	u32 reg = readl_relaxed(priv->base + offset + off);		\
 	return reg;							\
 }									\
-static inline void name##_writel(struct bcm_sysport_priv *priv,		\
-				  u32 val, u32 off)			\
+									\
+static void __maybe_unused						\
+name##_writel(struct bcm_sysport_priv *priv, u32 val, u32 off)		\
 {									\
 	writel_relaxed(val, priv->base + offset + off);			\
 }									\
-- 
2.43.0

Re: [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers

Posted by Florian Fainelli 1 month, 1 week ago

On 10/14/24 08:01, Vladimir Oltean wrote:
> A clang-16 W=1 build emits the following (abridged):
> 
> warning: unused function 'txchk_readl' [-Wunused-function]
> BCM_SYSPORT_IO_MACRO(txchk, SYS_PORT_TXCHK_OFFSET);
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> warning: unused function 'txchk_writel' [-Wunused-function]
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> warning: unused function 'tbuf_readl' [-Wunused-function]
> BCM_SYSPORT_IO_MACRO(tbuf, SYS_PORT_TBUF_OFFSET);
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> warning: unused function 'tbuf_writel' [-Wunused-function]
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> Annotate the functions with the __maybe_unused attribute to tell the
> compiler it's fine to do dead code elimination, and suppress the
> warnings.
> 
> Also, remove the "inline" keyword from C files, since the compiler is
> free anyway to inline or not.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

clang is adequately warning that the txchk_{read,write}l functions are 
not used at all, so while your patch is correct, I think we could also 
go with this one liner in addition, or as a replacement to your patch:

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c 
b/drivers/net/ethernet/broadcom/bcmsysport.c
index c9faa8540859..7cea30eac83a 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -46,7 +46,6 @@ BCM_SYSPORT_IO_MACRO(umac, SYS_PORT_UMAC_OFFSET);
  BCM_SYSPORT_IO_MACRO(gib, SYS_PORT_GIB_OFFSET);
  BCM_SYSPORT_IO_MACRO(tdma, SYS_PORT_TDMA_OFFSET);
  BCM_SYSPORT_IO_MACRO(rxchk, SYS_PORT_RXCHK_OFFSET);
-BCM_SYSPORT_IO_MACRO(txchk, SYS_PORT_TXCHK_OFFSET);
  BCM_SYSPORT_IO_MACRO(rbuf, SYS_PORT_RBUF_OFFSET);
  BCM_SYSPORT_IO_MACRO(tbuf, SYS_PORT_TBUF_OFFSET);
  BCM_SYSPORT_IO_MACRO(topctrl, SYS_PORT_TOPCTRL_OFFSET);
-- 
Florian

Re: [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers

Posted by Vladimir Oltean 1 month, 1 week ago

On Mon, Oct 14, 2024 at 09:56:25AM -0700, Florian Fainelli wrote:
> On 10/14/24 08:01, Vladimir Oltean wrote:
> > A clang-16 W=1 build emits the following (abridged):
> > 
> > warning: unused function 'txchk_readl' [-Wunused-function]
> > BCM_SYSPORT_IO_MACRO(txchk, SYS_PORT_TXCHK_OFFSET);
> > note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> > 
> > warning: unused function 'txchk_writel' [-Wunused-function]
> > note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> > 
> > warning: unused function 'tbuf_readl' [-Wunused-function]
> > BCM_SYSPORT_IO_MACRO(tbuf, SYS_PORT_TBUF_OFFSET);
> > note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> > 
> > warning: unused function 'tbuf_writel' [-Wunused-function]
> > note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> > 
> > Annotate the functions with the __maybe_unused attribute to tell the
> > compiler it's fine to do dead code elimination, and suppress the
> > warnings.
> > 
> > Also, remove the "inline" keyword from C files, since the compiler is
> > free anyway to inline or not.
> > 
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> clang is adequately warning that the txchk_{read,write}l functions are not
> used at all, so while your patch is correct, I think we could also go with
> this one liner in addition, or as a replacement to your patch:
> 
> diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c
> b/drivers/net/ethernet/broadcom/bcmsysport.c
> index c9faa8540859..7cea30eac83a 100644
> --- a/drivers/net/ethernet/broadcom/bcmsysport.c
> +++ b/drivers/net/ethernet/broadcom/bcmsysport.c
> @@ -46,7 +46,6 @@ BCM_SYSPORT_IO_MACRO(umac, SYS_PORT_UMAC_OFFSET);
>  BCM_SYSPORT_IO_MACRO(gib, SYS_PORT_GIB_OFFSET);
>  BCM_SYSPORT_IO_MACRO(tdma, SYS_PORT_TDMA_OFFSET);
>  BCM_SYSPORT_IO_MACRO(rxchk, SYS_PORT_RXCHK_OFFSET);
> -BCM_SYSPORT_IO_MACRO(txchk, SYS_PORT_TXCHK_OFFSET);
>  BCM_SYSPORT_IO_MACRO(rbuf, SYS_PORT_RBUF_OFFSET);
>  BCM_SYSPORT_IO_MACRO(tbuf, SYS_PORT_TBUF_OFFSET);
>  BCM_SYSPORT_IO_MACRO(topctrl, SYS_PORT_TOPCTRL_OFFSET);
> -- 
> Florian

As a maintainer, you know best about how much to preserve from currently
unused code, in the idea that it might get used later.

Should I also delete this?

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index 9332a9390f0d..113d4251a243 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -46,9 +46,7 @@ BCM_SYSPORT_IO_MACRO(umac, SYS_PORT_UMAC_OFFSET);
 BCM_SYSPORT_IO_MACRO(gib, SYS_PORT_GIB_OFFSET);
 BCM_SYSPORT_IO_MACRO(tdma, SYS_PORT_TDMA_OFFSET);
 BCM_SYSPORT_IO_MACRO(rxchk, SYS_PORT_RXCHK_OFFSET);
-BCM_SYSPORT_IO_MACRO(txchk, SYS_PORT_TXCHK_OFFSET);
 BCM_SYSPORT_IO_MACRO(rbuf, SYS_PORT_RBUF_OFFSET);
-BCM_SYSPORT_IO_MACRO(tbuf, SYS_PORT_TBUF_OFFSET);
 BCM_SYSPORT_IO_MACRO(topctrl, SYS_PORT_TOPCTRL_OFFSET);
 
 /* On SYSTEMPORT Lite, any register after RDMA_STATUS has the exact
diff --git a/drivers/net/ethernet/broadcom/bcmsysport.h b/drivers/net/ethernet/broadcom/bcmsysport.h
index 335cf6631db5..55d72a16efcc 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.h
+++ b/drivers/net/ethernet/broadcom/bcmsysport.h
@@ -168,10 +168,6 @@ struct bcm_rsb {
 #define RXCHK_BRCM_TAG_CID_SHIFT	16
 #define RXCHK_BRCM_TAG_CID_MASK		0xff
 
-/* TXCHCK offsets and defines */
-#define SYS_PORT_TXCHK_OFFSET		0x380
-#define TXCHK_PKT_RDY_THRESH		0x00
-
 /* Receive buffer offset and defines */
 #define SYS_PORT_RBUF_OFFSET		0x400
 
@@ -202,16 +198,6 @@ struct bcm_rsb {
 #define RBUF_OVFL_DISC_CNTR		0x0c
 #define RBUF_ERR_PKT_CNTR		0x10
 
-/* Transmit buffer offset and defines */
-#define SYS_PORT_TBUF_OFFSET		0x600
-
-#define TBUF_CONTROL			0x00
-#define  TBUF_BP_EN			(1 << 0)
-#define  TBUF_MAX_PKT_THRESH_SHIFT	1
-#define  TBUF_MAX_PKT_THRESH_MASK	0x1f
-#define  TBUF_FULL_THRESH_SHIFT		8
-#define  TBUF_FULL_THRESH_MASK		0x1f
-
 /* UniMAC offset and defines */
 #define SYS_PORT_UMAC_OFFSET		0x800
 

They shouldn't contribute to the generated code size anyway.
Plus, __maybe_unused would allow only the readl() or the writel()
function to be used, which doesn't seem to have been needed so far,
though.

Something I haven't thought of, until now, is that if the I/O macros
were defined as static inline in a header rather than C file, the
compiler wouldn't have warned. So I guess that option is also on the
table if we're keeping after all.

Re: [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers

Posted by Simon Horman 1 month, 1 week ago

On Mon, Oct 14, 2024 at 06:01:38PM +0300, Vladimir Oltean wrote:
> A clang-16 W=1 build emits the following (abridged):
> 
> warning: unused function 'txchk_readl' [-Wunused-function]
> BCM_SYSPORT_IO_MACRO(txchk, SYS_PORT_TXCHK_OFFSET);
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> warning: unused function 'txchk_writel' [-Wunused-function]
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> warning: unused function 'tbuf_readl' [-Wunused-function]
> BCM_SYSPORT_IO_MACRO(tbuf, SYS_PORT_TBUF_OFFSET);
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> warning: unused function 'tbuf_writel' [-Wunused-function]
> note: expanded from macro 'BCM_SYSPORT_IO_MACRO'
> 
> Annotate the functions with the __maybe_unused attribute to tell the
> compiler it's fine to do dead code elimination, and suppress the
> warnings.
> 
> Also, remove the "inline" keyword from C files, since the compiler is
> free anyway to inline or not.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <horms@kernel.org>

...