diff mbox series

[net-next] freescale: ucc_geth: Remove set but unused variables

Message ID 20250110-ucc-unused-var-v1-1-4cf02475b21d@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] freescale: ucc_geth: Remove set but unused variables | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 1 maintainers not CCed: maxime.chevallier@bootlin.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 126 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Simon Horman Jan. 10, 2025, 10:18 a.m. UTC 
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(-)

Comments

Christophe Leroy Jan. 10, 2025, 11:07 a.m. UTC | #1 
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);
> 
>
Simon Horman Jan. 10, 2025, 12:45 p.m. UTC | #2 
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]];

...
diff mbox series

Patch

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);