Skip to contentSkip to conversation

IBM Power RAID driver (ipr) 2.0.6

PATCH In review Brian King 4 participants 6 messages May 4, 2004 – May 10, 2004

Subsystems (2)
lore
Share & tools
lore

Brian King

author maintainer
#0 lore raw

PATCH 1/1 IBM Power RAID driver (ipr) 2.0.6

James,

Here is the latest version of the LLD for the IBM Power RAID family of adapters. This includes several IBM iSeries and pSeries SCSI adapters. Please apply.

Changes since last submission include:

1. Don't run BIST at module load time since it is unnecessary and it increases module load time dramatically.
2. Adapter dump fixes.
3. Add adapter type and microcode version to the dump.
4. Don't zero DMA address of hostrcb during adapter reset, which was causing a DMA to address 0.
5. Change to use the scsi_timeout_mod patch rather than the sd_timeout_mod patch

The driver can be found at:

http://www-124.ibm.com/storageio/ipr/patch-2.6.6-rc3-ipr-2.0.6.gz

The patch to provide pci-ids can be found here:

http://www-124.ibm.com/storageio/ipr/patch-2.6.3-ipr-pci-ids

The now required scsi_timeout_mod patch can be found:

http://www-124.ibm.com/storageio/ipr/patch-2.6.6-rc3-scsi_timeout_mod

Thanks

Signature · 3 lines
Brian King
eServer Storage I/O
IBM Linux Technology Center
#1 lore raw
Quoted 5 lines · Brian King

James,

Here is the latest version of the LLD for the IBM Power RAID family of adapters. This includes several IBM iSeries and pSeries SCSI adapters. Please apply.

BTW, any plans to add support for the older iSeries models supported by the binary only ibmsis driver?

author maintainer
#2 lore raw
Quoted 11 lines · Christoph Hellwig
Quoted 5 lines · Brian King

James,

Here is the latest version of the LLD for the IBM Power RAID family of adapters. This includes several IBM iSeries and pSeries SCSI adapters. Please apply.

BTW, any plans to add support for the older iSeries models supported by the binary only ibmsis driver?

No. The older models have several "issues" with them. First of all, while the firmware interface has similarities, there are enough differences to make writing a driver to support both somewhat ugly. Yes, ibmsis did both, but it was also a pretty bulky driver. Secondly, the way RAID is done on the legacy adapters is completely different. It is done how OS/400 needs it done, which means data striping is not done in the adapter, but rather by the system, as each device is exposed to the system. Thirdly, the adapters based on the Chukar chip have lots of "features" with them. To name a few, BIST does not work. The only way to reset the card is to assert PCI reset. PCI config space can come up in either big endian or little endian mode, which requires creative programming to detect and fixup.

Not supporting the legacy adapters resulted in a *much* cleaner OSS driver.

ibmsis will continue to be supported for bug fixes, etc. for the older adapters, but all new adapters will be supported only by ipr.

-Brian

Signature · 3 lines
Brian King
eServer Storage I/O
IBM Linux Technology Center
#3 lore raw
In reply to Brian King
Quoted 3 lines · Brian King

Here is the latest version of the LLD for the IBM Power RAID family of adapters. This includes several IBM iSeries and pSeries SCSI adapters. Please apply.

OK, I put this in the tree. However, it fails to compile on parisc:

CC [M] drivers/scsi/ipr.o
{standard input}: Assembler messages:
{standard input}:1788: Error: Field not properly aligned [8] (1028). {standard input}:1788: Error: Invalid operands
[...]

The problem is your use of list_head pointers in packed structures. You can't simply pass a pointer to a misaligned structure to a common kernel function and expect it to work: the packed attribute is lost doing this and what you end up with is a misaligned pointer reference.

The attached actually allows me to compile, but all it's doing is hiding the problem.

James

===== drivers/scsi/ipr.h 1.1 vs edited =====

drivers/scsi/ipr.h +1 -1
@@ -666,7 +666,7 @@
 
 	/* Driver added data */
 	u32 hostrcb_dma;
-	struct list_head queue;
+	struct list_head queue __attribute__((aligned(sizeof(unsigned long))));
 }__attribute__((packed, aligned (4)));
 
 #define IPR_HOSTRCB_SZ offsetof(struct ipr_hostrcb, hostrcb_dma)
#4 lore raw
Quoted 9 lines · James Bottomley
Quoted 3 lines · Brian King

Here is the latest version of the LLD for the IBM Power RAID family of adapters. This includes several IBM iSeries and pSeries SCSI adapters. Please apply.

The problem is your use of list_head pointers in packed structures. You can't simply pass a pointer to a misaligned structure to a common kernel function and expect it to work: the packed attribute is lost doing this and what you end up with is a misaligned pointer reference.

It also causes gcc to pessimise references to this structure; often doing byte-loads instead of larger quantities. Why does this struct need to be packed?

Signature · 6 lines
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain
author maintainer
#5 lore raw
Quoted 14 lines · Matthew Wilcox
Quoted 10 lines · James Bottomley
Quoted 3 lines · Brian King

Here is the latest version of the LLD for the IBM Power RAID family of adapters. This includes several IBM iSeries and pSeries SCSI adapters. Please apply.

The problem is your use of list_head pointers in packed structures. You can't simply pass a pointer to a misaligned structure to a common kernel function and expect it to work: the packed attribute is lost doing this and what you end up with is a misaligned pointer reference.

Quoted 3 lines

It also causes gcc to pessimise references to this structure; often doing byte-loads instead of larger quantities. Why does this struct need to be packed?

It is a shared structure with the adapter with a little bit of added driver data at the end of it. I'll roll a patch to clean this up. The better way to do this is probably to break out the data that needs to be packed into its own structure.

-Brian

Signature · 3 lines
Brian King
eServer Storage I/O
IBM Linux Technology Center