On Mon, May 17, 2021 at 11:11:07AM +0200, Takashi Iwai wrote: > On Sat, 15 May 2021 09:11:09 +0200, > Takashi Sakamoto wrote: > > --- a/sound/firewire/oxfw/oxfw.h > > +++ b/sound/firewire/oxfw/oxfw.h > > @@ -32,6 +32,12 @@ > > #include "../amdtp-am824.h" > > #include "../cmp.h" > > > > +enum snd_oxfw_quirk { > > + // Postpone transferring packets during handling asynchronous transaction. As a result, > > + // next isochronous packet includes more events than one packet can include. > > + SND_OXFW_QUIRK_JUMBO_PAYLOAD = 0x01, > > +}; > > + > > /* This is an arbitrary number for convinience. */ > > #define SND_OXFW_STREAM_FORMAT_ENTRIES 10 > > struct snd_oxfw { > > @@ -43,6 +49,7 @@ struct snd_oxfw { > > bool registered; > > struct delayed_work dwork; > > > > + enum snd_oxfw_quirk quirks; > > Declaring the field as this enum type for bit flags isn't really > right, IMO. Usually an enum *type* is used for storing only the > enumerated values as-is, but the actual code (in a later patch) stores > the combination of the defined values as bits. > That is, if a field is defined with an enum type, readers and > compilers may believe that only the defined values are stored there, > while the code doesn't follow that, which is a confusing situation. > > I see that a similar pattern is used already in the firewire code, but > I don't think this justifies to introduce it to yet another place. The concern is in the category of human practice, and heuristics, in my opinion. I check C language specification and it says that enumeration-constant has type int, and enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type and the choice of type is implementation-defined. The assignment of ORed enumeration-constant (int) to enumerated type (int with 32 bit storage in most System V ABIs) is not forbidden past and future though the specification mentions about its warnings in the annex. Nevertheless, the practical point should be respected as well. I'll prepare take 3 patchset including fix for some issued points. Thanks Takashi Sakamoto