[alsa-devel] hdsp driver broken for Multiface II Rev 35

[Takashi Iwai]

At Mon, 14 Jan 2008 02:21:42 +0100,
Andreas Degert wrote:
> 
> Hi folks,
> 
> changeset 5326 from 2007/08/31 broke the driver for this card revision.
> There was an older thread about this problem which didn't seem to come
> to a conclusion.
> 
> ioctl SNDRV_HDSP_IOCTL_GET_VERSION returns
> 
> io_type = 1 (Multiface)
> firmware_rev = 0x35
> 
> which is correct. The old code for hdsp_playback_to_output_key looks
> like this:
> 
> static int hdsp_playback_to_output_key (struct hdsp *hdsp, int in, int out)
> {
>         switch (hdsp->firmware_rev) {
>         case 0xa:
>                 return (64 * out) + (32 + (in));
>         case 0x96:
>         case 0x97:
>                 return (32 * out) + (16 + (in));
>         default:
>                 return (52 * out) + (26 + (in));
>         }
> }
> 
> For my card the result is "(52 * out) + (26 + (in))". The new code
> looks like this:
> 
> static int hdsp_playback_to_output_key (struct hdsp *hdsp, int in, int out)
> {
>         switch (hdsp->io_type) {
>         case Multiface:
>         case Digiface:
>         default:
>                 return (64 * out) + (32 + (in));
>         case H9632:
>                 return (32 * out) + (16 + (in));
>         case H9652:
>                 return (52 * out) + (26 + (in));
>         }
> }
> 
> which gives the wrong result "(64 * out) + (32 + (in))". The old code
> returns this when firmware_rev == 0xa, which might be the revision of
> the older Multiface (not Multiface II). I changed the code to
> 
> static int hdsp_playback_to_output_key (struct hdsp *hdsp, int in, int out)
> {
>         switch (hdsp->io_type) {
>         case Multiface:
>         case Digiface:
>         default:
>                 if (hdsp->firmware_rev == 0xa)
>                         return (64 * out) + (32 + (in));
>                 else
>                         return (52 * out) + (26 + (in));
>         case H9632:
>                 return (32 * out) + (16 + (in));
>         case H9652:
>                 return (52 * out) + (26 + (in));
>         }
> }

This looks OK.  Could you prepare a patch (with a proper changelog and
your sing-off)?


> Another possibility would be to go back to the old version of the code
> and only make changes for io_type H9632/H9652 with new firmware
> revisions:

It's a wrong approach in general when the driver supports multiple
hardwares.  The firmware is basically strongly bound with the hardware
model, so checking only the revision is dangerous as well.


Thanks,

Takashi