Re: [PATCH] (2.4.22-pre6 BK) new & changed (IDE) driver: SGI IOC4]

From: Christoph Hellwig (hch@infradead.org)
Date: Sat Jul 19 2003 - 03:45:40 EST


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/



This archive was generated by hypermail 2b29 : Wed Jul 23 2003 - 22:00:37 EST