All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i
@ 2013-04-21 23:44 Eldad Zack
  2013-04-22  8:51 ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Eldad Zack @ 2013-04-21 23:44 UTC (permalink / raw)
  To: Takashi Iwai, Jaroslav Kysela, Daniel Mack, Clemens Ladisch
  Cc: alsa-devel, Eldad Zack

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.
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] <noident>
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] <noident>
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] <noident>

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i
  2013-04-21 23:44 [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i Eldad Zack
@ 2013-04-22  8:51 ` Takashi Iwai
  2013-04-22 11:29   ` Eldad Zack
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2013-04-22  8:51 UTC (permalink / raw)
  To: Eldad Zack; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch

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


thanks,

Takashi

> 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] <noident>
> 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] <noident>
> 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] <noident>
> 
> Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i
  2013-04-22  8:51 ` Takashi Iwai
@ 2013-04-22 11:29   ` Eldad Zack
  2013-04-22 11:34     ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Eldad Zack @ 2013-04-22 11:29 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch



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?

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] <noident>
> > 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] <noident>
> > 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] <noident>
> > 
> > Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> > ---
> >  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
> > 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i
  2013-04-22 11:29   ` Eldad Zack
@ 2013-04-22 11:34     ` Takashi Iwai
  2013-04-22 11:47       ` Eldad Zack
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2013-04-22 11:34 UTC (permalink / raw)
  To: Eldad Zack; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch

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] <noident>
> > > 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] <noident>
> > > 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] <noident>
> > > 
> > > Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> > > ---
> > >  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
> > > 
> > 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i
  2013-04-22 11:34     ` Takashi Iwai
@ 2013-04-22 11:47       ` Eldad Zack
  2013-04-22 11:49         ` Takashi Iwai
  0 siblings, 1 reply; 8+ messages in thread
From: Eldad Zack @ 2013-04-22 11:47 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch



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).

Cheers,
Eldad

> 
> 
> 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] <noident>
> > > > 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] <noident>
> > > > 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] <noident>
> > > > 
> > > > Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
> > > > ---
> > > >  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
> > > > 
> > > 
> > 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i
  2013-04-22 11:47       ` Eldad Zack
@ 2013-04-22 11:49         ` Takashi Iwai
  2013-04-22 23:00           ` [PATCH, for-next 1/2] ALSA: pcm_format_to_bits strong-typed conversion Eldad Zack
  0 siblings, 1 reply; 8+ messages in thread
From: Takashi Iwai @ 2013-04-22 11:49 UTC (permalink / raw)
  To: Eldad Zack; +Cc: alsa-devel, Daniel Mack, Clemens Ladisch

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH, for-next 1/2] ALSA: pcm_format_to_bits strong-typed conversion
  2013-04-22 11:49         ` Takashi Iwai
@ 2013-04-22 23:00           ` Eldad Zack
  2013-04-22 23:00             ` [PATCH, for-next 2/2] ALSA: asihpi: add format support check in snd_card_asihpi_capture_formats Eldad Zack
  0 siblings, 1 reply; 8+ messages in thread
From: Eldad Zack @ 2013-04-22 23:00 UTC (permalink / raw)
  To: Takashi Iwai, alsa-devel; +Cc: Eldad Zack, Daniel Mack, Clemens Ladisch

Add a function to handle conversion from snd_pcm_format_t
to bitwise with proper typing.

Change such conversions to use this function and silence sparse
warnings.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 include/sound/pcm.h             | 6 ++++++
 sound/aoa/soundbus/i2sbus/pcm.c | 2 +-
 sound/atmel/ac97c.c             | 4 ++--
 sound/drivers/aloop.c           | 2 +-
 sound/pci/asihpi/asihpi.c       | 4 ++--
 sound/usb/format.c              | 5 +++--
 sound/usb/pcm.c                 | 4 ++--
 sound/usb/proc.c                | 2 +-
 8 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 1b0c648..5357ecb 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -1133,4 +1133,10 @@ int snd_pcm_add_chmap_ctls(struct snd_pcm *pcm, int stream,
 			   unsigned long private_value,
 			   struct snd_pcm_chmap **info_ret);
 
+/* Strong-typed conversion of pcm_format to bitwise */
+static inline u64 pcm_format_to_bits(snd_pcm_format_t pcm_format)
+{
+	return 1ULL << (__force int) pcm_format;
+}
+
 #endif /* __SOUND_PCM_H */
