Skip to contentSkip to conversation

allow SGI IOC4 chipset support

PATCH In review Jesse Barnes 3 participants 10 messages Jan 6, 2004 – Jan 13, 2004

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 allow SGI IOC4 chipset support

The 'depends' directive for SGI IOC4 support is too restrictive. Just kill it altogether.

Jesse

===== drivers/ide/Kconfig 1.33 vs edited =====

drivers/ide/Kconfig +0 -1
@@ -747,7 +747,6 @@
 
 config BLK_DEV_SGIIOC4
 	tristate "Silicon Graphics IOC4 chipset support"
-	depends on IA64_SGI_SN2
 	help
 	  This driver adds PIO & MultiMode DMA-2 support for the SGI IOC4
 	  chipset, which has one channel and can support two devices.
#1 lore raw
Quoted 2 lines · Jesse Barnes

The 'depends' directive for SGI IOC4 support is too restrictive. Just kill it altogether.

Umm, it won't work for anything but a kernel with SN2 support compile in due to the bridge-level dma byteswapping it needs (through a week symbol, that's why you don't see compile failures for other architectures, eek!).

So at least make it depend on CONFIG_IA64

#2 lore raw

Re: [PATCH] allow SGI IOC4 chipset support in ia64 generic kernels

Quoted 7 lines · Christoph Hellwig
Quoted 2 lines · Jesse Barnes

The 'depends' directive for SGI IOC4 support is too restrictive. Just kill it altogether.

Umm, it won't work for anything but a kernel with SN2 support compile in due to the bridge-level dma byteswapping it needs (through a week symbol, that's why you don't see compile failures for other architectures, eek!).

Good point. They'll need either CONFIG_IA64_SGI_SN2 or CONFIG_IA64_GENERIC to get the right stuff.

Quoted 1 line

So at least make it depend on CONFIG_IA64

Here's a more correct fix that should prevent people from seeing build failures at all.

Jesse

===== drivers/ide/Kconfig 1.33 vs edited =====

drivers/ide/Kconfig +1 -1
@@ -747,7 +747,7 @@
 
 config BLK_DEV_SGIIOC4
 	tristate "Silicon Graphics IOC4 chipset support"
-	depends on IA64_SGI_SN2
+	depends on IA64_SGI_SN2 || IA64_GENERIC
 	help
 	  This driver adds PIO & MultiMode DMA-2 support for the SGI IOC4
 	  chipset, which has one channel and can support two devices.
#3 lore raw
In reply to Christoph Hellwig
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Christoph" == Christoph Hellwig <[email protected]> writes:

Christoph> On Mon, Jan 05, 2004 at 05:09:24PM -0800, Jesse Barnes

Quoted 2 lines · Christoph> wrote:
Quoted 2 lines

The 'depends' directive for SGI IOC4 support is too restrictive. Just kill it altogether.

Christoph> Umm, it won't work for anything but a kernel with SN2 Christoph> support compile in due to the bridge-level dma byteswapping Christoph> it needs (through a week symbol, that's why you don't see Christoph> compile failures for other architectures, eek!).

Christoph> So at least make it depend on CONFIG_IA64

What about adding this?

Though shall not use weak symbols in though kernel ....

Jes

ide/pci/sgiioc4.c renamed from ide/pci/sgiioc4.c~ +7 -4
@@ -719,6 +719,7 @@
 	return 0;
 }
 
+#if defined(CONFIG_IA64_GENERIC) || defined(CONFIG_IA64_SGI_SN2)
 /* This ensures that we can build this for generic kernels without
  * having all the SN2 code sync'd and merged.
  */
@@ -726,9 +727,10 @@
 	PCIDMA_ENDIAN_BIG,
 	PCIDMA_ENDIAN_LITTLE
 } pciio_endian_t;
-pciio_endian_t __attribute__ ((weak)) snia_pciio_endian_set(struct pci_dev
-					    *pci_dev, pciio_endian_t device_end,
-					    pciio_endian_t desired_end);
+pciio_endian_t snia_pciio_endian_set(struct pci_dev
+				     *pci_dev, pciio_endian_t device_end,
+				     pciio_endian_t desired_end);
+#endif
 
 static unsigned int __init
 pci_init_sgiioc4(struct pci_dev *dev, ide_pci_device_t * d)
@@ -754,6 +756,7 @@
 		return 1;
 	}
 
+#if defined(CONFIG_IA64_GENERIC) || defined(CONFIG_IA64_SGI_SN2)
 	/* Enable Byte Swapping in the PIC... */
 	if (snia_pciio_endian_set) {
 		snia_pciio_endian_set(dev, PCIDMA_ENDIAN_LITTLE,
@@ -764,7 +767,7 @@
 		       d->name, dev->slot_name);
 		return 1;
 	}
-
+#endif
 	return sgiioc4_ide_setup_pci_device(dev, d);
 }
 
