All of
 help / color / mirror / Atom feed
From: Christoph Hellwig <>
To: Aniket Malatpure <>
Subject: Re: [PATCH] (2.4.22-pre6 BK) new & changed (IDE) driver: SGI IOC4]
Date: Sat, 19 Jul 2003 09:45:40 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>; from on Fri, Jul 18, 2003 at 04:10:31PM -0700

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
>  ( 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.

+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.


	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);

	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?

      parent reply	other threads:[~2003-07-19  8:31 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <>
2003-07-18 23:10 ` [PATCH] (2.4.22-pre6 BK) new & changed (IDE) driver: SGI IOC4] Aniket Malatpure
2003-07-18 23:32   ` Alan Cox
2003-07-19  8:45   ` Christoph Hellwig [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.