Hi Matthew, On Thu, Sep 8, 2022 at 7:34 PM <matthew.gerlach@xxxxxxxxxxxxxxx> wrote: > On Thu, 8 Sep 2022, Andy Shevchenko wrote: > > On Wed, Sep 07, 2022 at 02:37:32PM -0700, matthew.gerlach@xxxxxxxxxxxxxxx wrote: > >> On Tue, 6 Sep 2022, Andy Shevchenko wrote: > >>> On Tue, Sep 06, 2022 at 12:04:25PM -0700, matthew.gerlach@xxxxxxxxxxxxxxx wrote: > > > > ... > > > >>>> + if (fid != FEATURE_ID_AFU && fid != PORT_FEATURE_ID_ERROR && > >>>> + fid != PORT_FEATURE_ID_UINT && fid != FME_FEATURE_ID_GLOBAL_ERR) { > >>>> + v = readq(base); > >>>> + v = FIELD_GET(DFH_VERSION, v); > >>>> + > >>>> + if (v == 1) { > >>>> + v = readq(base + DFHv1_CSR_SIZE_GRP); > >>> > >>> I am already lost what v keeps... > >>> > >>> Perhaps > >>> > >>> v = readq(base); > >>> switch (FIELD_GET(DFH_VERSION, v)) { > >>> case 1: > >>> ... > >>> break; > >>> } > >> > >> How about? > >> if (FIELD_GET(DFH_VERSION, readq(base)) == 1) { > >> ... > >> } > > > > This one tends to be expanded in the future, so I would keep it switch case. > > > > I'm okay with using the switch statement, but how about the following? > > switch (FIELD_GET(DFH_VERSION, readq(base))) { > case 1: > ... > break; > } Would it make sense to print an error if a newer version than 1 is detected? BTW, what is the expected value when DFHv1 is not detected? Zero or an arbitrary number? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds