xilinx_sysace

xilinx_sysace

[Jon Masters]

Hi there,

I am doing some work on a board port (Xilinx Virtex II Pro 405D
running 2.4.23) which uses the xilinx_sysace driver on a Xilinx Virtex
II Pro system (not the ML300 board - an inhouse custom thing). The
driver comes in several pieces because Xilinx supply a (rather
unpleasant) HAL which Monta's adapter.c driver code wraps to create a
block device.

The driver works fine for me as a read only device but I'm still
seeing occasional hard locks on writes (looks to be something not
getting io_request_lock at the right moment - currently
investigating). They use a kernel thread which just sits and
compensates for the hardware not being able to signal when it is ready
for a new request - and I think there's a race there.

I looked in to this before but got pulled off to look at some other
more pressing bits, and I want to fix this now. Does anyone else have
problems writing with it though? Of the people I have spoken to about
this most seem to just use System ACE as a configuration device but we
use it for configuration, kernel image, userland, system
settings...etc.

Cheers,

Jon.

Re: xilinx_sysace

[Jon Masters]

On Mon, 6 Sep 2004 15:24:11 +0100, Jon Masters <jonmasters@gmail.com> wrote:

> The driver works fine for me as a read only device but I'm still
> seeing occasional hard locks on writes (looks to be something not
> getting io_request_lock at the right moment - currently
> investigating). They use a kernel thread which just sits and
> compensates for the hardware not being able to signal when it is ready
> for a new request - and I think there's a race there.

It's as I thought on and off and then on again - the code checks out
ok (it's not pretty but it works) - and I seem to be getting unwanted
extra unhandled interrupts from the hardware. This driver needs a lot
of cleanup anyway - it doesn't handle these kinds of error state, nor
does it handle the removal of a mounted CompactFlash, and a dozen
other typical problems. I'll post a patch when I've solved the main
problem - moan at me by private mail if you're using this, having
similar issues, and feel like helping.

Jon.

[PATCH] xsa_use_interrupts flag [WAS: xilinx_sysace]

[Jon Masters]

On Sun, 19 Sep 2004 21:18:14 +0100, Jon Masters <jonmasters@gmail.com> wrote:

> It's as I thought on and off and then on again - the code checks out
> ok (it's not pretty but it works) - and I seem to be getting unwanted
> extra unhandled interrupts from the hardware. This driver needs a lot
> of cleanup anyway - it doesn't handle these kinds of error state, nor
> does it handle the removal of a mounted CompactFlash, and a dozen
> other typical problems. I'll post a patch when I've solved the main
> problem - moan at me by private mail if you're using this, having
> similar issues, and feel like helping.

Hi all,

I have spoken to a number of people about this ongoing issue with the
Xilinx System ACE hardware that I am using (I have a Memec board - not
the Xilinx one) and the fact that the hardware insists on generating
an extra interrupt on SectorWrite operations which is neither
documented nor otherwise explainable (but I'd love it if someone would
enlighten me).

So I've added a trivial patch to xilinx_sysace to provide a flag for
disabling interrupts at least for now. I am working on a bunch of
other Virtex II Pro bits and pieces that I'll feed upstream when
they're ready but for now xsa_interrupt should probably also check
whether the request queue is empty in the case that it's called with
no good reason.

The following patch has been briefly tested on a modified inhouse
2.4.23 kernel which uses a few drivers from the Monta tree (like
xilinux_sysace) but otherwise differs quite heavily - I don't rely on
any generated xparameters and other EDK nonesense and I don't want to
rely on that in any way either - least of all for determining whether
or not to use interrupts.

Jon.

diff --unified --recursive --new-file xilinx_sysace_mvista/adapter.c
xilinx_sysace_nointr/adapter.c
--- xilinx_sysace_mvista/adapter.c	2004-03-15 11:41:30.000000000 +0000
+++ xilinx_sysace_nointr/adapter.c	2004-09-22 17:31:58.000000000 +0100
@@ -6,6 +6,10 @@
  * Author: MontaVista Software, Inc.
  *         source@mvista.com
  *
+ * History:
+ *         22/09/2004 - Added xsa_use_interrupts.
+ *         Jon Masters <jcm@jonmasters.org>.
+ *
  * 2002 (c) MontaVista, Software, Inc.  This file is licensed under the terms
  * of the GNU General Public License version 2.  This program is licensed
  * "as is" without any warranty of any kind, whether express or implied.
@@ -59,9 +63,17 @@
 #include <xbasic_types.h>
 #include "xsysace.h"
 
+/*
+ * We have to disable interrupts on certain boards where writing causes an
+ * additional unexpected hardware interrupt on sector write completion.
+ */
+
+static int xsa_use_interrupts = 0;
+
 MODULE_AUTHOR("MontaVista Software, Inc. <source@mvista.com>");
 MODULE_DESCRIPTION("Xilinx System ACE block driver");
 MODULE_LICENSE("GPL");
+MODULE_PARM(xsa_enable_interrupts,"i");
 
 /*
  * We have to wait for a lock and for the CompactFlash to not be busy
@@ -497,11 +509,17 @@
 			       DEVICE_NAME, stat, req_str, sector);
 			xsa_complete_request(0);	/* Request failed. */
 		}
+
+		if ((!xsa_use_interrupts) && (stat == XST_SUCCESS)) {
+			printk("RIQC: request complete.\n");
+			xsa_complete_request(1);
+		}
 	}
 
