From: Jakub Kicinski > Sent: 05 November 2020 22:38 > On Wed, 4 Nov 2020 16:48:52 +0100 Andrew Lunn wrote: > > drivers/net/ethernet/smsc/smc91x.c:706:51: warning: variable ‘pkt_len’ set > > but not used [-Wunused- > but-set-variable] > > 706 | unsigned int saved_packet, packet_no, tx_status, pkt_len; > > > > Add a new macro for getting fields out of the header, which only gets > > the status, not the length which in this case is not needed. > > > > Signed-off-by: Andrew Lunn <and...@lunn.ch> > > Sorry I missed something on v1 > > > +#define SMC_GET_PKT_HDR_STATUS(lp, status) \ > > + do { \ > > + if (SMC_32BIT(lp)) { \ > > + unsigned int __val = SMC_inl(ioaddr, DATA_REG(lp)); \ > > + (status) = __val & 0xffff; \ > > + } else { \ > > + (status) = SMC_inw(ioaddr, DATA_REG(lp)); \ > > + } \ > > + } while (0) > > This is the original/full macro: > > #define SMC_GET_PKT_HDR(lp, status, length) \ > do { \ > if (SMC_32BIT(lp)) { \ > unsigned int __val = SMC_inl(ioaddr, DATA_REG(lp)); \ > (status) = __val & 0xffff; \ > (length) = __val >> 16; \ > } else { \ > (status) = SMC_inw(ioaddr, DATA_REG(lp)); \ > (length) = SMC_inw(ioaddr, DATA_REG(lp)); \ > } \ > } while (0) > > Note that it reads the same address twice in the else branch. > > I'm 90% sure we can't remove the read here either so best treat it > like the ones in patch 3, right?
One of the two SMC_inw() needs to use 'ioaddr + 2'. Probably the one for (length). The code may also be buggy on BE systems. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)