Messages in this thread | | | Date | Sat, 19 Jul 2003 09:45:40 +0100 | From | Christoph Hellwig <> | Subject | Re: [PATCH] (2.4.22-pre6 BK) new & changed (IDE) driver: SGI IOC4] |
| |
On Fri, Jul 18, 2003 at 04:10:31PM -0700, Aniket Malatpure wrote: > Hi > > This patch (against the 2.4 BK tree as of yesterday evening) adds > support for SGI IOC4 IDE driver as found in SN2/Altix systems > (http://sgi.com/altix for more details). > > No generic/non-sn2 code paths should be affected by this.
A few comments:
+ dep_tristate ' SGI IOC4 chipset support' CONFIG_BLK_DEV_SGIIOC4 $CONFIG_BLK_DEV_IDEDMA_PCI
This probably wants a depency on CONFIG_IA64_SGI_SN2?
+#include <linux/config.h> +#include <linux/module.h> +#include <linux/types.h> +#include <linux/pci.h> +#include <linux/delay.h> +#include <linux/hdreg.h> +#include <linux/init.h> +#include <asm/io.h>
<asm/*> includes after <linux/*> ones please.
+#ifdef CONFIG_PROC_FS +static u8 sgiioc4_proc = 0; +#endif /* CONFIG_PROC_FS */
As all procfs code is properly stubbed out you can probably nuke the ifdef.
+ +static int n_sgiioc4_devs = 0;
Also both of these don't need zero initialization, Linux takes care of that for you.
+#define SGIIOC4_HD_SUPPORT 0
Please make this either a config ã¨ption or nuke it.
+ +static inline void +xide_delay(long ticks)
Documentation/CodingStyle says
static inline void xide_delay(long ticks)
The same is true for many other function in this file.
+ printk(KERN_INFO "%s: %s Bus-Master DMA disabled \n", hwif->name, name);
Please linewrap after 80 chars.
+static unsigned int __init +pci_init_sgiioc4(struct pci_dev *dev, const char *name) +{ + extern pciio_endian_t snia_pciio_endian_set(struct pci_dev *pci_dev, + pciio_endian_t device_end, + pciio_endian_t desired_end);
Shouldn't this be in a header?
+#if defined(CONFIG_IA64_SGI_SN2) || defined(CONFIG_IA64_GENERIC) + /* Enable Byte Swapping in the PIC */ + snia_pciio_endian_set(dev, PCIDMA_ENDIAN_LITTLE, PCIDMA_ENDIAN_BIG); +#endif
Currentl 2.4 mainline doesn't seem to include SN2 in IA64_GENERIC so compilation will fail for this case.
+static void __init +ide_init_sgiioc4(ide_hwif_t * hwif) +{ + hwif->autodma = 1; + hwif->index = 0; /* Channel 0 */ + hwif->channel = 0; + hwif->atapi_dma = 1; + hwif->ultra_mask = 0x0; /* Disable Ultra DMA */
Use a memset here so you only have to initialize the non-NULL fields?
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |