[PATCH 1/2] libata: Support PIO polling-only hosts.

[PATCH 1/2] libata: Support PIO polling-only hosts.

[Paul Mundt]

By default ata_host_activate() expects a valid IRQ in order to
successfully register the host. This patch enables a special case
for registering polling-only hosts that either don't have IRQs
or have buggy IRQ generation (either in terms of handling or
sensing), which otherwise work fine.

Hosts that want to use polling mode can simply set ATA_FLAG_PIO_POLLING
and pass in a NULL IRQ handler or invalid (< 0) IRQ.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 drivers/ata/libata-core.c |    7 +++++++
 1 file changed, 7 insertions(+)

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)


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[PATCH 2/2] libata: pata_platform: Support polling-mode configuration.

[Paul Mundt]

Some SH boards (old R2D-1 boards) have generally not had working CF
under libata, due to both buswidth issues (handled by Aoi Shinkai
in 43f4b8c7578b928892b6f01d374346ae14e5eb70), and buggy interrupt
controllers. For these sorts of boards simply disabling the IRQ and
polling ends up working fine.

This conditionalizes the IRQ resource for pata_platform and lets
platforms that want to use polling mode simply omit the resource
entirely.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 drivers/ata/pata_platform.c |   33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/ata/pata_platform.c b/drivers/ata/pata_platform.c
index fc72a96..6b2d731 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;
 
