| Message ID | 20210518024326.67576-3-o-takashi@sakamocchi.jp (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ALSA: oxfw: code refactoring for quirks specific to ASICs | expand |
On Tue, 18 May 2021 04:43:17 +0200, Takashi Sakamoto wrote: > > It brings some inconvenience in practice to use enumerated type for > variable to which bitwise OR with enumerator constant is assigned. > > This commit replaces declarations of enumerated type with int type. Better to use unsigned int for bit flags. Otherwise the highest bit becomes harder to use. thanks, Takashi
On Tue, May 18, 2021 at 09:02:54AM +0200, Takashi Iwai wrote: > On Tue, 18 May 2021 04:43:17 +0200, > Takashi Sakamoto wrote: > > > > It brings some inconvenience in practice to use enumerated type for > > variable to which bitwise OR with enumerator constant is assigned. > > > > This commit replaces declarations of enumerated type with int type. > > Better to use unsigned int for bit flags. Otherwise the highest bit > becomes harder to use. I can't imagine such situation that the signed value causes issue. Would I request actual example with such issue? At least, the highest bit is still available as bit even if the value is negative by assigning 0x80000000... Regards Takashi Sakamoto
On Tue, 18 May 2021 10:13:34 +0200, Takashi Sakamoto wrote: > > On Tue, May 18, 2021 at 09:02:54AM +0200, Takashi Iwai wrote: > > On Tue, 18 May 2021 04:43:17 +0200, > > Takashi Sakamoto wrote: > > > > > > It brings some inconvenience in practice to use enumerated type for > > > variable to which bitwise OR with enumerator constant is assigned. > > > > > > This commit replaces declarations of enumerated type with int type. > > > > Better to use unsigned int for bit flags. Otherwise the highest bit > > becomes harder to use. > > I can't imagine such situation that the signed value causes issue. Would > I request actual example with such issue? At least, the highest bit is > still available as bit even if the value is negative by assigning > 0x80000000... It's available in signed int, but this is inconvenient, e.g. if you shift the bit. Maybe I forgot something else, too. You may still use signed int if you are sure that you'll never reach to the highest number, but other than that, using unsigned for bit flags is a *VERY* common practice in the kernel programming, so there is no reason to ignore it. Takashi
On Tue, May 18, 2021 at 10:23:30AM +0200, Takashi Iwai wrote: > On Tue, 18 May 2021 10:13:34 +0200, > Takashi Sakamoto wrote: > > > > On Tue, May 18, 2021 at 09:02:54AM +0200, Takashi Iwai wrote: > > > On Tue, 18 May 2021 04:43:17 +0200, > > > Takashi Sakamoto wrote: > > > > > > > > It brings some inconvenience in practice to use enumerated type for > > > > variable to which bitwise OR with enumerator constant is assigned. > > > > > > > > This commit replaces declarations of enumerated type with int type. > > > > > > Better to use unsigned int for bit flags. Otherwise the highest bit > > > becomes harder to use. > > > > I can't imagine such situation that the signed value causes issue. Would > > I request actual example with such issue? At least, the highest bit is > > still available as bit even if the value is negative by assigning > > 0x80000000... > > It's available in signed int, but this is inconvenient, e.g. if you > shift the bit. Maybe I forgot something else, too. > > You may still use signed int if you are sure that you'll never reach > to the highest number, but other than that, using unsigned for bit > flags is a *VERY* common practice in the kernel programming, so there > is no reason to ignore it. Ok. I just follow to the convention under the Linux kernel development. Thanks Takashi Sakamoto
diff --git a/sound/firewire/amdtp-am824.c b/sound/firewire/amdtp-am824.c index fea92e148790..9662a3ba9139 100644 --- a/sound/firewire/amdtp-am824.c +++ b/sound/firewire/amdtp-am824.c @@ -410,10 +410,10 @@ static unsigned int process_ir_ctx_payloads(struct amdtp_stream *s, * @s: the AMDTP stream to initialize * @unit: the target of the stream * @dir: the direction of stream - * @flags: the packet transmission method to use + * @flags: the details of the streaming protocol consist of cip_flags enumeration-constants. */ int amdtp_am824_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags) + enum amdtp_stream_direction dir, int flags) { amdtp_stream_process_ctx_payloads_t process_ctx_payloads; diff --git a/sound/firewire/amdtp-am824.h b/sound/firewire/amdtp-am824.h index 06d280783581..fe6652bde4a5 100644 --- a/sound/firewire/amdtp-am824.h +++ b/sound/firewire/amdtp-am824.h @@ -45,5 +45,5 @@ void amdtp_am824_midi_trigger(struct amdtp_stream *s, unsigned int port, struct snd_rawmidi_substream *midi); int amdtp_am824_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags); + enum amdtp_stream_direction dir, int flags); #endif diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index e0faa6601966..094c9b2d772f 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -71,13 +71,13 @@ static void pcm_period_work(struct work_struct *work); * @s: the AMDTP stream to initialize * @unit: the target of the stream * @dir: the direction of stream - * @flags: the packet transmission method to use + * @flags: the details of the streaming protocol consist of cip_flags enumeration-constants. * @fmt: the value of fmt field in CIP header * @process_ctx_payloads: callback handler to process payloads of isoc context * @protocol_size: the size to allocate newly for protocol */ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags, + enum amdtp_stream_direction dir, int flags, unsigned int fmt, amdtp_stream_process_ctx_payloads_t process_ctx_payloads, unsigned int protocol_size) diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index a3daa1f2c1c4..1f294df1d64f 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -112,7 +112,8 @@ typedef unsigned int (*amdtp_stream_process_ctx_payloads_t)( struct amdtp_domain; struct amdtp_stream { struct fw_unit *unit; - enum cip_flags flags; + // The combination of cip_flags enumeration-constants. + int flags; enum amdtp_stream_direction direction; struct mutex mutex; @@ -184,7 +185,7 @@ struct amdtp_stream { }; int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags, + enum amdtp_stream_direction dir, int flags, unsigned int fmt, amdtp_stream_process_ctx_payloads_t process_ctx_payloads, unsigned int protocol_size); diff --git a/sound/firewire/digi00x/amdtp-dot.c b/sound/firewire/digi00x/amdtp-dot.c index d613642a2ce3..a47d9800e0d6 100644 --- a/sound/firewire/digi00x/amdtp-dot.c +++ b/sound/firewire/digi00x/amdtp-dot.c @@ -396,7 +396,7 @@ int amdtp_dot_init(struct amdtp_stream *s, struct fw_unit *unit, enum amdtp_stream_direction dir) { amdtp_stream_process_ctx_payloads_t process_ctx_payloads; - enum cip_flags flags; + int flags; // Use different mode between incoming/outgoing. if (dir == AMDTP_IN_STREAM) { diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index 3d0236ee6716..c0de2720aeac 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -106,7 +106,8 @@ enum snd_motu_protocol_version { struct snd_motu_spec { const char *const name; enum snd_motu_protocol_version protocol_version; - enum snd_motu_spec_flags flags; + // The combination of snd_motu_spec_flags enumeration-constants. + int flags; unsigned char tx_fixed_pcm_chunks[3]; unsigned char rx_fixed_pcm_chunks[3];
It brings some inconvenience in practice to use enumerated type for variable to which bitwise OR with enumerator constant is assigned. This commit replaces declarations of enumerated type with int type. Suggested-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp> --- sound/firewire/amdtp-am824.c | 4 ++-- sound/firewire/amdtp-am824.h | 2 +- sound/firewire/amdtp-stream.c | 4 ++-- sound/firewire/amdtp-stream.h | 5 +++-- sound/firewire/digi00x/amdtp-dot.c | 2 +- sound/firewire/motu/motu.h | 3 ++- 6 files changed, 11 insertions(+), 9 deletions(-)