Re: [PATCH 2/2] sound: usb: format: don't warn that raw DSD is unsupported

[Takashi Iwai <tiwai@xxxxxxx>]

On Wed, 04 Dec 2024 16:19:54 +0100,
Adrian Ratiu wrote:
> 
> UAC 2 & 3 DAC's set bit 31 of the format to signal support for a
> RAW_DATA type, typically used for DSD playback.
> 
> This is correctly tested by (format & UAC*_FORMAT_TYPE_I_RAW_DATA),
> fp->dsd_raw = true; and call snd_usb_interface_dsd_format_quirks(),
> however a confusing and unnecessary message gets printed because
> the bit is not properly tested in the last "unsupported" if test.
> 
> For example:
> 
> usb 7-1: new high-speed USB device number 5 using xhci_hcd
> usb 7-1: New USB device found, idVendor=262a, idProduct=9302, bcdDevice=0.01
> usb 7-1: New USB device strings: Mfr=1, Product=2, SerialNumber=6
> usb 7-1: Product: TC44C
> usb 7-1: Manufacturer: TC44C
> usb 7-1: SerialNumber: 5000000001
> hid-generic 0003:262A:9302.001E: No inputs registered, leaving
> hid-generic 0003:262A:9302.001E: hidraw6: USB HID v1.00 Device [DDHIFI TC44C] on usb-0000:08:00.3-1/input0
> usb 7-1: 2:4 : unsupported format bits 0x100000000
> 
> This last "unsupported format" is actually wrong: we know the
> format is a RAW_DATA which we assume is DSD, so there is no need
> to print the confusing message.
> 
> Thus we update the condition to take into account bit 31 for DSD
> (notice the "format <<= 1;" line above).
> 
> Signed-off-by: Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx>

IMO, it's better to clear the already checked format bit instead, as
there are two distinct bits depending on the protocol.
That is, something like:

--- a/sound/usb/format.c
+++ b/sound/usb/format.c
@@ -54,6 +54,7 @@ static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
 
 		if (format & UAC2_FORMAT_TYPE_I_RAW_DATA) {
 			pcm_formats |= SNDRV_PCM_FMTBIT_SPECIAL;
+			format &= ~UAC2_FORMAT_TYPE_I_RAW_DATA;
 			/* flag potentially raw DSD capable altsettings */
 			fp->dsd_raw = true;
 		}
@@ -67,8 +68,10 @@ static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
 		sample_width = as->bBitResolution;
 		sample_bytes = as->bSubslotSize;
 
-		if (format & UAC3_FORMAT_TYPE_I_RAW_DATA)
+		if (format & UAC3_FORMAT_TYPE_I_RAW_DATA) {
 			pcm_formats |= SNDRV_PCM_FMTBIT_SPECIAL;
+			format &= ~UAC3_FORMAT_TYPE_I_RAW_DATA;
+		}
 
 		format <<= 1;
 		break;


thanks,

Takashi


> ---
>  sound/usb/format.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/format.c b/sound/usb/format.c
> index 0cbf1d4fbe6e..fe2e0580e296 100644
> --- a/sound/usb/format.c
> +++ b/sound/usb/format.c
> @@ -142,7 +142,7 @@ static u64 parse_audio_format_i_type(struct snd_usb_audio *chip,
>  		pcm_formats |= SNDRV_PCM_FMTBIT_A_LAW;
>  	if (format & BIT(UAC_FORMAT_TYPE_I_MULAW))
>  		pcm_formats |= SNDRV_PCM_FMTBIT_MU_LAW;
> -	if (format & ~0x3f) {
> +	if (format & ~0x10000003F) {
>  		usb_audio_info(chip,
>  			 "%u:%d : unsupported format bits %#llx\n",
>  			 fp->iface, fp->altsetting, format);
> -- 
> 2.45.2
>