I2O: SPARC fixes

Changes:
--------
- Fixed lot of BE <-> LE bugs which prevent it from working on SPARC.

Index: linux-2.6.14/drivers/message/i2o/Kconfig
===================================================================
@@ -24,6 +24,18 @@
 
 	  If unsure, say N.
 
+config I2O_LCT_NOTIFY_ON_CHANGES
+	bool "Enable LCT notification"
+	depends on I2O
+	default y
+	---help---
+	  Only say N here if you have a I2O controller from SUN. The SUN
+	  firmware doesn't support LCT notification on changes. If this option
+	  is enabled on such a controller the driver will hang up in a endless
+	  loop. On all other controllers say Y.
+
+	  If unsure, say Y.
+
 config I2O_EXT_ADAPTEC
 	bool "Enable Adaptec extensions"
 	depends on I2O

Index: linux-2.6.14/drivers/message/i2o/exec-osm.c
===================================================================
@@ -72,7 +72,7 @@
 
 	wait = kmalloc(sizeof(*wait), GFP_KERNEL);
 	if (!wait)
-		return ERR_PTR(-ENOMEM);
+		return NULL;
 
 	memset(wait, 0, sizeof(*wait));
 

@@ -266,8 +266,8 @@
 	struct i2o_device *dev = to_i2o_device(d);
 	u16 id;
 
