Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i

[Takashi Iwai <tiwai@suse.de>]

At Mon, 22 Apr 2013 13:47:23 +0200 (CEST),
Eldad Zack wrote:
> 
> 
> 
> On Mon, 22 Apr 2013, Takashi Iwai wrote:
> 
> > At Mon, 22 Apr 2013 13:29:41 +0200 (CEST),
> > Eldad Zack wrote:
> > > 
> > > 
> > > 
> > > On Mon, 22 Apr 2013, Takashi Iwai wrote:
> > > 
> > > > At Mon, 22 Apr 2013 01:44:04 +0200,
> > > > Eldad Zack wrote:
> > > > > 
> > > > > Replace the usage of SNDRV_PCM_FORMAT_* macros with the equivalent
> > > > > SNDRV_PCM_FMTBIT_* macros, and the result be can assigned directly
> > > > > to the formats field.
> > > > 
> > > > Note that SNDRV_PCM_FMTBIT_* are 64bit integer.  And it makes sense to
> > > > use FMTBIT only when multiple formats are supposed.  For a single
> > > > format, keeping SNDRV_PCM_FORMAT_* is more reasonable.
> > > > 
> > > > In other words, a proper fix would be to replace the type of variable
> > > > format with snd_pcm_format_t, and cast at converting to format bits
> > > > like
> > > > 	1ULL << (unsigned int)pcm_format
> > > 
> > > I see your point. I just figured making the assignment directly would
> > > make sense just for this case.
> > > 
> > > There are a couple of other places with the same conversion that 
> > > "break" the strong typing - how about something like this:
> > > 
> > > 	#define pcm_format_to_bits(fmt) (1uLL << ((__force int)(fmt)))
> > > 
> > > Or an equivalent function?
> > 
> > Yep, I just wanted to propose that :)
> 
> Cool :)
> 
> I think a function would be better so the format can be checked to use
> the correct type as well, and put it in pcm.h:
> 
> static inline u64 pcm_format_to_bits(snd_pcm_format_t pcm_format)
> {
> 	return 1ULL << (__force int) pcm_format;
> }
> 
> If that looks good I'll search for all the relevant places and change 
> them (in one patch).

It looks good.


thanks,

Takashi