-      exit:
+	
+ exit:
 	remove_wait_queue(&req_wait, &wait);
-
+	
 	xsa_task = NULL;
 	complete_and_exit(&task_sync, 0);
 }
@@ -757,16 +775,25 @@
 		return -ENODEV;
 	}
 
-	i = request_irq(XSA_IRQ, xsysace_interrupt, 0, DEVICE_NAME, NULL);
-	if (i) {
-		printk(KERN_ERR "%s: Could not allocate interrupt %d.\n",
-		       DEVICE_NAME, XSA_IRQ);
-		cleanup();
-		return i;
+	if (xsa_use_interrupts) {
+
+		i = request_irq(XSA_IRQ, xsysace_interrupt, 0, DEVICE_NAME, NULL);
+		if (i) {
+			printk(KERN_ERR "%s: Could not allocate interrupt %d.\n",
+			       DEVICE_NAME, XSA_IRQ);
+			cleanup();
+			return i;
+		}
+		reqirq = 1;
 	}
-	reqirq = 1;
+
 	XSysAce_SetEventHandler(&SysAce, EventHandler, (void *) NULL);
-	XSysAce_EnableInterrupt(&SysAce);
+	
+	if (xsa_use_interrupts) {
+		XSysAce_EnableInterrupt(&SysAce);
+	} else {
+		XSysAce_DisableInterrupt(&SysAce);
+	}
 
 	/* Time to identify the drive. */
 	while (XSysAce_Lock(&SysAce, 0) == XST_DEVICE_BUSY)
@@ -832,10 +859,17 @@
 	register_disk(&xsa_gendisk, MKDEV(xsa_major, 0),
 		      NR_HD << PARTN_BITS, &xsa_fops, size);
 
-	printk(KERN_INFO
-	       "%s at 0x%08X mapped to 0x%08X, irq=%d, %ldKB\n",
-	       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
-	       size / 2);
+	if (xsa_use_interrupts) {
+		printk(KERN_INFO
+		       "%s at 0x%08X mapped to 0x%08X, irq=%d, %ldKB\n",
+		       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
+		       size / 2);
+	} else {
+		printk(KERN_INFO
+		       "%s at 0x%08X mapped to 0x%08X, polled IO, %ldKB\n",
+		       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
+		       size / 2);
+	}
 
 	/* Hook our reset function into system's restart code. */
 	old_restart = ppc_md.restart;

[PATCH] xsa_use_interrupts flag [WAS: xilinx_sysace]

[Jon Masters]

On Sun, 19 Sep 2004 21:18:14 +0100, Jon Masters <jonmasters@gmail.com> wrote:

> It's as I thought on and off and then on again - the code checks out
> ok (it's not pretty but it works) - and I seem to be getting unwanted
> extra unhandled interrupts from the hardware. This driver needs a lot
> of cleanup anyway - it doesn't handle these kinds of error state, nor
> does it handle the removal of a mounted CompactFlash, and a dozen
> other typical problems. I'll post a patch when I've solved the main
> problem - moan at me by private mail if you're using this, having
> similar issues, and feel like helping.

Hi all,

I have spoken to a number of people about this ongoing issue with the
Xilinx System ACE hardware that I am using (I have a Memec board - not
the Xilinx one) and the fact that the hardware insists on generating
an extra interrupt on SectorWrite operations which is neither
documented nor otherwise explainable (but I'd love it if someone would
enlighten me).

So I've added a trivial patch to xilinx_sysace to provide a flag for
disabling interrupts at least for now. I am working on a bunch of
other Virtex II Pro bits and pieces that I'll feed upstream when
they're ready but for now xsa_interrupt should probably also check
whether the request queue is empty in the case that it's called with
no good reason.

The following patch has been briefly tested on a modified inhouse
2.4.23 kernel which uses a few drivers from the Monta tree (like
xilinux_sysace) but otherwise differs quite heavily - I don't rely on
any generated xparameters and other EDK nonesense and I don't want to
rely on that in any way either - least of all for determining whether
or not to use interrupts.

Jon.

diff --unified --recursive --new-file xilinx_sysace_mvista/adapter.c
xilinx_sysace_nointr/adapter.c
--- xilinx_sysace_mvista/adapter.c	2004-03-15 11:41:30.000000000 +0000
+++ xilinx_sysace_nointr/adapter.c	2004-09-22 17:31:58.000000000 +0100
@@ -6,6 +6,10 @@
  * Author: MontaVista Software, Inc.
  *         source@mvista.com
  *
+ * History:
+ *         22/09/2004 - Added xsa_use_interrupts.
+ *         Jon Masters <jcm@jonmasters.org>.
+ *
  * 2002 (c) MontaVista, Software, Inc.  This file is licensed under the terms
  * of the GNU General Public License version 2.  This program is licensed
  * "as is" without any warranty of any kind, whether express or implied.
@@ -59,9 +63,17 @@
 #include <xbasic_types.h>
 #include "xsysace.h"
 
+/*
+ * We have to disable interrupts on certain boards where writing causes an
+ * additional unexpected hardware interrupt on sector write completion.
+ */
+
+static int xsa_use_interrupts = 0;
+
 MODULE_AUTHOR("MontaVista Software, Inc. <source@mvista.com>");
 MODULE_DESCRIPTION("Xilinx System ACE block driver");
 MODULE_LICENSE("GPL");
+MODULE_PARM(xsa_enable_interrupts,"i");
 
 /*
  * We have to wait for a lock and for the CompactFlash to not be busy
@@ -497,11 +509,17 @@
 			       DEVICE_NAME, stat, req_str, sector);
 			xsa_complete_request(0);	/* Request failed. */
 		}
+
+		if ((!xsa_use_interrupts) && (stat == XST_SUCCESS)) {
+			printk("RIQC: request complete.\n");
+			xsa_complete_request(1);
+		}
 	}
 
