At Mon, 23 Mar 2009 11:12:22 +0100, Tim Blechmann wrote:
prototype of a driver for the digigram lx6464es 64 channel ethersound interface.
Signed-off-by: Tim Blechmann tim@klingt.org
Thanks.
First off, the current code looks a bit different from the standard kernel coding style. Please run $LINUX/scripts/checkpatch.pl and fix the issues reported there.
diff --git a/sound/pci/lx6464es/LXES_registers.h b/sound/pci/lx6464es/LXES_registers.h new file mode 100644 index 0000000..f665084 --- /dev/null +++ b/sound/pci/lx6464es/LXES_registers.h @@ -0,0 +1,122 @@ +/**
The comments with "/**" in the kernel codes are used for kernel-doc in general. Unless you want to process it via kernel-doc, avoid it.
+* Copyright (C) 2006 DIGIGRAM S.A.
Hrm, is it the copy from Digigram's source? Then we'd need their sign-off as well...
+//********************************************************************************* +// $history:$ +//
Better to remove unneeded parts like this.
diff --git a/sound/pci/lx6464es/Makefile b/sound/pci/lx6464es/Makefile new file mode 100644 index 0000000..a23257c --- /dev/null +++ b/sound/pci/lx6464es/Makefile @@ -0,0 +1,13 @@ +ifeq ($(KERNELRELEASE),) +snd-lx6464es-objs := lx6464es.o lx_core.o +obj-m += snd-lx6464es.o +all:
- make -C /lib/modules/$(shell uname -r)/build M=$(PWD) modules
+clean:
- make -C /lib/modules/$(shell uname -r)/build M=$(PWD) clean
+else +snd-lx6464es-objs := lx6464es.o lx_core.o +obj-$(CONFIG_SND_LX6464ES) += snd-lx6464es.o +endif
Don't put it the stuff for custom builds.
diff --git a/sound/pci/lx6464es/PcxErr_e.h b/sound/pci/lx6464es/PcxErr_e.h new file mode 100644 index 0000000..dd48c46 --- /dev/null +++ b/sound/pci/lx6464es/PcxErr_e.h +// Return value when OK +// ********************
+#define SUCCESS 0
Isn't it defined anywhere? If it's an internal value, I'd suggest to put a prefix to avoid confusion. No big issue, though.
diff --git a/sound/pci/lx6464es/lx6464es.c b/sound/pci/lx6464es/lx6464es.c new file mode 100644 index 0000000..e1df693 --- /dev/null +++ b/sound/pci/lx6464es/lx6464es.c
...
+#ifndef PCI_DEVICE_ID_PLX_9056 /* LATER: this should go to pci_ids.h */ +#define PCI_DEVICE_ID_PLX_9056 0x9056 +#endif
You patch already includes the change in pci_ids.h :) Remove this.
+#define PCI_DEVICE_ID_PLX_LX6464ES PCI_DEVICE_ID_PLX_9056
+#ifndef PCI_VENDOR_ID_DIGIGRAM +#define PCI_VENDOR_ID_DIGIGRAM 0x1369 +#endif
+#ifndef PCI_SUBDEVICE_ID_DIGIGRAM_LX6464ES_SERIAL_SUBSYSTEM +#define PCI_SUBDEVICE_ID_DIGIGRAM_LX6464ES_SERIAL_SUBSYSTEM 0xc001 +#define PCI_SUBDEVICE_ID_DIGIGRAM_LX6464ES_CAE_SERIAL_SUBSYSTEM 0xc002 +#endif
Ditto.
+static int lx_proc_create(struct snd_card *card, struct lx6464es *chip)
Can be __devinit.
+static int __devinit snd_lx6464es_create(struct snd_card *card,
struct pci_dev *pci,struct lx6464es **rchip)
...
- chip->port_mem = pci_resource_start(pci, 0);
- /* plx port */
- chip->port_plx = pci_resource_start(pci, 1);
- chip->port_plx_remapped = ioport_map(chip->port_plx,
pci_resource_len(pci, 1));
Better to use pci_ioremap_bar().
- /* dsp port */
- chip->port_dsp = pci_resource_start(pci, 2);
- chip->port_dsp_resource = request_mem_region(chip->port_dsp,
pci_resource_len(pci, 2),card_name);
Hm, isn't it already requested via pci_request_regions()?
- chip->port_dsp_remapped = ioremap_nocache(chip->port_dsp,
pci_resource_len(pci, 2));
Use pci_ioremap_bar().
- err = request_irq(pci->irq, lx_interrupt, IRQF_SHARED,
card_name, chip);- if (err) {
snd_printk(KERN_ERR LXP "unable to grab IRQ %d\n", pci->irq);goto request_irq_failed;- }
- chip->irq = pci->irq;
- err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, chip, &ops);
- if (err < 0)
return err;
This error path can be a leak.
diff --git a/sound/pci/lx6464es/lx_core.h b/sound/pci/lx6464es/lx_core.h new file mode 100644 index 0000000..901d443 --- /dev/null +++ b/sound/pci/lx6464es/lx_core.h +static inline void unpack_pointer(dma_addr_t ptr, u32 *r_low, u32 *r_high) +{
- *r_low = (u32)(ptr & 0xffffffff);
+#if BITS_PER_LONG == 32
- *r_high = 0;
+#else
- *r_high = (u32)((u64)ptr>>32);
+#endif
Better to use upper_32_bits() macro.
thanks,
Takashi