@@ -120,15 +120,20 @@ static void pata_platform_setup_port(struct ata_ioports *ioaddr,
  *	Register a platform bus IDE interface. Such interfaces are PIO and we
  *	assume do not support IRQ sharing.
  *
- *	Platform devices are expected to contain 3 resources per port:
+ *	Platform devices are expected to contain at least 2 resources per port:
  *
  *		- I/O Base (IORESOURCE_IO or IORESOURCE_MEM)
  *		- CTL Base (IORESOURCE_IO or IORESOURCE_MEM)
+ *
+ *	and optionally:
+ *
  *		- IRQ	   (IORESOURCE_IRQ)
  *
  *	If the base resources are both mem types, the ioremap() is handled
  *	here. For IORESOURCE_IO, it's assumed that there's no remapping
  *	necessary.
+ *
+ *	If no IRQ resource is present, PIO polling mode is used instead.
  */
 static int __devinit pata_platform_probe(struct platform_device *pdev)
 {
@@ -137,11 +142,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 +179,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 +196,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 +232,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);
 }
 
 /**

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 2/2] libata: pata_platform: Support polling-mode configuration.

[Alan Cox]

> +	 * 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");
> +	}
> +
> +	/*

Zero means no IRQ. Not < 0. We enforce that policy across the kernel and
Linus has been very clear about it.

Re: [PATCH 1/2] libata: Support PIO polling-only hosts.

[Alan Cox]

On Wed, 7 Nov 2007 17:10:52 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> By default ata_host_activate() expects a valid IRQ in order to
> successfully register the host. This patch enables a special case
> for registering polling-only hosts that either don't have IRQs
> or have buggy IRQ generation (either in terms of handling or
> sensing), which otherwise work fine.
> 
> Hosts that want to use polling mode can simply set ATA_FLAG_PIO_POLLING
> and pass in a NULL IRQ handler or invalid (< 0) IRQ.

NAK

Zero is "no IRQ", please use that for polling not "< 0"

> + *	A NULL @irq_handler or invalid IRQ skips the IRQ registration
> + *	and expects the host to have set polling mode on the port.
> + *

There is no need to llow NULL irq_handler as a different case here. Any
caller can provide (NULL, 0). If anything you might want to warn if they
provide IRQ 0, and a handler

Re: [PATCH 1/2] libata: Support PIO polling-only hosts.

[Paul Mundt]

On Wed, Nov 07, 2007 at 01:09:40PM +0000, Alan Cox wrote:
> On Wed, 7 Nov 2007 17:10:52 +0900
> Paul Mundt <lethal@linux-sh.org> wrote:
> > By default ata_host_activate() expects a valid IRQ in order to
> > successfully register the host. This patch enables a special case
> > for registering polling-only hosts that either don't have IRQs
> > or have buggy IRQ generation (either in terms of handling or
> > sensing), which otherwise work fine.
> > 
> > Hosts that want to use polling mode can simply set ATA_FLAG_PIO_POLLING
> > and pass in a NULL IRQ handler or invalid (< 0) IRQ.
> 
> NAK
> 
> Zero is "no IRQ", please use that for polling not "< 0"
> 
However, platform_get_irq() will happily return IRQ#0, and it's a valid
vector on plenty of machines. NO_IRQ is also < 0 on at least FR-V, ARM,
blackin, PA-RISC, some PowerPC, and even IDE.

We do have some devices that are physically on IRQ#0 that otherwise work
fine, they aren't ATA devices mind you, but to claim that IRQ#0 isn't a
valid vector is not in line with what hardware actually does, whether
it's a good idea or not. In our case the IRQ vector maps to an exception
offset, which we bump down to zero. We could force an off-by-1 there so
that the math that indexes IRQ#0 is bumped up one, but that entails
fixing up every one of our IRQ numbers for no obvious gain.

I don't really see any value in purposely crippling the range of
allowable vectors for these machines. Though I don't mind switching to a
NO_IRQ comparison instead of the < 0 case, so both can be handled.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 1/2] libata: Support PIO polling-only hosts.

[Mark Lord]

Paul Mundt wrote:
> On Wed, Nov 07, 2007 at 01:09:40PM +0000, Alan Cox wrote:
>> On Wed, 7 Nov 2007 17:10:52 +0900
>> Paul Mundt <lethal@linux-sh.org> wrote:
>>> By default ata_host_activate() expects a valid IRQ in order to
>>> successfully register the host. This patch enables a special case
>>> for registering polling-only hosts that either don't have IRQs
>>> or have buggy IRQ generation (either in terms of handling or
>>> sensing), which otherwise work fine.
>>>
>>> Hosts that want to use polling mode can simply set ATA_FLAG_PIO_POLLING
>>> and pass in a NULL IRQ handler or invalid (< 0) IRQ.
>> NAK
>>
>> Zero is "no IRQ", please use that for polling not "< 0"
>>
> However, platform_get_irq() will happily return IRQ#0, and it's a valid
> vector on plenty of machines. NO_IRQ is also < 0 on at least FR-V, ARM,
> blackin, PA-RISC, some PowerPC, and even IDE.

Too bad.  The Penultimate Penguin wants zero to continue to mean "no IRQ".

Dig into the archives for multiple threads on this exact topic.
The end result is that "0" means "no IRQ".  If your physical IRQ actually
is the number 0, then reencode it to some other value for this purpose.

Yes, a bit of pain, but that's how many parts of the kernel expect it,
and in the end it's no more overall hassle than doing it differently might
have been.

Cheers

Re: [PATCH 1/2] libata: Support PIO polling-only hosts.

[Paul Mundt]

On Wed, Nov 07, 2007 at 09:09:30AM -0500, Mark Lord wrote:
> Paul Mundt wrote:
> >On Wed, Nov 07, 2007 at 01:09:40PM +0000, Alan Cox wrote:
> >>On Wed, 7 Nov 2007 17:10:52 +0900
> >>Paul Mundt <lethal@linux-sh.org> wrote:
> >>>By default ata_host_activate() expects a valid IRQ in order to
> >>>successfully register the host. This patch enables a special case
> >>>for registering polling-only hosts that either don't have IRQs
> >>>or have buggy IRQ generation (either in terms of handling or
> >>>sensing), which otherwise work fine.
> >>>
> >>>Hosts that want to use polling mode can simply set ATA_FLAG_PIO_POLLING
> >>>and pass in a NULL IRQ handler or invalid (< 0) IRQ.
> >>NAK
> >>
> >>Zero is "no IRQ", please use that for polling not "< 0"
> >>
> >However, platform_get_irq() will happily return IRQ#0, and it's a valid
> >vector on plenty of machines. NO_IRQ is also < 0 on at least FR-V, ARM,
> >blackin, PA-RISC, some PowerPC, and even IDE.
> 
> Too bad.  The Penultimate Penguin wants zero to continue to mean "no IRQ".
> 
> Dig into the archives for multiple threads on this exact topic.
> The end result is that "0" means "no IRQ".  If your physical IRQ actually
> is the number 0, then reencode it to some other value for this purpose.
> 
I've read the threads, but this does little to do with the fact it's
still a perfectly valid vector, and I'm not about to force every IRQ
vector on my platform off-by-1 in order to satisfy a religious point of
view with zero reflection on what the hardware actually looks like.

So I'll change the check to IRQ#0 == invalid, but if that's to be
enforced kernel-wide, then all of the existing NO_IRQ cases should be
ripped out and set to 0. This way at least people are getting screwed
consistently, rather than just in particular subsystems.

> Yes, a bit of pain, but that's how many parts of the kernel expect it,

Just as many parts of the kernel make no such assumption.

> and in the end it's no more overall hassle than doing it differently might
> have been.
> 
Spoken like someone who doesn't have to contend with off-by-1 IRQ
vectors as a result of an entirely cosmetic change. It's certainly easier
to parrot a party line when you aren't being bitten by it.

So again, I'll make the change, but it's utter nonsense.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 1/2] libata: Support PIO polling-only hosts.

[Alan Cox]

> So I'll change the check to IRQ#0 == invalid, but if that's to be
> enforced kernel-wide, then all of the existing NO_IRQ cases should be
> ripped out and set to 0. This way at least people are getting screwed
> consistently, rather than just in particular subsystems.

Thats been gradually happening. Its now pretty clean except for powerpc
and odd bits of ARM stuff

Alan

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [PATCH 1/2] libata: Support PIO polling-only hosts.

[Alan Cox]

> > Zero is "no IRQ", please use that for polling not "< 0"
> > 
> However, platform_get_irq() will happily return IRQ#0, and it's a valid
> vector on plenty of machines. NO_IRQ is also < 0 on at least FR-V, ARM,
> blackin, PA-RISC, some PowerPC, and even IDE.

No it is not. The platform IRQ code is responsible for ensuring that 0 is
not a real IRQ and doing any neccessary remapping.

Large parts of the kernel assume that
	-	IRQ 0 is "no IRQ assigned" (serial, pci, ide etc )
	-	IRQ is *unsigned*

> We do have some devices that are physically on IRQ#0 that otherwise work
> fine, they aren't ATA devices mind you, but to claim that IRQ#0 isn't a
> valid vector is not in line with what hardware actually does, whether
> it's a good idea or not. In our case the IRQ vector maps to an exception
> offset, which we bump down to zero. We could force an off-by-1 there so
> that the math that indexes IRQ#0 is bumped up one, but that entails
> fixing up every one of our IRQ numbers for no obvious gain.
> 
> I don't really see any value in purposely crippling the range of
> allowable vectors for these machines. Though I don't mind switching to a
> NO_IRQ comparison instead of the < 0 case, so both can be handled.

NO_IRQ is an obsolete old-IDE hack.

http://www.uwsg.indiana.edu/hypermail/linux/kernel/0511.2/2197.html

http://www.uwsg.indiana.edu/hypermail/linux/kernel/0511.2/1789.html

Alan

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/