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.
thanks,
Takashi
