[PATCH] Xilinx: hwicap: cleanup polling timeout.

In order to avoid polling forever if an error occurs, this driver
includes a timeout counter.  However, in fast systems, this counter
wasn't high enough.  This patch fixes the bug and also makes the
buffer-based and fifo-based drivers return the same error condition on
a timeout (-EIO).

Signed-off-by: Stephen Neuendorffer <redacted>
---
 drivers/char/xilinx_hwicap/buffer_icap.c   |   10 ++++++++--
 drivers/char/xilinx_hwicap/fifo_icap.c     |    9 +++++++++
 drivers/char/xilinx_hwicap/xilinx_hwicap.h |    3 ---
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c b/drivers/char/xilinx_hwicap/buffer_icap.c
index aa7f796..0993883 100644
--- a/drivers/char/xilinx_hwicap/buffer_icap.c
+++ b/drivers/char/xilinx_hwicap/buffer_icap.c

@@ -35,6 +35,12 @@
 
 #include "buffer_icap.h"
 
+/* Number of times to poll the done register.  This has to be large
+   enough to allow an entire configuration to complete. If an entire
+   page (4kb) is configured at once, that could take up to 4k cycles
+   with a byte-wide icap interface. */
+#define XHI_MAX_RETRIES     5000
+
 /* Indicates how many bytes will fit in a buffer. (1 BRAM) */
 #define XHI_MAX_BUFFER_BYTES        2048
 #define XHI_MAX_BUFFER_INTS         (XHI_MAX_BUFFER_BYTES >> 2)