-	if (i2o_parm_field_get(dev, 0x0000, 0, &id, 2)) {
-		sprintf(buf, "0x%04x", id);
+	if (!i2o_parm_field_get(dev, 0x0000, 0, &id, 2)) {
+		sprintf(buf, "0x%04x", le16_to_cpu(id));
 		return strlen(buf) + 1;
 	}
 

@@ -288,8 +288,8 @@
 	struct i2o_device *dev = to_i2o_device(d);
 	u16 id;
 
-	if (i2o_parm_field_get(dev, 0x0000, 1, &id, 2)) {
-		sprintf(buf, "0x%04x", id);
+	if (!i2o_parm_field_get(dev, 0x0000, 1, &id, 2)) {
+		sprintf(buf, "0x%04x", le16_to_cpu(id));
 		return strlen(buf) + 1;
 	}
 

@@ -359,7 +359,9 @@
 	if (i2o_device_parse_lct(c) != -EAGAIN)
 		change_ind = c->lct->change_ind + 1;
 
+#ifdef CONFIG_I2O_LCT_NOTIFY_ON_CHANGES
 	i2o_exec_lct_notify(c, change_ind);
+#endif
 };
 
 /**

@@ -507,7 +509,8 @@
 
 	dev = &c->pdev->dev;
 
-	if (i2o_dma_realloc(dev, &c->dlct, sb->expected_lct_size, GFP_KERNEL))
+	if (i2o_dma_realloc
+	    (dev, &c->dlct, le32_to_cpu(sb->expected_lct_size), GFP_KERNEL))
 		return -ENOMEM;
 
 	msg = i2o_msg_get_wait(c, I2O_TIMEOUT_MESSAGE_GET);

Index: linux-2.6.14/drivers/message/i2o/i2o_block.c
===================================================================
@@ -1050,8 +1050,8 @@
 	int rc;
 	u64 size;
 	u32 blocksize;
-	u32 flags, status;
 	u16 body_size = 4;
+	u16 power;
 	unsigned short max_sectors;
 
 #ifdef CONFIG_I2O_EXT_ADAPTEC

@@ -1109,22 +1109,20 @@
 	 *      Ask for the current media data. If that isn't supported
 	 *      then we ask for the device capacity data
 	 */
-	if (i2o_parm_field_get(i2o_dev, 0x0004, 1, &blocksize, 4) ||
-	    i2o_parm_field_get(i2o_dev, 0x0000, 3, &blocksize, 4)) {
-		blk_queue_hardsect_size(queue, blocksize);
+	if (!i2o_parm_field_get(i2o_dev, 0x0004, 1, &blocksize, 4) ||
+	    !i2o_parm_field_get(i2o_dev, 0x0000, 3, &blocksize, 4)) {
+		blk_queue_hardsect_size(queue, le32_to_cpu(blocksize));
 	} else
 		osm_warn("unable to get blocksize of %s\n", gd->disk_name);
 
-	if (i2o_parm_field_get(i2o_dev, 0x0004, 0, &size, 8) ||
-	    i2o_parm_field_get(i2o_dev, 0x0000, 4, &size, 8)) {
-		set_capacity(gd, size >> KERNEL_SECTOR_SHIFT);
+	if (!i2o_parm_field_get(i2o_dev, 0x0004, 0, &size, 8) ||
+	    !i2o_parm_field_get(i2o_dev, 0x0000, 4, &size, 8)) {
+		set_capacity(gd, le64_to_cpu(size) >> KERNEL_SECTOR_SHIFT);
 	} else
 		osm_warn("could not get size of %s\n", gd->disk_name);
 
-	if (!i2o_parm_field_get(i2o_dev, 0x0000, 2, &i2o_blk_dev->power, 2))
-		i2o_blk_dev->power = 0;
-	i2o_parm_field_get(i2o_dev, 0x0000, 5, &flags, 4);
-	i2o_parm_field_get(i2o_dev, 0x0000, 6, &status, 4);
+	if (!i2o_parm_field_get(i2o_dev, 0x0000, 2, &power, 2))
+		i2o_blk_dev->power = power;
 
 	i2o_event_register(i2o_dev, &i2o_block_driver, 0, 0xffffffff);
 

Index: linux-2.6.14/drivers/message/i2o/i2o_scsi.c
===================================================================
@@ -113,7 +113,7 @@
 
 	list_for_each_entry(i2o_dev, &c->devices, list)
 	    if (i2o_dev->lct_data.class_id == I2O_CLASS_BUS_ADAPTER) {
-		if (i2o_parm_field_get(i2o_dev, 0x0000, 0, &type, 1)
+		if (!i2o_parm_field_get(i2o_dev, 0x0000, 0, &type, 1)
 		    && (type == 0x01))	/* SCSI bus */
 			max_channel++;
 	}

@@ -146,7 +146,7 @@
 	i = 0;
 	list_for_each_entry(i2o_dev, &c->devices, list)
 	    if (i2o_dev->lct_data.class_id == I2O_CLASS_BUS_ADAPTER) {
-		if (i2o_parm_field_get(i2o_dev, 0x0000, 0, &type, 1)
+		if (!i2o_parm_field_get(i2o_dev, 0x0000, 0, &type, 1)
 		    && (type == 0x01))	/* only SCSI bus */
 			i2o_shost->channel[i++] = i2o_dev;
 

@@ -238,13 +238,15 @@
 			u8 type;
 			struct i2o_device *d = i2o_shost->channel[0];
 
-			if (i2o_parm_field_get(d, 0x0000, 0, &type, 1)
+			if (!i2o_parm_field_get(d, 0x0000, 0, &type, 1)
 			    && (type == 0x01))	/* SCSI bus */
-				if (i2o_parm_field_get(d, 0x0200, 4, &id, 4)) {
+				if (!i2o_parm_field_get(d, 0x0200, 4, &id, 4)) {
 					channel = 0;
 					if (i2o_dev->lct_data.class_id ==
 					    I2O_CLASS_RANDOM_BLOCK_STORAGE)
-						lun = i2o_shost->lun++;
+						lun =
+						    cpu_to_le64(i2o_shost->
+								lun++);
 					else
 						lun = 0;
 				}

@@ -253,10 +255,10 @@
 		break;
 
 	case I2O_CLASS_SCSI_PERIPHERAL:
-		if (i2o_parm_field_get(i2o_dev, 0x0000, 3, &id, 4) < 0)
+		if (i2o_parm_field_get(i2o_dev, 0x0000, 3, &id, 4))
 			return -EFAULT;
 
-		if (i2o_parm_field_get(i2o_dev, 0x0000, 4, &lun, 8) < 0)
+		if (i2o_parm_field_get(i2o_dev, 0x0000, 4, &lun, 8))
 			return -EFAULT;
 
 		parent = i2o_iop_find_device(c, i2o_dev->lct_data.parent_tid);

@@ -281,20 +283,22 @@
 		return -EFAULT;
 	}
 
-	if (id >= scsi_host->max_id) {
-		osm_warn("SCSI device id (%d) >= max_id of I2O host (%d)", id,
-			 scsi_host->max_id);
+	if (le32_to_cpu(id) >= scsi_host->max_id) {
+		osm_warn("SCSI device id (%d) >= max_id of I2O host (%d)",
+			 le32_to_cpu(id), scsi_host->max_id);
 		return -EFAULT;
 	}
 
-	if (lun >= scsi_host->max_lun) {
-		osm_warn("SCSI device id (%d) >= max_lun of I2O host (%d)",
-			 (unsigned int)lun, scsi_host->max_lun);
+	if (le64_to_cpu(lun) >= scsi_host->max_lun) {
+		osm_warn("SCSI device lun (%lu) >= max_lun of I2O host (%d)",
+			 (long unsigned int)le64_to_cpu(lun),
+			 scsi_host->max_lun);
 		return -EFAULT;
 	}
 
 	scsi_dev =
-	    __scsi_add_device(i2o_shost->scsi_host, channel, id, lun, i2o_dev);
+	    __scsi_add_device(i2o_shost->scsi_host, channel, le32_to_cpu(id),
+			      le64_to_cpu(lun), i2o_dev);
 
 	if (IS_ERR(scsi_dev)) {
 		osm_warn("can not add SCSI device %03x\n",

@@ -306,7 +310,8 @@
 			  "scsi");
 
 	osm_info("device added (TID: %03x) channel: %d, id: %d, lun: %d\n",
-		 i2o_dev->lct_data.tid, channel, id, (unsigned int)lun);
+		 i2o_dev->lct_data.tid, channel, le32_to_cpu(id),
+		 (unsigned int)le64_to_cpu(lun));
 
 	return 0;
 };

From: Markus Lidel <[email protected]>
Date: Tue, 15 Nov 2005 10:31:09 +0100

Hello,

OK, i'll look at it... Thanks for the hint!

Best regards,

Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

From: Markus Lidel <[email protected]>
Date: Wed, 16 Nov 2005 13:25:50 +0100

Actually, my idea won't work if the card is used in a non-Sparc system. Do these cards have PCI subsystem vendor or device ID's that identify it as a Sun card?

Hello,

I don't know, because currently i only look for the PCI class. I'll check it out and report back...

Thank you very much for your help!

Best regards,

Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

Hello,

Here's the output of lspci:

0003:01:03.0 Memory controller: Adaptec (formerly DPT) Domino RAID Engine (rev 02)
Subsystem: Adaptec (formerly DPT) Domino RAID Engine
Flags: bus master, medium devsel, latency 32, IRQ 0082efe0 BIST result: 00
Memory at 000001c980100000 (32-bit, non-prefetchable) [size=64K] Memory at 000001c988000000 (32-bit, prefetchable) [size=128M]

0003:01:04.0 I2O: Adaptec (formerly DPT) SmartRAID V Controller (rev 03)
Subsystem: Adaptec (formerly DPT) SmartRAID V Controller
Flags: bus master, medium devsel, latency 1, IRQ 0082ef80 BIST result: 00
Memory at 000001c990000000 (32-bit, non-prefetchable) [size=2M] I/O ports at 0000000002010400 [size=256]
Expansion ROM at 0000000080000000 [disabled] [size=32K]

As it looks it's equal to the "Intel" based cards...

Don't think it will work then, right?

Best regards,

Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

From: Markus Lidel <[email protected]>
Date: Sat, 19 Nov 2005 02:07:39 +0100

No, it won't.

Ho hum, I guess keep it a config option for now until we find a way to auto-detect this reliably.

The notify functionality is mandatory. You are seeing the same cards fail on sparc but work on x86. This sounds to me a lot more like an unfound endian bug that needs fixing than a real lack of support

From: Alan Cox <[email protected]>
Date: Sat, 19 Nov 2005 03:30:39 +0000

That's very possible, but it also could be that the cards that fail only on Sparc have Sun forth firmware on them, which would thus only load firmware on Sparc boxes.

I still think the endianness theory is more likely, however.

Perhaps the I2O datastructures should be endian annotated and then pushed through sparse. :-)

The firmware on the DPT I2O card processor is kept in flash on the processor. The BIOS setup code might be different but nothing at the business end of things

Hi...

That's right there are two different firmwares. One for "Intel"-cards and one for SUN-cards. The LCT-Notify function works as follow.

The LCT on the I2O controller has a change indicator which is incremented by the I2O controller each time something changes (e. g. a disk is removed or added or so). The i2o_exec_lct_notify() function only send a number to the I2O controller. If this number is <= then the current change indicator of the LCT, then the controller send out the new LCT to the OS. On startup the change indicator is normally 1. So if there is some BE<->LE issue, then the function wouldn't send in 0x00000002 but 0x02000000. What should happen then is that you won't be notified for a long time. But that didn't happened, and the controller immediately send out the LCT, although if you send in 0xffffffff. And as i'm told, this is wanted for some reason for the "SUN"-firmware.

It should probably be "fixed" if you upload a "Intel"-firmware onto the I2O controller, but because i don't own a SPARC machine with a PCI slot myself, i don't want to try it out on someones else machine and probably break it :-)

If someone has a SPARC machine with an I2O controller, and he want to try it out, please let me know...

Bye...
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

Hi...

Sounds good to me. Could i then find out if some BE<=>LE issues are still left? If so, is there a document which describes how it is done, or at least has a driver added it already?

Bye...
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

Hmm...

struct i2o_message {
        union {
                struct {
                        u8 version_offset;
                        u8 flags;
                        u16 size;
                        u32 target_tid:12;
                        u32 init_tid:12;
                        u32 function:8;
                        u32 icntxt;     /* initiator context */
                        u32 tcntxt;     /* transaction context */
                } s;
                u32 head[4];
        } u;
        /* List follows */
        u32 body[0];
};

is one hell of an endianness issue right fscking there.

	And here's another fun one:
                evt->size = size;
                evt->tcntxt = le32_to_cpu(msg->u.s.tcntxt);
                evt->event_indicator = le32_to_cpu(msg->body[0]);
                memcpy(&evt->tcntxt, &msg->u.s.tcntxt, size * 4);

in i2o_driver_dispatch().

We have
struct i2o_event {
struct work_struct work;
struct i2o_device *i2o_dev; /* I2O device pointer from which the
event reply was initiated */
u16 size; /* Size of data in 32-bit words */
u32 tcntxt; /* Transaction context used at
registration */
u32 event_indicator; /* Event indicator from reply */
u32 data[0]; /* Event data from reply */
};

and
in msg tcntxt goes right before body[0]. So we copy two 32bit values converting to host order and then immediately overwrite them with unconverted ones.

Looks like these assignments were meant to go *after* memcpy() and serve as a fixup...

Gaaack... Old code used to be

                       evt = kmalloc(size * 4 + sizeof(*evt), GFP_ATOMIC);
                       if (!evt)
                               return -ENOMEM;
                       memset(evt, 0, size * 4 + sizeof(*evt));

                       evt->size = size;
                       memcpy_fromio(&evt->tcntxt, &msg->u.s.tcntxt,
                                    (size + 2) * 4);

Then it became

               evt->size = size;
               evt->tcntxt = readl(&msg->u.s.tcntxt);
               evt->event_indicator = readl(&msg->body[0]);
               memcpy_fromio(&evt->tcntxt, &msg->u.s.tcntxt, size * 4);

See the problem with it? The last copy should be from &msg->body[1] to evt->data. As it is, we do not copy the last 8 bytes (which might or might not be a problem) *AND* we overwrite tcntxt and event_indicator with bus-endian values right after having host-endian ones carefully assigned to them.

Breakage happened in
diff-tree 61fbfa8129c1771061a0e9f47747854293081c5b (from 34d6e07570ef74b96513145

Author: Markus Lidel <[email protected]>
Date:   Thu Jun 23 22:02:11 2005 -0700

[PATCH] I2O: bugfixes and compability enhancements

Hello,

At the moment it's not a problem, because the event system is not used...

Yep, you're right... the memcpy_fromio is wrong... It should be:

memcpy_fromio(&evt->body[1], &msg->body[1], size * 4);

as you already mentioned...

Thanks for your help.

Best regards,

Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com

BTW, should that be memcpy() or memcpy_fromio()? IOW, what memory does msg point to?

                evt->data, surely?

Hello,

Hehehe, probably i should first try to compile it before replying :-) But better late then never :-)

Here the proper line:

  memcpy(&evt->data, &msg->body[1], size * 4);

Thanks for you help!

Best regards,

Markus Lidel
------------------------------------------
Markus Lidel (Senior IT Consultant)

Shadow Connect GmbH
Carl-Reisch-Weg 12
D-86381 Krumbach
Germany

Phone:  +49 82 82/99 51-0
Fax:    +49 82 82/99 51-11

E-Mail: [email protected]
URL:    http://www.shadowconnect.com