[alsa-devel] [PATCH - [hdspm] 2/3] Enable RME RayDAT and AIO

[Takashi Iwai]

At Tue, 18 Jan 2011 15:12:18 +0100,
Adrian Knoth wrote:
> 
> @@ -29,12 +42,13 @@
>  #include <linux/moduleparam.h>
>  #include <linux/slab.h>
>  #include <linux/pci.h>
> -#include <linux/math64.h>

No math64 stuff is necessary?

> +#include <linux/sysfs.h>

It's not implemented yet, so don't include it.

> -static DEFINE_PCI_DEVICE_TABLE(snd_hdspm_ids) = {
> +static struct pci_device_id snd_hdspm_ids[] __devinitdata = {

Why open-coded again?


> @@ -796,6 +1009,8 @@ static void hdspm_silence_playback(struct hdspm *hdspm)
>  	int n = hdspm->period_bytes;
>  	void *buf = hdspm->playback_buffer;
>  
> +	snd_printk(KERN_INFO "hdspm_silence_playback\n");

Don't put such a verbose debug message in frequently called callbacks.

>  static void hdspm_set_dds_value(struct hdspm *hdspm, int rate)
...
>  	n = div_u64(n, rate);
>  	/* n should be less than 2^32 for being written to FREQ register */
> -	snd_BUG_ON(n >> 32);
> +	snd_BUG_ON((n >> 32) == 0);

This check is consistent with the comment above...

The rest is too long to review.  Would be nice if you can split the
whole patch in logical orders...


thanks,

Takashi