[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8b080760-c1c7-4d9d-a17b-3c0115392b36@csgroup.eu>
Date: Fri, 10 Jan 2025 12:07:25 +0100
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Simon Horman <horms@...nel.org>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH net-next] freescale: ucc_geth: Remove set but unused
variables
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@...nel.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);
>
>
Powered by blists - more mailing lists