Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 02 Jul 2019 16:37:40 +0200
Message-ID: <s5hlfxg4i4r.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     "Nicola Lunghi" <nick83ola@gmail.com>
Cc:     <alsa-devel@alsa-project.org>, <info@jensverwiebe.de>,
        "Thomas Gleixner" <tglx@linutronix.de>,
        "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
        "Allison Randal" <allison@lohutok.net>,
        "Jaroslav Kysela" <perex@perex.cz>,
        "Richard Fontana" <rfontana@redhat.com>,
        "Jussi Laako" <jussi@sonarnerd.net>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ALSA: usb-audio: fix Line6 Helix audio format rates
In-Reply-To: <20190702004439.30131-1-nick83ola@gmail.com>
References: <20190702004439.30131-1-nick83ola@gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka)
 FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3
 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka")
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 02 Jul 2019 02:43:14 +0200,
Nicola Lunghi wrote:
> 
> Line6 Helix and HX stomp don't support retrieving
> the number of clock sample rate.
> 
> Add a quirk to return the default value of 48Khz.
> 
> Signed-off-by: Nicola Lunghi <nick83ola@gmail.com>

It's not particularly good place to put a quirk, but there seems no
other better place, unfortunately.  Is this specific to certain unit
or all I/Os on the device suffer from this problem?

In anyway, if the behavior is expected, we don't need to use
dev_warn() to annoy users unnecessarily.  Replace it with dev_info().

Also, the code that creates a single 48k entry would be better to be
put into a function for readability.

Could you resubmit with that change?


Thanks!

Takashi

> ---
>  sound/usb/format.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/usb/format.c b/sound/usb/format.c
> index c02b51a82775..05442f6ada62 100644
> --- a/sound/usb/format.c
> +++ b/sound/usb/format.c
> @@ -313,10 +313,32 @@ static int parse_audio_format_rates_v2v3(struct snd_usb_audio *chip,
>  			      tmp, sizeof(tmp));
>  
>  	if (ret < 0) {
> -		dev_err(&dev->dev,
> -			"%s(): unable to retrieve number of sample rates (clock %d)\n",
> +		switch (chip->usb_id) {
> +		/* LINE 6 HX pedals don't support getting the clock sample rate.
> +		 * Set the framerate to 48khz by default
> +		 */
> +		case USB_ID(0x0E41, 0x4244): /* HELIX */
> +		case USB_ID(0x0E41, 0x4246): /* HX STOMP */
> +			dev_warn(&dev->dev,
> +				"%s(): line6 helix: unable to retrieve number of sample rates. Set it to default value (clock %d).\n",
>  				__func__, clock);
> -		goto err;
> +			fp->nr_rates = 1;
> +			fp->rate_min = 48000;
> +			fp->rate_max = 48000;
> +			fp->rates = SNDRV_PCM_RATE_48000;
> +			fp->rate_table = kmalloc(sizeof(int), GFP_KERNEL);
> +			if (!fp->rate_table) {
> +				ret = -ENOMEM;
> +				goto err_free;
> +			}
> +			fp->rate_table[0] = 48000;
> +			return 0;
> +		default:
> +			dev_err(&dev->dev,
> +				"%s(): unable to retrieve number of sample rates (clock %d)\n",
> +					__func__, clock);
> +			goto err;
> +		}
>  	}
>  
>  	nr_triplets = (tmp[1] << 8) | tmp[0];
> -- 
> 2.19.1
> 
>