diff --git a/sound/aoa/soundbus/i2sbus/pcm.c b/sound/aoa/soundbus/i2sbus/pcm.c
index 19491ed..7b74a4b 100644
--- a/sound/aoa/soundbus/i2sbus/pcm.c
+++ b/sound/aoa/soundbus/i2sbus/pcm.c
@@ -179,7 +179,7 @@ static int i2sbus_pcm_open(struct i2sbus_dev *i2sdev, int in)
 	 */
 	if (other->active) {
 		/* FIXME: is this guaranteed by the alsa api? */
-		hw->formats &= (1ULL << i2sdev->format);
+		hw->formats &= pcm_format_to_bits(i2sdev->format);
 		/* see above, restrict rates to the one we already have */
 		hw->rate_min = i2sdev->rate;
 		hw->rate_max = i2sdev->rate;
diff --git a/sound/atmel/ac97c.c b/sound/atmel/ac97c.c
index 79d6bda..6b7e2b5 100644
--- a/sound/atmel/ac97c.c
+++ b/sound/atmel/ac97c.c
@@ -182,7 +182,7 @@ static int atmel_ac97c_playback_open(struct snd_pcm_substream *substream)
 		runtime->hw.rate_max = chip->cur_rate;
 	}
 	if (chip->cur_format)
-		runtime->hw.formats = (1ULL << chip->cur_format);
+		runtime->hw.formats = pcm_format_to_bits(chip->cur_format);
 	mutex_unlock(&opened_mutex);
 	chip->playback_substream = substream;
 	return 0;
@@ -201,7 +201,7 @@ static int atmel_ac97c_capture_open(struct snd_pcm_substream *substream)
 		runtime->hw.rate_max = chip->cur_rate;
 	}
 	if (chip->cur_format)
-		runtime->hw.formats = (1ULL << chip->cur_format);
+		runtime->hw.formats = pcm_format_to_bits(chip->cur_format);
 	mutex_unlock(&opened_mutex);
 	chip->capture_substream = substream;
 	return 0;
diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index 64d5347..6f78de9 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -325,7 +325,7 @@ static void params_change(struct snd_pcm_substream *substream)
 	struct loopback_pcm *dpcm = runtime->private_data;
 	struct loopback_cable *cable = dpcm->cable;
 
-	cable->hw.formats = (1ULL << runtime->format);
+	cable->hw.formats = pcm_format_to_bits(runtime->format);
 	cable->hw.rate_min = runtime->rate;
 	cable->hw.rate_max = runtime->rate;
 	cable->hw.channels_min = runtime->channels;
diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c
index 0aabfed..160cf83 100644
--- a/sound/pci/asihpi/asihpi.c
+++ b/sound/pci/asihpi/asihpi.c
@@ -966,7 +966,7 @@ static u64 snd_card_asihpi_playback_formats(struct snd_card_asihpi *asihpi,
 		if (!err)
 			err = hpi_outstream_query_format(h_stream, &hpi_format);
 		if (!err && (hpi_to_alsa_formats[format] != -1))
-			formats |= (1ULL << hpi_to_alsa_formats[format]);
+			formats |= pcm_format_to_bits(hpi_to_alsa_formats[format]);
 	}
 	return formats;
 }
@@ -1142,7 +1142,7 @@ static u64 snd_card_asihpi_capture_formats(struct snd_card_asihpi *asihpi,
 		if (!err)
 			err = hpi_instream_query_format(h_stream, &hpi_format);
 		if (!err)
-			formats |= (1ULL << hpi_to_alsa_formats[format]);
+			formats |= pcm_format_to_bits(hpi_to_alsa_formats[format]);
 	}
 	return formats;
 }
