Hi everyone,

sorry for resurrecting this patch from the dead.

Ian: No need to test again, I believe something in your hardware may
have been fundamentally broken. I have no idea how to debug it, but
failing/aborting is better than hanging, so I think I'll merge this patch.

GatoLoko: This patch will probably change the hang into a really slow
abort. Still, aborting is better than hanging.


Am 15.10.2011 09:32 schrieb Stefan Tauner:
> On Fri, 14 Oct 2011 23:15:09 +0100
> Iain Paton <[email protected]> wrote:
>
>> Stefan Tauner wrote:
>>> AT49BV002A(N)'s ID is 0x07 afaics and that's equal to AT49F002(N)... hm.
>> tried 7 & 8, 8 for the T version according to the datasheet, but nothing to 
>> lose trying both
> does the log show any other result than 0xff/0x00 (for any chip, if you
> probe for all of them)? i think you have not posted a log for that one,
> have you?
>
>>> what did you try? does probing with the nicintel_spi code help?
>> didn't appear to, but I'm assuming that when you did it you left it stuck in 
>> the loop while you tried the nicintel code ?  Carl-Daniel's patch means it 
>> doesn't get stuck anymore, so I need to either revert that or try something 
>> else.
> no, i aborted after entering the loop. the enable method is executed at
> startup of the programmer... hm. maybe it gets reversed since then.
> carldani added some kind of roll-back mechanism where the code
> registers certain actions in init functions etc. that are reversed on
> shutdown. but i think ctrl+c just aborts anyway...
> after looking at the code... the comments in nicintel_spic.c explain
> that the roll-back thingy is not possible, but it does it manually in
> the shutdown function, but i am quite sure that this is not called at
> all when you abort while probing... so it should be ok. *shrug*
>
>>> so it is certainly not exactly the same problem i had; and i cant
>>> help you further right now, sorry.
>> np, with these being pci cards their usefulness is limited to me nowadays, 
>> in fact I think they've sat in a box since last time :)  so it's mostly 
>> curiosity on my part, no burning need for me to solve it.
> that was my motivation too... and that's not a very strong one ;)

Updated version of the timeout patch.

Abort controller accesses which take too long instead of hanging indefinitely.

Signed-off-by: Carl-Daniel Hailfinger <[email protected]>

Index: flashrom-bitbang_timeout/ogp_spi.c
===================================================================
--- flashrom-bitbang_timeout/ogp_spi.c  (Revision 1781)
+++ flashrom-bitbang_timeout/ogp_spi.c  (Arbeitskopie)
@@ -53,9 +53,13 @@
        {0},
 };
 
-static void ogp_request_spibus(void)
+static int ogp_request_spibus(void)
 {
+       /* FIXME: Do we have to check if any other SPI accesses are running
+        * right now? Can the request fail?
+        */
        pci_mmio_writel(1, ogp_spibar + ogp_reg_sel);
+       return 0;
 }
 
 static void ogp_release_spibus(void)
Index: flashrom-bitbang_timeout/bitbang_spi.c
===================================================================
--- flashrom-bitbang_timeout/bitbang_spi.c      (Revision 1781)
+++ flashrom-bitbang_timeout/bitbang_spi.c      (Arbeitskopie)
@@ -46,10 +46,11 @@
        return master->get_miso();
 }
 
-static void bitbang_spi_request_bus(const struct bitbang_spi_master * const 
master)
+static int bitbang_spi_request_bus(const struct bitbang_spi_master * const 
master)
 {
        if (master->request_bus)
-               master->request_bus();
+               return master->request_bus();
+       return 0;
 }
 
 static void bitbang_spi_release_bus(const struct bitbang_spi_master * const 
master)
@@ -136,14 +137,16 @@
                                    const unsigned char *writearr,
                                    unsigned char *readarr)
 {
-       int i;
+       int i, ret;
        const struct bitbang_spi_master *master = flash->pgm->spi.data;
 
        /* FIXME: Run bitbang_spi_request_bus here or in programmer init?
         * Requesting and releasing the SPI bus is handled in here to allow the
         * programmer to use its own SPI engine for native accesses.
         */
-       bitbang_spi_request_bus(master);
+       ret = bitbang_spi_request_bus(master);
+       if (ret)
+               return ret;
        bitbang_spi_set_cs(master, 0);
        for (i = 0; i < writecnt; i++)
                bitbang_spi_rw_byte(master, writearr[i]);
Index: flashrom-bitbang_timeout/nicintel_spi.c
===================================================================
--- flashrom-bitbang_timeout/nicintel_spi.c     (Revision 1781)
+++ flashrom-bitbang_timeout/nicintel_spi.c     (Arbeitskopie)
@@ -71,6 +71,9 @@
 // #define FL_BUSY     30
 // #define FL_ER       31
 
+/* 1000000 loops are roughly 1 s. */
+#define MAX_REQUEST_LOOPS      1000000
+
 uint8_t *nicintel_spibar;
 
 const struct dev_entry nics_intel_spi[] = {
@@ -83,16 +86,27 @@
        {0},
 };
 
-static void nicintel_request_spibus(void)
+static void nicintel_release_spibus(void);
+
+static int nicintel_request_spibus(void)
 {
        uint32_t tmp;
+       int i = 0;
 
        tmp = pci_mmio_readl(nicintel_spibar + FLA);
        tmp |= 1 << FL_REQ;
        pci_mmio_writel(tmp, nicintel_spibar + FLA);
 
        /* Wait until we are allowed to use the SPI bus. */
-       while (!(pci_mmio_readl(nicintel_spibar + FLA) & (1 << FL_GNT))) ;
+       while (!(pci_mmio_readl(nicintel_spibar + FLA) & (1 << FL_GNT))) {
+               if (i++ > MAX_REQUEST_LOOPS) {
+                       nicintel_release_spibus();
+                       msg_perr("%s: Timeout! Aborting.\n", __func__);
+                       return TIMEOUT_ERROR;
+               }
+       }
+
+       return 0;
 }
 
 static void nicintel_release_spibus(void)
Index: flashrom-bitbang_timeout/sb600spi.c
===================================================================
--- flashrom-bitbang_timeout/sb600spi.c (Revision 1781)
+++ flashrom-bitbang_timeout/sb600spi.c (Arbeitskopie)
@@ -159,12 +159,19 @@
        return ret;
 }
 
