On Fri, Nov 9, 2018 at 4:42 PM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > On Fri, Nov 9, 2018 at 5:29 AM Sunil Kovvuri <sunil.kovvuri@xxxxxxxxx> wrote: > > On Fri, Nov 9, 2018 at 2:17 AM Arnd Bergmann <arnd@xxxxxxxx> wrote: > > > On Thu, Nov 8, 2018 at 7:37 PM <sunil.kovvuri@xxxxxxxxx> wrote: > > > > > > > Here is another instance of bitfields in an interface structure. As > > > before, please try to avoid doing that and use bit shifts and masks > > > instead. > > > > > > Arnd > > > > No, this struct is not part of communication interface. > > This is used to fill up a register in a bit more readable fashion > > instead of plain bit shifts. > > But this is still an interface, isn't it? Writing to the register > implies that there is some hardware that interprets the > bits, so they have to be in the right place. > > > === > > struct nix_rx_vtag_action vtag_action; > > > > *(u64 *)&vtag_action = 0; > > vtag_action.vtag0_valid = 1; > > /* must match type set in NIX_VTAG_CFG */ > > vtag_action.vtag0_type = 0; > > vtag_action.vtag0_lid = NPC_LID_LA; > > vtag_action.vtag0_relptr = 12; > > entry.vtag_action = *(u64 *)&vtag_action; > > > > /* Set TAG 'action' */ > > rvu_write64(rvu, blkaddr, NPC_AF_MCAMEX_BANKX_TAG_ACT(index, actbank), > > entry->vtag_action); > > I assume this rvu_write64() does a cpu_to_le64() swap on big-endian, > so the contents again are in the wrong place. I don't see any non-reserved > fields that span an 8-bit boundary, so you can probably rearrange the bits > to make it work, but generally speaking it's better to not rely on how the > compiler lays out bit fields. > > Arnd Agreed. Will fix and submit a new series. Thanks, Sunil.
