From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i Date: Mon, 22 Apr 2013 13:34:27 +0200 Message-ID: References: <1366587844-1252-1-git-send-email-eldad@fogrefinery.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 9C120261630 for ; Mon, 22 Apr 2013 13:34:27 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Eldad Zack Cc: alsa-devel@alsa-project.org, Daniel Mack , Clemens Ladisch List-Id: alsa-devel@alsa-project.org 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 :) Takashi > > Cheers, > Eldad > > > > Modify the variable name to reflect the change. > > > > > > The following sparse messages are silenced: > > > > > > sound/usb/format.c:377:44: warning: incorrect type in assignment (different base types) > > > sound/usb/format.c:377:44: expected int [signed] pcm_format > > > sound/usb/format.c:377:44: got restricted snd_pcm_format_t [usertype] > > > sound/usb/format.c:379:44: warning: incorrect type in assignment (different base types) > > > sound/usb/format.c:379:44: expected int [signed] pcm_format > > > sound/usb/format.c:379:44: got restricted snd_pcm_format_t [usertype] > > > sound/usb/format.c:382:36: warning: incorrect type in assignment (different base types) > > > sound/usb/format.c:382:36: expected int [signed] pcm_format > > > sound/usb/format.c:382:36: got restricted snd_pcm_format_t [usertype] > > > > > > Signed-off-by: Eldad Zack > > > --- > > > sound/usb/format.c | 10 +++++----- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/sound/usb/format.c b/sound/usb/format.c > > > index 020ede0..e025e28 100644 > > > --- a/sound/usb/format.c > > > +++ b/sound/usb/format.c > > > @@ -365,7 +365,7 @@ static int parse_audio_format_i(struct snd_usb_audio *chip, > > > { > > > struct usb_interface_descriptor *altsd = get_iface_desc(iface); > > > int protocol = altsd->bInterfaceProtocol; > > > - int pcm_format, ret; > > > + int pcm_formats, ret; > > > > > > if (fmt->bFormatType == UAC_FORMAT_TYPE_III) { > > > /* FIXME: the format type is really IECxxx > > > @@ -377,14 +377,14 @@ static int parse_audio_format_i(struct snd_usb_audio *chip, > > > case USB_ID(0x0763, 0x2003): /* M-Audio Audiophile USB */ > > > if (chip->setup == 0x00 && > > > fp->altsetting == 6) > > > - pcm_format = SNDRV_PCM_FORMAT_S16_BE; > > > + pcm_formats = SNDRV_PCM_FMTBIT_S16_BE; > > > else > > > - pcm_format = SNDRV_PCM_FORMAT_S16_LE; > > > + pcm_formats = SNDRV_PCM_FMTBIT_S16_LE; > > > break; > > > default: > > > - pcm_format = SNDRV_PCM_FORMAT_S16_LE; > > > + pcm_formats = SNDRV_PCM_FMTBIT_S16_LE; > > > } > > > - fp->formats = 1uLL << pcm_format; > > > + fp->formats = pcm_formats; > > > } else { > > > fp->formats = parse_audio_format_i_type(chip, fp, format, > > > fmt, protocol); > > > -- > > > 1.8.1.5 > > > > > >