[alsa-devel] [PATCH] Gallant SC-6000 driver

[Clemens Ladisch]

Krzysztof Helt wrote:
> This is port of the Gallant SC-6000 driver from the OSS aedsp16 driver.

Several minor nitpicks below.  Otherwise, it looks fine.

> +       tristate "Gallant SC-6000, Audio Excel DSP 16"
> +       help
> +         Say Y here to include support for Gallant SC-6000 card and
> +         compatible soundcards: Audio Excel DSP 16 and Zoltrix AV302.

It may be a good idea to mention the chip name here (CompuMedia ASC-9308
if Google is to be believed).

> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA

The indentation of the last paragraph is inconsistent.  :-)

> +#define COMMAND_88     0x88    /*                                      */

This command is used.  Could you document it at least a little, "unknown
setup magic" or something like that?

> +/*
> + * sc6000_irq_to_softcfg - Decode irq number into cfg code.
> + */
> +static inline unsigned char sc6000_irq_to_softcfg(int irq)
> +{
> +       unsigned char val = 0;
> +
> +       switch (irq) {
> +       ...
> +       default:
> +               break;

I didn't find any code that checks that the user-supplied irq/port/dma
values are corret.  What happens when the driver tries to configure the
board with invalid values?

> +       if (strcmp("SC-6000", answer))
> +               snd_printk(KERN_ERR PFX "Warning: non SC-6000 audio card!\n");

KERN_ERR for a warning?

> +       snd_printk(KERN_DEBUG PFX "Initializing BASE[0x%lx] IRQ[%d] "

For debug output, please use snd_printd() so that the call isn't
compiled in in non-debug builds.

> +       strcpy(card->driver, "Gallant SC-6000");

If the SC-6600 could be supported by this driver and if there is the
same number of PCM devices (so that alsa-lib can use the same
dmix configuration for both), the driver name should be something
generic like "SC-6x00" or something like that.


Regards,
Clemens