Re: rts7751r2d cf_ide rework

[Paul M. <le...@li...>]

On Wed, Nov 07, 2007 at 11:38:42AM +0900, Magnus Damm wrote:
> On Nov 7, 2007 8:49 AM, Paul Mundt <le...@li...> wrote:
> > On Tue, Nov 06, 2007 at 08:43:16PM +0900, Magnus Damm wrote:
> > > I happen to have two of these R2D-1 boards and I can't get CF working
> > > using the latest kernel (latest sh-2.6 git, post 2.6.24-rc1) :
> > >
> > > ...
> > > irq 107: nobody cared (try booting with the "irqpoll" option)
> > > [stack dump]
> > > [call trace]
> > > handlers:
> > > [<8c24f660>] (ata_interrupt+0x0/0x240)
> > > Disabling IRQ #107
> > > ...
> > >
> > Did R2D-1 CF work for you with IRQs prior to this change? And does
> > reverting it fix get R2D-1 CF working again? The nobody cared thing is
> > curious, if you can provide the stack dump/call trace and register state
> > it might shed some light on why ata_interrupt isn't handling it.
> 
> The CF for R2D-1 was disabled without this patch - mainly because I
> had trouble trying to use it. I thought it had something to do with
> the access size and this patch would solve it, but apparently not. I
> wonder if CF support in 2.6.24-rc2 works on R2D-1 for other people...
> 
The access size problem causes a bus lock, so that's still a problem,
even if IRQs are hosed.

> I'm guessing that something is wrong with trigger level. The R2D-1
> documentation does say something about the CF IDE interrupt level to
> be H while the rest of the sources are L what ever that means. I don't
> think we can control it in the FPGA either.
> 
That suggestions that R2D-1 is using level high. It's possible to set the
sense selection on this already at request_irq() time, blackfin ended up
having to do this, and so added the ability to pass on IRQ flags via
private data. See pata_platform_info->irq_flags in linux/pata_platform.h.
I suppose we should be setting the sense selection level high for all of
the R2D boards, in that case.

> > > I can get things working by hacking in code to enable polling in
> > > pata_platform.c though, so I assume that the IO-ports are ok.
> > >
> > It might be worthwhile making the polling configurable, if you want to
> > tidy up a patch for that we can see if it's something work abstracting.
> > It's handy for debugging, at least.
> 
> The generic ata code needed an IRQ last time i checked. Otherwise i'd
> prefer to not set the interrupt in the platform data to enable
> polling. In the end the only thing required is this (mangled patch
> hunk):
> 
That's easy enough to fix. How about this?

---

 drivers/ata/libata-core.c   |    7 +++++++
 drivers/ata/pata_platform.c |   26 ++++++++++++++++++++------
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index ec3ce12..a0cd6bb 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -7178,6 +7178,9 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
  *	request IRQ and register it.  This helper takes necessasry
  *	arguments and performs the three steps in one go.
  *
+ *	A NULL @irq_handler or invalid IRQ skips the IRQ registration
+ *	and expects the host to have set polling mode on the port.
+ *
  *	LOCKING:
  *	Inherited from calling layer (may sleep).
  *
@@ -7194,6 +7197,10 @@ int ata_host_activate(struct ata_host *host, int irq,
 	if (rc)
 		return rc;
 
+	/* Special case for polling mode */
+	if (!irq_handler || irq < 0)
+		return ata_host_register(host, sht);
+
 	rc = devm_request_irq(host->dev, irq, irq_handler, irq_flags,
 			      dev_driver_string(host->dev), host);
 	if (rc)
diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index fc72a96..1d712ec 100644
--- a/drivers/ata/pata_platform.c
+++ b/drivers/ata/pata_platform.c
@@ -1,7 +1,7 @@
 /*
  * Generic platform device PATA driver
  *
- * Copyright (C) 2006  Paul Mundt
+ * Copyright (C) 2006 - 2007  Paul Mundt
  *
  * Based on pata_pcmcia:
  *
@@ -22,7 +22,7 @@
 #include <linux/pata_platform.h>
 
 #define DRV_NAME "pata_platform"
-#define DRV_VERSION "1.1"
+#define DRV_VERSION "1.2"
 
 static int pio_mask = 1;
 
@@ -137,11 +137,12 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 	struct ata_port *ap;
 	struct pata_platform_info *pp_info;
 	unsigned int mmio;
+	int irq;
 
 	/*
 	 * Simple resource validation ..
 	 */
-	if (unlikely(pdev->num_resources != 3)) {
+	if ((pdev->num_resources != 3) && (pdev->num_resources != 2)) {
 		dev_err(&pdev->dev, "invalid number of resources\n");
 		return -EINVAL;
 	}
@@ -173,6 +174,11 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 		(ctl_res->flags == IORESOURCE_MEM));
 
 	/*
+	 * And the IRQ
+	 */
+	irq = platform_get_irq(pdev, 0);
+
+	/*
 	 * Now that that's out of the way, wire up the port..
 	 */
 	host = ata_host_alloc(&pdev->dev, 1);
@@ -185,6 +191,14 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 	ap->flags |= ATA_FLAG_SLAVE_POSS;
 
 	/*
+	 * Use polling mode if there's no IRQ
+	 */
+	if (irq < 0) {
+		ap->flags |= ATA_FLAG_PIO_POLLING;
+		ata_port_desc(ap, "no IRQ, using PIO polling");
+	}
+
+	/*
 	 * Handle the MMIO case
 	 */
 	if (mmio) {
@@ -213,9 +227,9 @@ static int __devinit pata_platform_probe(struct platform_device *pdev)
 		      (unsigned long long)ctl_res->start);
 
 	/* activate */
-	return ata_host_activate(host, platform_get_irq(pdev, 0),
-				 ata_interrupt, pp_info ? pp_info->irq_flags
-				 : 0, &pata_platform_sht);
+	return ata_host_activate(host, irq, ata_interrupt,
+				 pp_info ? pp_info->irq_flags : 0,
+				 &pata_platform_sht);
 }
 
 /**