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 <[email protected]>
>
> 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)
