* [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers
@ 2024-10-14 15:01 Vladimir Oltean
2024-10-14 16:12 ` Simon Horman
2024-10-14 16:56 ` Florian Fainelli
0 siblings, 2 replies; 4+ messages in thread
From: Vladimir Oltean @ 2024-10-14 15:01 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, Broadcom internal kernel review list,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-kernel
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers
2024-10-14 15:01 [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers Vladimir Oltean
@ 2024-10-14 16:12 ` Simon Horman
2024-10-14 16:56 ` Florian Fainelli
1 sibling, 0 replies; 4+ messages in thread
From: Simon Horman @ 2024-10-14 16:12 UTC (permalink / raw)
To: Vladimir Oltean
Cc: netdev, Florian Fainelli, Broadcom internal kernel review list,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
linux-kernel
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>
...
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers
2024-10-14 15:01 [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers Vladimir Oltean
2024-10-14 16:12 ` Simon Horman
@ 2024-10-14 16:56 ` Florian Fainelli
2024-10-14 17:40 ` Vladimir Oltean
1 sibling, 1 reply; 4+ messages in thread
From: Florian Fainelli @ 2024-10-14 16:56 UTC (permalink / raw)
To: Vladimir Oltean, netdev
Cc: Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers
2024-10-14 16:56 ` Florian Fainelli
@ 2024-10-14 17:40 ` Vladimir Oltean
0 siblings, 0 replies; 4+ messages in thread
From: Vladimir Oltean @ 2024-10-14 17:40 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, Broadcom internal kernel review list, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-kernel
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.
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-10-14 17:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-14 15:01 [PATCH net-next] net: systemport: avoid build warnings due to unused I/O helpers Vladimir Oltean
2024-10-14 16:12 ` Simon Horman
2024-10-14 16:56 ` Florian Fainelli
2024-10-14 17:40 ` Vladimir Oltean
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).