-static void execute_command(void)
+static int execute_command(void)
 {
+       int timeout = 1000000;
+
        mmio_writeb(mmio_readb(sb600_spibar + 2) | 1, sb600_spibar + 2);
 
-       while (mmio_readb(sb600_spibar + 2) & 1)
+       while ((mmio_readb(sb600_spibar + 2) & 1) && --timeout)
                ;
+       if (!timeout) {
+               msg_perr("SB600: execute_command timeout!\n");
+               return TIMEOUT_ERROR;
+       }
+       return 0;
 }
 
 static int sb600_spi_send_command(struct flashctx *flash, unsigned int 
writecnt,
@@ -177,6 +184,7 @@
        unsigned char cmd = *writearr++;
        unsigned int readoffby1;
        unsigned char readwrite;
+       int ret;
 
        writecnt--;
 
@@ -225,7 +233,9 @@
                return SPI_PROGRAMMER_ERROR;
 
        msg_pspew("Executing: \n");
-       execute_command();
+       ret = execute_command();
+       if (ret)
+               return ret;
 
        /*
         * After the command executed, we should find out the index of the
Index: flashrom-bitbang_timeout/mcp6x_spi.c
===================================================================
--- flashrom-bitbang_timeout/mcp6x_spi.c        (Revision 1781)
+++ flashrom-bitbang_timeout/mcp6x_spi.c        (Arbeitskopie)
@@ -40,22 +40,38 @@
 #define MCP6X_SPI_REQUEST      0
 #define MCP6X_SPI_GRANT                8
 
+/* 1000000 loops are roughly 1 s. */
+#define MAX_REQUEST_LOOPS      1000000
+
 void *mcp6x_spibar = NULL;
 
 /* Cached value of last GPIO state. */
 static uint8_t mcp_gpiostate;
 
-static void mcp6x_request_spibus(void)
+static void mcp6x_release_spibus(void);
+
+static int mcp6x_request_spibus(void)
 {
+       int i = 0;
+
        mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530);
        mcp_gpiostate |= 1 << MCP6X_SPI_REQUEST;
        mmio_writeb(mcp_gpiostate, mcp6x_spibar + 0x530);
 
        /* Wait until we are allowed to use the SPI bus. */
-       while (!(mmio_readw(mcp6x_spibar + 0x530) & (1 << MCP6X_SPI_GRANT))) ;
+       while (!(mmio_readw(mcp6x_spibar + 0x530) & (1 << MCP6X_SPI_GRANT))) {
+               if (i++ > MAX_REQUEST_LOOPS) {
+                       /* Update the cache. */
+                       mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530);
+                       mcp6x_release_spibus();
+                       msg_perr("%s: Timeout! Aborting.\n", __func__);
+                       return TIMEOUT_ERROR;
+               }
+       }
 
        /* Update the cache. */
        mcp_gpiostate = mmio_readb(mcp6x_spibar + 0x530);
+       return 0;
 }
 
 static void mcp6x_release_spibus(void)
Index: flashrom-bitbang_timeout/programmer.h
===================================================================
--- flashrom-bitbang_timeout/programmer.h       (Revision 1781)
+++ flashrom-bitbang_timeout/programmer.h       (Arbeitskopie)
@@ -157,7 +157,7 @@
        void (*set_sck) (int val);
        void (*set_mosi) (int val);
        int (*get_miso) (void);
-       void (*request_bus) (void);
+       int (*request_bus) (void);
        void (*release_bus) (void);
        /* Length of half a clock period in usecs. */
        unsigned int half_period;

-- 
http://www.hailfinger.org/


_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to