-      exit:
+	
+ exit:
 	remove_wait_queue(&req_wait, &wait);
-
+	
 	xsa_task = NULL;
 	complete_and_exit(&task_sync, 0);
 }
@@ -757,16 +775,25 @@
 		return -ENODEV;
 	}
 
-	i = request_irq(XSA_IRQ, xsysace_interrupt, 0, DEVICE_NAME, NULL);
-	if (i) {
-		printk(KERN_ERR "%s: Could not allocate interrupt %d.\n",
-		       DEVICE_NAME, XSA_IRQ);
-		cleanup();
-		return i;
+	if (xsa_use_interrupts) {
+
+		i = request_irq(XSA_IRQ, xsysace_interrupt, 0, DEVICE_NAME, NULL);
+		if (i) {
+			printk(KERN_ERR "%s: Could not allocate interrupt %d.\n",
+			       DEVICE_NAME, XSA_IRQ);
+			cleanup();
+			return i;
+		}
+		reqirq = 1;
 	}
-	reqirq = 1;
+
 	XSysAce_SetEventHandler(&SysAce, EventHandler, (void *) NULL);
-	XSysAce_EnableInterrupt(&SysAce);
+	
+	if (xsa_use_interrupts) {
+		XSysAce_EnableInterrupt(&SysAce);
+	} else {
+		XSysAce_DisableInterrupt(&SysAce);
+	}
 
 	/* Time to identify the drive. */
 	while (XSysAce_Lock(&SysAce, 0) == XST_DEVICE_BUSY)
@@ -832,10 +859,17 @@
 	register_disk(&xsa_gendisk, MKDEV(xsa_major, 0),
 		      NR_HD << PARTN_BITS, &xsa_fops, size);
 
-	printk(KERN_INFO
-	       "%s at 0x%08X mapped to 0x%08X, irq=%d, %ldKB\n",
-	       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
-	       size / 2);
+	if (xsa_use_interrupts) {
+		printk(KERN_INFO
+		       "%s at 0x%08X mapped to 0x%08X, irq=%d, %ldKB\n",
+		       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
+		       size / 2);
+	} else {
+		printk(KERN_INFO
+		       "%s at 0x%08X mapped to 0x%08X, polled IO, %ldKB\n",
+		       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
+		       size / 2);
+	}
 
 	/* Hook our reset function into system's restart code. */
 	old_restart = ppc_md.restart;

Re: [PATCH] xsa_use_interrupts flag [WAS: xilinx_sysace]

[Tonnerre]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

Salut,

On Thu, Sep 23, 2004 at 02:12:07AM +0100, Jon Masters wrote:
> @@ -832,10 +859,17 @@
>  	register_disk(&xsa_gendisk, MKDEV(xsa_major, 0),
>  		      NR_HD << PARTN_BITS, &xsa_fops, size);
>  
> -	printk(KERN_INFO
> -	       "%s at 0x%08X mapped to 0x%08X, irq=%d, %ldKB\n",
> -	       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
> -	       size / 2);
> +	if (xsa_use_interrupts) {
> +		printk(KERN_INFO
> +		       "%s at 0x%08X mapped to 0x%08X, irq=%d, %ldKB\n",
> +		       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress, XSA_IRQ,
> +		       size / 2);
> +	} else {
> +		printk(KERN_INFO
> +		       "%s at 0x%08X mapped to 0x%08X, polled IO, %ldKB\n",
> +		       DEVICE_NAME, save_BaseAddress, cfg->BaseAddress,
> +		       size / 2);

(No XSA_IRQ here!)

> +	}
>  
>  	/* Hook our reset function into system's restart code. */
>  	old_restart = ppc_md.restart;

				    Tonnerre

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] xsa_use_interrupts flag [WAS: xilinx_sysace]

