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 <[email protected]>
---
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
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 <[email protected]>
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
>
>
On 7/2/2019 4:37 PM, Takashi Iwai wrote:
> 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 <[email protected]>
> 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
If the listed USB devices do not support sample rate format retrieval
then maybe it would be a better idea to perform below check before
sending message?
Have you also considered new function or macro that check device
support? This would separate formatfunctionality code from routine
that identifies applicable devices- in case if in future more devices
will require quirk.
Michal W.
>> ---
>> 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
>>
>>
> _______________________________________________
> Alsa-devel mailing list
> [email protected]
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
On Tue, 02 Jul 2019 17:52:01 +0200,
Wasko, Michal wrote:
>
> On 7/2/2019 4:37 PM, Takashi Iwai wrote:
> > 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 <[email protected]>
> > 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
>
> If the listed USB devices do not support sample rate format retrieval
> then maybe it would be a better idea to perform below check before
> sending message?
Yes, if we know that it always fails, we don't need to query.
> Have you also considered new function or macro that check device
> support? This would separate formatfunctionality code from routine
> that identifies applicable devices- in case if in future more devices
> will require quirk.
The split can be done later. It's always hard to know what kind of
quirk would be needed in future. If any more devices show the same
problem, we can reorganize the quirk in a saner way.
thanks,
Takashi
>
> Michal W.
>
> >> ---
> >> 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
> >>
> >>
> > _______________________________________________
> > Alsa-devel mailing list
> > [email protected]
> > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
>