From: Song, Yoong Siang > Sent: 14 April 2023 12:16 ... > >I have checked Foxville manual for SRRCTL (Split and Replication Receive > >Control) register and below GENMASKs looks correct. > > > >> -#define IGC_SRRCTL_BSIZEPKT_SHIFT 10 /* Shift _right_ */ > >> -#define IGC_SRRCTL_BSIZEHDRSIZE_SHIFT 2 /* Shift _left_ */ > >> +#define IGC_SRRCTL_BSIZEPKT_MASK GENMASK(6, 0) > >> +#define IGC_SRRCTL_BSIZEPKT_SHIFT 10 /* Shift _right_ */ > > > >Shift due to 1 KB resolution of BSIZEPKT (manual field BSIZEPACKET) > > Ya, 1K = BIT(10), so need to shift right 10 bits. I bet the code would be easier to read if it did 'value / 1024u'. The object code will be (much) the same. > >> +#define IGC_SRRCTL_BSIZEHDRSIZE_MASK GENMASK(13, 8) > >> +#define IGC_SRRCTL_BSIZEHDRSIZE_SHIFT 2 /* Shift _left_ */ > > > >This shift is suspicious, but as you inherited it I guess it works. > >I did the math, and it happens to work, knowing (from manual) value is in 64 bytes > >resolution. > > It is in 64 = BIT(6) resolution, so need to shift right 6 bits. > But it start on 8th bit, so need to shift left 8 bits. > Thus, total = shift left 2 bits. > > I dint put the explanation into the header file because it is too lengthy > and user can know from databook. > > How do you feel on the necessary of explaining the shifting logic? Not everyone trying to grok the code will have the manual. Even writing (8 - 6) will help. Or (I think) if the value is in bits 13-8 in units of 64 then just: ((value >> 8) & 0x1f) * 64 gcc will do a single shift right and a mask 9at some point). You might want some defines, but if they aren't used much just comments that refer to the names in the manual/datasheet can be enough. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