diff --git a/sound/usb/format.c b/sound/usb/format.c
index 020ede0..99299ff 100644
--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -365,7 +365,8 @@ 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;
+	snd_pcm_format_t pcm_format;
+	int ret;
 
 	if (fmt->bFormatType == UAC_FORMAT_TYPE_III) {
 		/* FIXME: the format type is really IECxxx
@@ -384,7 +385,7 @@ static int parse_audio_format_i(struct snd_usb_audio *chip,
 		default:
 			pcm_format = SNDRV_PCM_FORMAT_S16_LE;
 		}
-		fp->formats = 1uLL << pcm_format;
+		fp->formats = pcm_format_to_bits(pcm_format);
 	} else {
 		fp->formats = parse_audio_format_i_type(chip, fp, format,
 							fmt, protocol);
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 9723f3c..93b6e32 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -100,7 +100,7 @@ static struct audioformat *find_format(struct snd_usb_substream *subs)
 	int cur_attr = 0, attr;
 
 	list_for_each_entry(fp, &subs->fmt_list, list) {
-		if (!(fp->formats & (1uLL << subs->pcm_format)))
+		if (!(fp->formats & pcm_format_to_bits(subs->pcm_format)))
 			continue;
 		if (fp->channels != subs->channels)
 			continue;
@@ -478,7 +478,7 @@ static int match_endpoint_audioformats(struct audioformat *fp,
 		return 0;
 	}
 
-	if (!(fp->formats & (1ULL << pcm_format))) {
+	if (!(fp->formats & pcm_format_to_bits(pcm_format))) {
 		snd_printdd("%s: (fmt @%p) no match for format %d\n", __func__,
 			fp, pcm_format);
 		return 0;
diff --git a/sound/usb/proc.c b/sound/usb/proc.c
index 0182ef6..135c768 100644
--- a/sound/usb/proc.c
+++ b/sound/usb/proc.c
@@ -85,7 +85,7 @@ static void proc_dump_substream_formats(struct snd_usb_substream *subs, struct s
 		snd_iprintf(buffer, "    Altset %d\n", fp->altsetting);
 		snd_iprintf(buffer, "    Format:");
 		for (fmt = 0; fmt <= SNDRV_PCM_FORMAT_LAST; ++fmt)
-			if (fp->formats & (1uLL << fmt))
+			if (fp->formats & pcm_format_to_bits(fmt))
 				snd_iprintf(buffer, " %s",
 					    snd_pcm_format_name(fmt));
 		snd_iprintf(buffer, "\n");
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH, for-next 2/2] ALSA: asihpi: add format support check in snd_card_asihpi_capture_formats
  2013-04-22 23:00           ` [PATCH, for-next 1/2] ALSA: pcm_format_to_bits strong-typed conversion Eldad Zack
@ 2013-04-22 23:00             ` Eldad Zack
  0 siblings, 0 replies; 8+ messages in thread
From: Eldad Zack @ 2013-04-22 23:00 UTC (permalink / raw)
  To: Takashi Iwai, alsa-devel; +Cc: Eldad Zack, Daniel Mack, Clemens Ladisch

Some Asihpi formats are not supported or invalid, and their mapping to
ALSA format is set to -1.
Before performing the format conversion into ALSA bitwise formats,
add a consistency check for the requested format, as done in
snd_card_asihpi_playback_formats().

Compile tested only.

Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
 sound/pci/asihpi/asihpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/pci/asihpi/asihpi.c b/sound/pci/asihpi/asihpi.c
index 160cf83..fbc1720 100644
--- a/sound/pci/asihpi/asihpi.c
+++ b/sound/pci/asihpi/asihpi.c
@@ -1141,7 +1141,7 @@ static u64 snd_card_asihpi_capture_formats(struct snd_card_asihpi *asihpi,
 					format, sample_rate, 128000, 0);
 		if (!err)
 			err = hpi_instream_query_format(h_stream, &hpi_format);
-		if (!err)
+		if (!err && (hpi_to_alsa_formats[format] != -1))
 			formats |= pcm_format_to_bits(hpi_to_alsa_formats[format]);
 	}
 	return formats;
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-04-22 23:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-21 23:44 [PATCH, for-next] ALSA: usb-audio: use FMTBITs in parse_audio_format_i Eldad Zack
2013-04-22  8:51 ` Takashi Iwai
2013-04-22 11:29   ` Eldad Zack
2013-04-22 11:34     ` Takashi Iwai
2013-04-22 11:47       ` Eldad Zack
2013-04-22 11:49         ` Takashi Iwai
2013-04-22 23:00           ` [PATCH, for-next 1/2] ALSA: pcm_format_to_bits strong-typed conversion Eldad Zack
2013-04-22 23:00             ` [PATCH, for-next 2/2] ALSA: asihpi: add format support check in snd_card_asihpi_capture_formats Eldad Zack

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.