Messages in this thread | | | Date | Thu, 8 Feb 2024 10:07:27 -0500 | Subject | Re: [PATCH][next] drivers: net: xgene: remove redundant assignment to variable offset | From | Vadim Fedorenko <> |
| |
On 08/02/2024 13:40, Dan Carpenter wrote: > On Thu, Feb 08, 2024 at 12:39:24PM +0000, Vadim Fedorenko wrote: >> On 08.02.2024 12:20, Colin Ian King wrote: >>> The variable offset is being initialized with a value that is never >>> read, it is being re-assigned later on in either path of an if >>> statement before being used. The initialization is redundant and >>> can be removed. >>> >>> Cleans up clang scan build warning: >>> drivers/net/ethernet/apm/xgene/xgene_enet_cle.c:736:2: warning: Value >>> stored to 'offset' is never read [deadcode.DeadStores] >>> >>> Signed-off-by: Colin Ian King <colin.i.king@gmail.com> >>> --- >>> drivers/net/ethernet/apm/xgene/xgene_enet_cle.c | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c >>> index de5464322311..8f104642897b 100644 >>> --- a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c >>> +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c >>> @@ -733,7 +733,6 @@ static int xgene_cle_setup_rss(struct xgene_enet_pdata *pdata) >>> u32 offset, val = 0; >>> int i, ret = 0; >>> - offset = CLE_PORT_OFFSET; >>> for (i = 0; i < cle->parsers; i++) { >>> if (cle->active_parser != PARSER_ALL) >>> offset = cle->active_parser * CLE_PORT_OFFSET; >> >> It looks like more refactoring can be done here. >> "if (cle->active_parser != PARSER_ALL)" is static, no need to check it inside >> the loop. >> > > You still need to check... I don't really think it's an improvement. > > regards, > dan carpenter > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c > index de5464322311..61e31cc55771 100644 > --- a/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c > +++ b/drivers/net/ethernet/apm/xgene/xgene_enet_cle.c > @@ -733,11 +733,11 @@ static int xgene_cle_setup_rss(struct xgene_enet_pdata *pdata) > u32 offset, val = 0; > int i, ret = 0; > > - offset = CLE_PORT_OFFSET; > + if (cle->active_parser != PARSER_ALL) > + offset = cle->active_parser * CLE_PORT_OFFSET; > +
I think we can add "else" here and avoid the loop in case of != PARSER_ALL and avoid "if" in the loop, wdyt?
> for (i = 0; i < cle->parsers; i++) { > - if (cle->active_parser != PARSER_ALL) > - offset = cle->active_parser * CLE_PORT_OFFSET; > - else > + if (cle->active_parser == PARSER_ALL) > offset = i * CLE_PORT_OFFSET; > > /* enable RSS */
| |