On Wed, May 1, 2019 at 11:30 AM Kalle Valo <kvalo@xxxxxxxxxxxxxx> wrote: > My comment was about handling firmware commands and events as a byte > array, not about bitfields. So that instead of accessing 'index + 1' and > 'index + 4' you should create a proper struct for the command and access > it using 'cmd->foo' and 'cmd->bar'. Sure, bitfields you still need to > access using FIELD_GET() or similar but having a struct for commands is > a lot cleaner approach. And most upstream drivers do this: ath10k, > ath6kl, iwlwifi, p54 and whatnot. I think I pushed Tony away from the bitfields (he was using a struct plus some ugly bitfields / #ifdefs pre-v8), and he ended up with this (in v8). I noted on the v8 cover letter that one can still use a struct, but just avoid using bitfields -- so you would still have 'u8' and '__le32' fields (or similar), and do the right le32_to_cpu() accessors (sparse will help you) plus FIELD_GET() (for any necessary bitfields). Brian
