* [PATCH net-next] freescale: ucc_geth: Remove set but unused variables
@ 2025-01-10 10:18 Simon Horman
2025-01-10 11:07 ` Christophe Leroy
0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2025-01-10 10:18 UTC (permalink / raw)
To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: netdev, linuxppc-dev
Remove set but unused variables. These seem to provide no value.
So in the spirit of less being more, remove them.
Flagged by gcc-14 as:
.../ucc_geth.c: In function 'ucc_geth_free_rx':
.../ucc_geth.c:1708:31: warning: variable 'uf_info' set but not used [-Wunused-but-set-variable]
1708 | struct ucc_fast_info *uf_info;
| ^~~~~~~
.../ucc_geth.c: In function 'ucc_geth_free_tx':
.../ucc_geth.c:1747:31: warning: variable 'uf_info' set but not used [-Wunused-but-set-variable]
1747 | struct ucc_fast_info *uf_info;
| ^~~~~~~
.../ucc_geth.c: In function 'ucc_geth_alloc_tx':
.../ucc_geth.c:2039:31: warning: variable 'uf_info' set but not used [-Wunused-but-set-variable]
2039 | struct ucc_fast_info *uf_info;
| ^~~~~~~
.../ucc_geth.c: In function 'ucc_geth_alloc_rx':
.../ucc_geth.c:2101:31: warning: variable 'uf_info' set but not used [-Wunused-but-set-variable]
2101 | struct ucc_fast_info *uf_info;
| ^~~~~~~
.../ucc_geth.c: In function 'ucc_geth_startup':
.../ucc_geth.c:2168:13: warning: variable 'ifstat' set but not used [-Wunused-but-set-variable]
2168 | u32 ifstat, i, j, size, l2qt, l3qt;
| ^~~~~~
.../ucc_geth.c:2158:62: warning: variable 'p_82xx_addr_filt' set but not used [-Wunused-but-set-variable]
2158 | struct ucc_geth_82xx_address_filtering_pram __iomem *p_82xx_addr_filt;
| ^~~~~~~~~~~~~~~~
.../ucc_geth.c: In function 'ucc_geth_rx':
.../ucc_geth.c:2893:13: warning: variable 'bdBuffer' set but not used [-Wunused-but-set-variable]
2893 | u8 *bdBuffer;
| ^~~~~~~~
.../ucc_geth.c: In function 'ucc_geth_irq_handler':
.../ucc_geth.c:3046:31: warning: variable 'ug_info' set but not used [-Wunused-but-set-variable]
3046 | struct ucc_geth_info *ug_info;
| ^~~~~~~
Compile tested only.
No runtime effect intended.
Signed-off-by: Simon Horman <horms@kernel.org>
---
drivers/net/ethernet/freescale/ucc_geth.c | 39 +++++++------------------------
1 file changed, 8 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 88510f822759..1e3a1cb997c3 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1704,14 +1704,8 @@ static int ugeth_82xx_filtering_clear_addr_in_paddr(struct ucc_geth_private *uge
static void ucc_geth_free_rx(struct ucc_geth_private *ugeth)
{
- struct ucc_geth_info *ug_info;
- struct ucc_fast_info *uf_info;
- u16 i, j;
u8 __iomem *bd;
-
-
- ug_info = ugeth->ug_info;
- uf_info = &ug_info->uf_info;
+ u16 i, j;
for (i = 0; i < ucc_geth_rx_queues(ugeth->ug_info); i++) {
if (ugeth->p_rx_bd_ring[i]) {
@@ -1743,16 +1737,11 @@ static void ucc_geth_free_rx(struct ucc_geth_private *ugeth)
static void ucc_geth_free_tx(struct ucc_geth_private *ugeth)
{
- struct ucc_geth_info *ug_info;
- struct ucc_fast_info *uf_info;
- u16 i, j;
u8 __iomem *bd;
+ u16 i, j;
netdev_reset_queue(ugeth->ndev);
- ug_info = ugeth->ug_info;
- uf_info = &ug_info->uf_info;
-
for (i = 0; i < ucc_geth_tx_queues(ugeth->ug_info); i++) {
bd = ugeth->p_tx_bd_ring[i];
if (!bd)
@@ -2036,13 +2025,11 @@ static int ucc_struct_init(struct ucc_geth_private *ugeth)
static int ucc_geth_alloc_tx(struct ucc_geth_private *ugeth)
{
struct ucc_geth_info *ug_info;
- struct ucc_fast_info *uf_info;
+ u8 __iomem *bd;
int length;
u16 i, j;
- u8 __iomem *bd;
ug_info = ugeth->ug_info;
- uf_info = &ug_info->uf_info;
/* Allocate Tx bds */
for (j = 0; j < ucc_geth_tx_queues(ug_info); j++) {
@@ -2098,13 +2085,11 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private *ugeth)
static int ucc_geth_alloc_rx(struct ucc_geth_private *ugeth)
{
struct ucc_geth_info *ug_info;
- struct ucc_fast_info *uf_info;
+ u8 __iomem *bd;
int length;
u16 i, j;
- u8 __iomem *bd;
ug_info = ugeth->ug_info;
- uf_info = &ug_info->uf_info;
/* Allocate Rx bds */
for (j = 0; j < ucc_geth_rx_queues(ug_info); j++) {
@@ -2155,7 +2140,6 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private *ugeth)
static int ucc_geth_startup(struct ucc_geth_private *ugeth)
{
- struct ucc_geth_82xx_address_filtering_pram __iomem *p_82xx_addr_filt;
struct ucc_geth_init_pram __iomem *p_init_enet_pram;
struct ucc_fast_private *uccf;
struct ucc_geth_info *ug_info;
@@ -2165,8 +2149,8 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
int ret_val = -EINVAL;
u32 remoder = UCC_GETH_REMODER_INIT;
u32 init_enet_pram_offset, cecr_subblock, command;
- u32 ifstat, i, j, size, l2qt, l3qt;
u16 temoder = UCC_GETH_TEMODER_INIT;
+ u32 i, j, size, l2qt, l3qt;
u8 function_code = 0;
u8 __iomem *endOfRing;
u8 numThreadsRxNumerical, numThreadsTxNumerical;
@@ -2260,7 +2244,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
/* Set IFSTAT */
/* For more details see the hardware spec. */
/* Read only - resets upon read */
- ifstat = in_be32(&ug_regs->ifstat);
+ in_be32(&ug_regs->ifstat);
/* Clear UEMPR */
/* For more details see the hardware spec. */
@@ -2651,10 +2635,6 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
for (j = 0; j < NUM_OF_PADDRS; j++)
ugeth_82xx_filtering_clear_addr_in_paddr(ugeth, (u8) j);
- p_82xx_addr_filt =
- (struct ucc_geth_82xx_address_filtering_pram __iomem *) ugeth->
- p_rx_glbl_pram->addressfiltering;
-
ugeth_82xx_filtering_clear_all_addr_in_hash(ugeth,
ENET_ADDR_TYPE_GROUP);
ugeth_82xx_filtering_clear_all_addr_in_hash(ugeth,
@@ -2889,9 +2869,8 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
struct sk_buff *skb;
u8 __iomem *bd;
u16 length, howmany = 0;
- u32 bd_status;
- u8 *bdBuffer;
struct net_device *dev;
+ u32 bd_status;
ugeth_vdbg("%s: IN", __func__);
@@ -2904,7 +2883,7 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
/* while there are received buffers and BD is full (~R_E) */
while (!((bd_status & (R_E)) || (--rx_work_limit < 0))) {
- bdBuffer = (u8 *) in_be32(&((struct qe_bd __iomem *)bd)->buf);
+ in_be32(&((struct qe_bd __iomem *)bd)->buf);
length = (u16) ((bd_status & BD_LENGTH_MASK) - 4);
skb = ugeth->rx_skbuff[rxQ][ugeth->skb_currx[rxQ]];
@@ -3043,14 +3022,12 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
struct net_device *dev = info;
struct ucc_geth_private *ugeth = netdev_priv(dev);
struct ucc_fast_private *uccf;
- struct ucc_geth_info *ug_info;
register u32 ucce;
register u32 uccm;
ugeth_vdbg("%s: IN", __func__);
uccf = ugeth->uccf;
- ug_info = ugeth->ug_info;
/* read and clear events */
ucce = (u32) in_be32(uccf->p_ucce);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] freescale: ucc_geth: Remove set but unused variables
2025-01-10 10:18 [PATCH net-next] freescale: ucc_geth: Remove set but unused variables Simon Horman
@ 2025-01-10 11:07 ` Christophe Leroy
2025-01-10 12:45 ` Simon Horman
0 siblings, 1 reply; 3+ messages in thread
From: Christophe Leroy @ 2025-01-10 11:07 UTC (permalink / raw)
To: Simon Horman, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linuxppc-dev
Le 10/01/2025 à 11:18, Simon Horman a écrit :
> Remove set but unused variables. These seem to provide no value.
> So in the spirit of less being more, remove them.
Would be good to identify when those variables became unused.
There is for instance commit 64a99fe596f9 ("ethernet: ucc_geth: remove
bd_mem_part and all associated code")
...
>
> Compile tested only.
> No runtime effect intended.
>
> Signed-off-by: Simon Horman <horms@kernel.org>
As you are playing with that driver, there are also sparse warnings to
be fixed, getting plenty when building with C=2
> ---
> drivers/net/ethernet/freescale/ucc_geth.c | 39 +++++++------------------------
> 1 file changed, 8 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index 88510f822759..1e3a1cb997c3 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -1704,14 +1704,8 @@ static int ugeth_82xx_filtering_clear_addr_in_paddr(struct ucc_geth_private *uge
>
> static void ucc_geth_free_rx(struct ucc_geth_private *ugeth)
> {
> - struct ucc_geth_info *ug_info;
> - struct ucc_fast_info *uf_info;
> - u16 i, j;
> u8 __iomem *bd;
> -
> -
> - ug_info = ugeth->ug_info;
> - uf_info = &ug_info->uf_info;
> + u16 i, j;
Why do you need to move this declaration ? Looks like cosmetics. That
goes beyond the purpose of this patch which is already big enough and
should be avoided. The same applies several times in this patch.
>
> for (i = 0; i < ucc_geth_rx_queues(ugeth->ug_info); i++) {
> if (ugeth->p_rx_bd_ring[i]) {
> @@ -1743,16 +1737,11 @@ static void ucc_geth_free_rx(struct ucc_geth_private *ugeth)
>
> static void ucc_geth_free_tx(struct ucc_geth_private *ugeth)
> {
> - struct ucc_geth_info *ug_info;
> - struct ucc_fast_info *uf_info;
> - u16 i, j;
> u8 __iomem *bd;
> + u16 i, j;
>
> netdev_reset_queue(ugeth->ndev);
>
> - ug_info = ugeth->ug_info;
> - uf_info = &ug_info->uf_info;
> -
> for (i = 0; i < ucc_geth_tx_queues(ugeth->ug_info); i++) {
> bd = ugeth->p_tx_bd_ring[i];
> if (!bd)
> @@ -2036,13 +2025,11 @@ static int ucc_struct_init(struct ucc_geth_private *ugeth)
> static int ucc_geth_alloc_tx(struct ucc_geth_private *ugeth)
> {
> struct ucc_geth_info *ug_info;
> - struct ucc_fast_info *uf_info;
> + u8 __iomem *bd;
> int length;
> u16 i, j;
> - u8 __iomem *bd;
>
> ug_info = ugeth->ug_info;
> - uf_info = &ug_info->uf_info;
>
> /* Allocate Tx bds */
> for (j = 0; j < ucc_geth_tx_queues(ug_info); j++) {
> @@ -2098,13 +2085,11 @@ static int ucc_geth_alloc_tx(struct ucc_geth_private *ugeth)
> static int ucc_geth_alloc_rx(struct ucc_geth_private *ugeth)
> {
> struct ucc_geth_info *ug_info;
> - struct ucc_fast_info *uf_info;
> + u8 __iomem *bd;
> int length;
> u16 i, j;
> - u8 __iomem *bd;
>
> ug_info = ugeth->ug_info;
> - uf_info = &ug_info->uf_info;
>
> /* Allocate Rx bds */
> for (j = 0; j < ucc_geth_rx_queues(ug_info); j++) {
> @@ -2155,7 +2140,6 @@ static int ucc_geth_alloc_rx(struct ucc_geth_private *ugeth)
>
> static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> {
> - struct ucc_geth_82xx_address_filtering_pram __iomem *p_82xx_addr_filt;
> struct ucc_geth_init_pram __iomem *p_init_enet_pram;
> struct ucc_fast_private *uccf;
> struct ucc_geth_info *ug_info;
> @@ -2165,8 +2149,8 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> int ret_val = -EINVAL;
> u32 remoder = UCC_GETH_REMODER_INIT;
> u32 init_enet_pram_offset, cecr_subblock, command;
> - u32 ifstat, i, j, size, l2qt, l3qt;
> u16 temoder = UCC_GETH_TEMODER_INIT;
> + u32 i, j, size, l2qt, l3qt;
> u8 function_code = 0;
> u8 __iomem *endOfRing;
> u8 numThreadsRxNumerical, numThreadsTxNumerical;
> @@ -2260,7 +2244,7 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> /* Set IFSTAT */
> /* For more details see the hardware spec. */
> /* Read only - resets upon read */
> - ifstat = in_be32(&ug_regs->ifstat);
> + in_be32(&ug_regs->ifstat);
>
> /* Clear UEMPR */
> /* For more details see the hardware spec. */
> @@ -2651,10 +2635,6 @@ static int ucc_geth_startup(struct ucc_geth_private *ugeth)
> for (j = 0; j < NUM_OF_PADDRS; j++)
> ugeth_82xx_filtering_clear_addr_in_paddr(ugeth, (u8) j);
>
> - p_82xx_addr_filt =
> - (struct ucc_geth_82xx_address_filtering_pram __iomem *) ugeth->
> - p_rx_glbl_pram->addressfiltering;
> -
> ugeth_82xx_filtering_clear_all_addr_in_hash(ugeth,
> ENET_ADDR_TYPE_GROUP);
> ugeth_82xx_filtering_clear_all_addr_in_hash(ugeth,
> @@ -2889,9 +2869,8 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
> struct sk_buff *skb;
> u8 __iomem *bd;
> u16 length, howmany = 0;
> - u32 bd_status;
> - u8 *bdBuffer;
> struct net_device *dev;
> + u32 bd_status;
>
> ugeth_vdbg("%s: IN", __func__);
>
> @@ -2904,7 +2883,7 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
>
> /* while there are received buffers and BD is full (~R_E) */
> while (!((bd_status & (R_E)) || (--rx_work_limit < 0))) {
> - bdBuffer = (u8 *) in_be32(&((struct qe_bd __iomem *)bd)->buf);
> + in_be32(&((struct qe_bd __iomem *)bd)->buf);
This line should go completely.
> length = (u16) ((bd_status & BD_LENGTH_MASK) - 4);
> skb = ugeth->rx_skbuff[rxQ][ugeth->skb_currx[rxQ]];
>
> @@ -3043,14 +3022,12 @@ static irqreturn_t ucc_geth_irq_handler(int irq, void *info)
> struct net_device *dev = info;
> struct ucc_geth_private *ugeth = netdev_priv(dev);
> struct ucc_fast_private *uccf;
> - struct ucc_geth_info *ug_info;
> register u32 ucce;
> register u32 uccm;
>
> ugeth_vdbg("%s: IN", __func__);
>
> uccf = ugeth->uccf;
> - ug_info = ugeth->ug_info;
>
> /* read and clear events */
> ucce = (u32) in_be32(uccf->p_ucce);
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net-next] freescale: ucc_geth: Remove set but unused variables
2025-01-10 11:07 ` Christophe Leroy
@ 2025-01-10 12:45 ` Simon Horman
0 siblings, 0 replies; 3+ messages in thread
From: Simon Horman @ 2025-01-10 12:45 UTC (permalink / raw)
To: Christophe Leroy
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, netdev, linuxppc-dev
On Fri, Jan 10, 2025 at 12:07:25PM +0100, Christophe Leroy wrote:
>
>
> Le 10/01/2025 à 11:18, Simon Horman a écrit :
> > Remove set but unused variables. These seem to provide no value.
> > So in the spirit of less being more, remove them.
>
> Would be good to identify when those variables became unused.
>
> There is for instance commit 64a99fe596f9 ("ethernet: ucc_geth: remove
> bd_mem_part and all associated code")
Sure, I can work on an updated commit message for v2 along those lines.
>
> ...
>
> >
> > Compile tested only.
> > No runtime effect intended.
> >
> > Signed-off-by: Simon Horman <horms@kernel.org>
>
> As you are playing with that driver, there are also sparse warnings to be
> fixed, getting plenty when building with C=2
Yes, I noticed.
That is on my todo list :)
>
> > ---
> > drivers/net/ethernet/freescale/ucc_geth.c | 39 +++++++------------------------
> > 1 file changed, 8 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> > index 88510f822759..1e3a1cb997c3 100644
> > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > @@ -1704,14 +1704,8 @@ static int ugeth_82xx_filtering_clear_addr_in_paddr(struct ucc_geth_private *uge
> > static void ucc_geth_free_rx(struct ucc_geth_private *ugeth)
> > {
> > - struct ucc_geth_info *ug_info;
> > - struct ucc_fast_info *uf_info;
> > - u16 i, j;
> > u8 __iomem *bd;
> > -
> > -
> > - ug_info = ugeth->ug_info;
> > - uf_info = &ug_info->uf_info;
> > + u16 i, j;
>
> Why do you need to move this declaration ? Looks like cosmetics. That goes
> beyond the purpose of this patch which is already big enough and should be
> avoided. The same applies several times in this patch.
It seemed convenient to move this around at the same time,
as the lines are adjacent. But will drop them as you wish.
>
> > for (i = 0; i < ucc_geth_rx_queues(ugeth->ug_info); i++) {
> > if (ugeth->p_rx_bd_ring[i]) {
...
> > @@ -2904,7 +2883,7 @@ static int ucc_geth_rx(struct ucc_geth_private *ugeth, u8 rxQ, int rx_work_limit
> > /* while there are received buffers and BD is full (~R_E) */
> > while (!((bd_status & (R_E)) || (--rx_work_limit < 0))) {
> > - bdBuffer = (u8 *) in_be32(&((struct qe_bd __iomem *)bd)->buf);
> > + in_be32(&((struct qe_bd __iomem *)bd)->buf);
>
> This line should go completely.
Thanks,
I was slightly concerned that it may have some side effect - that
I have no way to test. But I will remove it on your advice.
>
> > length = (u16) ((bd_status & BD_LENGTH_MASK) - 4);
> > skb = ugeth->rx_skbuff[rxQ][ugeth->skb_currx[rxQ]];
...
--
pw-bot: changes-requested
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-01-10 12:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-10 10:18 [PATCH net-next] freescale: ucc_geth: Remove set but unused variables Simon Horman
2025-01-10 11:07 ` Christophe Leroy
2025-01-10 12:45 ` Simon Horman
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).