#4 lore raw
Quoted 3 lines · Jes Sorensen

What about adding this?

Though shall not use weak symbols in though kernel ....

That's stupid. You should just not be allowed to compile the driver if it can work anywork.

#5 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Christoph" == Christoph Hellwig <[email protected]> writes:

Christoph> On Wed, Jan 07, 2004 at 06:18:30AM -0500, Jes Sorensen

Quoted 3 lines · Christoph> wrote:
Quoted 3 lines

What about adding this?

Though shall not use weak symbols in though kernel ....

Christoph> That's stupid. You should just not be allowed to compile Christoph> the driver if it can work anywork.

I don't know if it's actually possible to use the card in a non-SN2, not that I think anyone would want to. But if you prefer, just make the equivalent change to the Kconfig file and remove the weak reference.

Jes

#6 lore raw
Quoted 7 lines · Jes Sorensen

Christoph> That's stupid. You should just not be allowed to compile Christoph> the driver if it can work anywork.

I don't know if it's actually possible to use the card in a non-SN2, not that I think anyone would want to. But if you prefer, just make the equivalent change to the Kconfig file and remove the weak reference.

It might be possible to use an IOC4 card in other hardware, but not with this driver, as it relies on Xbridge/PIC to do byteswapping for it - a functionality common PCI bridges don't provide and that Linux doesn't have a generic API for.

#7 lore raw
In reply to Christoph Hellwig
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Christoph" == Christoph Hellwig <[email protected]> writes:

Christoph> On Mon, Jan 05, 2004 at 05:09:24PM -0800, Jesse Barnes

Quoted 2 lines · Christoph> wrote:
Quoted 2 lines

The 'depends' directive for SGI IOC4 support is too restrictive. Just kill it altogether.

Christoph> Umm, it won't work for anything but a kernel with SN2 Christoph> support compile in due to the bridge-level dma byteswapping Christoph> it needs (through a week symbol, that's why you don't see Christoph> compile failures for other architectures, eek!).

Christoph> So at least make it depend on CONFIG_IA64

Lets try this then, relative to 2.6.1.

Jes

drivers/ide/pci/sgiioc4.c renamed from linux-2.6.1-jb-boot/drivers/ide/pci/sgiioc4.c +2 -10
@@ -726,7 +726,7 @@
 	PCIDMA_ENDIAN_BIG,
 	PCIDMA_ENDIAN_LITTLE
 } pciio_endian_t;
-pciio_endian_t __attribute__ ((weak)) snia_pciio_endian_set(struct pci_dev
+extern pciio_endian_t snia_pciio_endian_set(struct pci_dev
 					    *pci_dev, pciio_endian_t device_end,
 					    pciio_endian_t desired_end);
 
@@ -755,15 +755,7 @@
 	}
 
 	/* Enable Byte Swapping in the PIC... */
-	if (snia_pciio_endian_set) {
-		snia_pciio_endian_set(dev, PCIDMA_ENDIAN_LITTLE,
-				      PCIDMA_ENDIAN_BIG);
-	} else {
-		printk(KERN_ERR
-		       "Failed to set endianness for device %s at slot %s\n",
-		       d->name, dev->slot_name);
-		return 1;
-	}
+	snia_pciio_endian_set(dev, PCIDMA_ENDIAN_LITTLE, PCIDMA_ENDIAN_BIG);
 
 	return sgiioc4_ide_setup_pci_device(dev, d);
 }
drivers/ide/Kconfig renamed from linux-2.6.1-jb-boot/drivers/ide/Kconfig +1 -1
@@ -747,7 +747,7 @@
 
 config BLK_DEV_SGIIOC4
 	tristate "Silicon Graphics IOC4 chipset support"
-	depends on IA64_SGI_SN2
+	depends on IA64_GENERIC || IA64_SGI_SN2
 	help
 	  This driver adds PIO & MultiMode DMA-2 support for the SGI IOC4
 	  chipset, which has one channel and can support two devices.
#8 lore raw

drivers/ide/pci/sgiioc4.c line 758

Quoted 1 line · Jes Sorensen
drivers/ide/pci/sgiioc4.c renamed from linux-2.6.1-jb-boot/drivers/ide/pci/sgiioc4.c +1 -0
+	snia_pciio_endian_set(dev, PCIDMA_ENDIAN_LITTLE, PCIDMA_ENDIAN_BIG);

Shouldn't we check for a failure here?

#9 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Christoph" == Christoph Hellwig <[email protected]> writes:

Christoph> On Mon, Jan 12, 2004 at 10:22:35AM -0500, Jes Sorensen

Quoted 2 lines · Christoph> wrote:
Quoted 2 lines

+ snia_pciio_endian_set(dev, PCIDMA_ENDIAN_LITTLE, PCIDMA_ENDIAN_BIG);

Christoph> Shouldn't we check for a failure here?

We could, but I don't believe it can actually fail.

Jes