[Jeff Angielski]

On Wed, 2004-09-22 at 21:12, Jon Masters wrote:
> On Sun, 19 Sep 2004 21:18:14 +0100, Jon Masters <jonmasters@gmail.com> wrote:
> 
> > It's as I thought on and off and then on again - the code checks out
> > ok (it's not pretty but it works) - and I seem to be getting unwanted
> > extra unhandled interrupts from the hardware. This driver needs a lot
> > of cleanup anyway - it doesn't handle these kinds of error state, nor
> > does it handle the removal of a mounted CompactFlash, and a dozen
> > other typical problems. I'll post a patch when I've solved the main
> > problem - moan at me by private mail if you're using this, having
> > similar issues, and feel like helping.
> 
> Hi all,
> 
> I have spoken to a number of people about this ongoing issue with the
> Xilinx System ACE hardware that I am using (I have a Memec board - not
> the Xilinx one) and the fact that the hardware insists on generating
> an extra interrupt on SectorWrite operations which is neither
> documented nor otherwise explainable (but I'd love it if someone would
> enlighten me).

We are using the SystemACE on our custom MPC8260 platform and I do not
recall seeing extra interrupts on the sector writes.

However, we did uncover some general interrupt issues with interrupt
handling in the PPC kernel.  We were seeing a large number of BAD
interrupts in /proc/interrupts.  Unfortunately, the only information I
have on the work around is this email snippet that I saved:

<begin snippet>
> > This problem was discussed on mailing list before also and you can
> > eliminate this problem by inserting a sync instruction at a
> certainplace in the 8260 interrupt handling code. See, for example,
> > http://www.geocrawler.com/archives/3/8358/2002/11/100/10173445/
> > 
> > Add a __asm__ volatile("sync"); at the end of the m8260_mask_and_ack
> > function in arch/ppc/kernel/ppc8260_pic.c to fix it.
<end snippet>


Jeff Angielski
The PTR Group

Re: [PATCH] xsa_use_interrupts flag [WAS: xilinx_sysace]

[Jon Masters]

On Thu, 23 Sep 2004 06:51:14 +0200, Tonnerre <tonnerre@thundrix.ch> wrote:

> (No XSA_IRQ here!)

Cheers. I also left in a printk that shouldn't be there - just
adopting the early and often philosophy since this has been bothering
me for a while.

Jon.

Re: [PATCH] xsa_use_interrupts flag [WAS: xilinx_sysace]

[Jon Masters]

On Thu, 23 Sep 2004 06:51:14 +0200, Tonnerre <tonnerre@thundrix.ch> wrote:

> (No XSA_IRQ here!)

Cheers. I also left in a printk that shouldn't be there - just
adopting the early and often philosophy since this has been bothering
me for a while.

Jon.

Re: [PATCH] xsa_use_interrupts flag [WAS: xilinx_sysace]

[Jon Masters]

On Thu, 23 Sep 2004 09:36:00 -0400, Jeff Angielski <jeff@theptrgroup.com> wrote:

> We are using the SystemACE on our custom MPC8260 platform and I do not
> recall seeing extra interrupts on the sector writes.

This is on a Virtex II Pro which does not use the interrupt controller
that you mention. I'm quite sure *some* people are able to use this
just fine - otherwise there wouldn't be interrupt driven IO in the
Monta driver, but it doesn't work on all boards, and it's not a simple
bug in the interrupt handling code - it's a real extra unwanted
interrupt from the chip.

The board in question is the Memec one with lots of custom logic, the
xintc and a bunch of other bits running 2.4.23 but without any of the
HAL nonesense and Xilinx xparameters stuff.

Cheers,

Jon.