@@ -208,7 +214,7 @@ static int buffer_icap_device_read(struct hwicap_drvdata *drvdata,
 	while (buffer_icap_busy(base_address)) {
 		retries++;
 		if (retries > XHI_MAX_RETRIES)
-			return -EBUSY;
+			return -EIO;
 	}
 	return 0;
 

@@ -242,7 +248,7 @@ static int buffer_icap_device_write(struct hwicap_drvdata *drvdata,
 	while (buffer_icap_busy(base_address)) {
 		retries++;
 		if (retries > XHI_MAX_RETRIES)
-			return -EBUSY;
+			return -EIO;
 	}
 	return 0;
 

diff --git a/drivers/char/xilinx_hwicap/fifo_icap.c b/drivers/char/xilinx_hwicap/fifo_icap.c
index 776b505..d1cd928 100644
--- a/drivers/char/xilinx_hwicap/fifo_icap.c
+++ b/drivers/char/xilinx_hwicap/fifo_icap.c

@@ -35,6 +35,15 @@
 
 #include "fifo_icap.h"
 
+/* Number of times to poll the done register.  This has to be large
+ * enough to allow an entire configuration to complete.  If an entire
+ * page (4kb) is configured at once, that could take up to 4k cycles
+ * with a byte-wide icap interface.  In most cases, this driver is
+ * used with a much smaller fifo, but this should be sufficient in the
+ * worst case.
+ */
+#define XHI_MAX_RETRIES     5000
+
 /* Register offsets for the XHwIcap device. */
 #define XHI_GIER_OFFSET	0x1C  /* Device Global Interrupt Enable Reg */
 #define XHI_IPISR_OFFSET 0x20  /* Interrupt Status Register */

diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.h b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
index 1f9c8b0..fd4be31 100644
--- a/drivers/char/xilinx_hwicap/xilinx_hwicap.h
+++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.h

@@ -89,9 +89,6 @@ struct hwicap_driver_config {
 	void (*reset)(struct hwicap_drvdata *drvdata);
 };
 
-/* Number of times to poll the done regsiter */
-#define XHI_MAX_RETRIES     10
-
 /************ Constant Definitions *************/
 
 #define XHI_PAD_FRAMES              0x1

-- 
1.5.3.4-dirty

On Mon, May 5, 2008 at 12:20 PM, Stephen Neuendorffer
[off-list ref] wrote:

In order to avoid polling forever if an error occurs, this driver
 includes a timeout counter.  However, in fast systems, this counter
 wasn't high enough.  This patch fixes the bug and also makes the
 buffer-based and fifo-based drivers return the same error condition on
 a timeout (-EIO).

Would it be better to base the timeout on real time instead of a hard
iteration count?

Cheers,
g.

 Signed-off-by: Stephen Neuendorffer [off-list ref]
 ---
  drivers/char/xilinx_hwicap/buffer_icap.c   |   10 ++++++++--
  drivers/char/xilinx_hwicap/fifo_icap.c     |    9 +++++++++
  drivers/char/xilinx_hwicap/xilinx_hwicap.h |    3 ---
  3 files changed, 17 insertions(+), 5 deletions(-)

 diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c b/drivers/char/xilinx_hwicap/buffer_icap.c
 index aa7f796..0993883 100644
 --- a/drivers/char/xilinx_hwicap/buffer_icap.c
 +++ b/drivers/char/xilinx_hwicap/buffer_icap.c
 @@ -35,6 +35,12 @@

  #include "buffer_icap.h"

 +/* Number of times to poll the done register.  This has to be large
 +   enough to allow an entire configuration to complete. If an entire
 +   page (4kb) is configured at once, that could take up to 4k cycles
 +   with a byte-wide icap interface. */
 +#define XHI_MAX_RETRIES     5000
 +
  /* Indicates how many bytes will fit in a buffer. (1 BRAM) */
  #define XHI_MAX_BUFFER_BYTES        2048
  #define XHI_MAX_BUFFER_INTS         (XHI_MAX_BUFFER_BYTES >> 2)
 @@ -208,7 +214,7 @@ static int buffer_icap_device_read(struct hwicap_drvdata *drvdata,
        while (buffer_icap_busy(base_address)) {
                retries++;
                if (retries > XHI_MAX_RETRIES)
 -                       return -EBUSY;
 +                       return -EIO;
        }
        return 0;

 @@ -242,7 +248,7 @@ static int buffer_icap_device_write(struct hwicap_drvdata *drvdata,
        while (buffer_icap_busy(base_address)) {
                retries++;
                if (retries > XHI_MAX_RETRIES)
 -                       return -EBUSY;
 +                       return -EIO;

43 more quoted lines

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Not really... In this case 'faster' means 'faster ICAP core' not 'faster
processor':
This is really something that is bounded by the number of cycles.

Steve

-----Original Message-----
From: <redacted> [mailto:<redacted>] On Behalf Of

Grant Likely

Sent: Monday, May 05, 2008 1:01 PM
To: Stephen Neuendorffer
Cc: <redacted>
Subject: Re: [PATCH] Xilinx: hwicap: cleanup polling timeout.
=20
On Mon, May 5, 2008 at 12:20 PM, Stephen Neuendorffer
[off-list ref] wrote:

quoted

In order to avoid polling forever if an error occurs, this driver
 includes a timeout counter.  However, in fast systems, this counter
 wasn't high enough.  This patch fixes the bug and also makes the
 buffer-based and fifo-based drivers return the same error condition

on

quoted

 a timeout (-EIO).

=20
Would it be better to base the timeout on real time instead of a hard
iteration count?
=20
Cheers,
g.
=20

quoted

 Signed-off-by: Stephen Neuendorffer

[off-list ref]

quoted

 ---
  drivers/char/xilinx_hwicap/buffer_icap.c   |   10 ++++++++--
  drivers/char/xilinx_hwicap/fifo_icap.c     |    9 +++++++++
  drivers/char/xilinx_hwicap/xilinx_hwicap.h |    3 ---
  3 files changed, 17 insertions(+), 5 deletions(-)

 diff --git a/drivers/char/xilinx_hwicap/buffer_icap.c

b/drivers/char/xilinx_hwicap/buffer_icap.c

quoted

 index aa7f796..0993883 100644
 --- a/drivers/char/xilinx_hwicap/buffer_icap.c
 +++ b/drivers/char/xilinx_hwicap/buffer_icap.c
 @@ -35,6 +35,12 @@

  #include "buffer_icap.h"

 +/* Number of times to poll the done register.  This has to be

large

quoted

 +   enough to allow an entire configuration to complete. If an

entire

quoted

 +   page (4kb) is configured at once, that could take up to 4k

cycles

quoted

 +   with a byte-wide icap interface. */
 +#define XHI_MAX_RETRIES     5000
 +
  /* Indicates how many bytes will fit in a buffer. (1 BRAM) */
  #define XHI_MAX_BUFFER_BYTES        2048
  #define XHI_MAX_BUFFER_INTS         (XHI_MAX_BUFFER_BYTES >> 2)
 @@ -208,7 +214,7 @@ static int buffer_icap_device_read(struct

hwicap_drvdata *drvdata,

quoted

        while (buffer_icap_busy(base_address)) {
                retries++;
                if (retries > XHI_MAX_RETRIES)
 -                       return -EBUSY;
 +                       return -EIO;
        }
        return 0;

 @@ -242,7 +248,7 @@ static int buffer_icap_device_write(struct

hwicap_drvdata *drvdata,

quoted

        while (buffer_icap_busy(base_address)) {
                retries++;
                if (retries > XHI_MAX_RETRIES)
 -                       return -EBUSY;
 +                       return -EIO;
        }
        return 0;

 diff --git a/drivers/char/xilinx_hwicap/fifo_icap.c

b/drivers/char/xilinx_hwicap/fifo_icap.c

quoted

 index 776b505..d1cd928 100644
 --- a/drivers/char/xilinx_hwicap/fifo_icap.c
 +++ b/drivers/char/xilinx_hwicap/fifo_icap.c
 @@ -35,6 +35,15 @@

  #include "fifo_icap.h"

 +/* Number of times to poll the done register.  This has to be

large

quoted

 + * enough to allow an entire configuration to complete.  If an

entire

quoted

 + * page (4kb) is configured at once, that could take up to 4k

cycles

quoted

 + * with a byte-wide icap interface.  In most cases, this driver is
 + * used with a much smaller fifo, but this should be sufficient in

the

quoted

 + * worst case.
 + */
 +#define XHI_MAX_RETRIES     5000
 +
  /* Register offsets for the XHwIcap device. */
  #define XHI_GIER_OFFSET        0x1C  /* Device Global Interrupt

Enable Reg */

quoted

  #define XHI_IPISR_OFFSET 0x20  /* Interrupt Status Register */
 diff --git a/drivers/char/xilinx_hwicap/xilinx_hwicap.h

b/drivers/char/xilinx_hwicap/xilinx_hwicap.h

quoted

 index 1f9c8b0..fd4be31 100644
 --- a/drivers/char/xilinx_hwicap/xilinx_hwicap.h
 +++ b/drivers/char/xilinx_hwicap/xilinx_hwicap.h
 @@ -89,9 +89,6 @@ struct hwicap_driver_config {
        void (*reset)(struct hwicap_drvdata *drvdata);
  };

 -/* Number of times to poll the done regsiter */
 -#define XHI_MAX_RETRIES     10
 -
  /************ Constant Definitions *************/

  #define XHI_PAD_FRAMES              0x1
 --
 1.5.3.4-dirty

=20
=20
=20
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.