From 65ecf1b1c184e56dab4f168c89a2b4c23dfe5cb0 Mon Sep 17 00:00:00 2001 From: px4dev Date: Sun, 9 Sep 2012 00:04:43 -0700 Subject: [PATCH 1/2] Rework the 'eeprom erase' path so it's possible to erase an EEPROM that can't be mounted. Add some bus reset code to the EEPROM read path to maybe help with bus lockup. --- apps/systemcmds/eeprom/24xxxx_mtd.c | 18 ++++++++++++- apps/systemcmds/eeprom/eeprom.c | 39 ++++++++++++++++++----------- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/apps/systemcmds/eeprom/24xxxx_mtd.c b/apps/systemcmds/eeprom/24xxxx_mtd.c index b5144512bc..1b8c59f77e 100644 --- a/apps/systemcmds/eeprom/24xxxx_mtd.c +++ b/apps/systemcmds/eeprom/24xxxx_mtd.c @@ -138,6 +138,8 @@ struct at24c_dev_s perf_counter_t perf_reads; perf_counter_t perf_writes; + perf_counter_t perf_resets; + perf_counter_t perf_read_retries; }; /************************************************************************************ @@ -267,6 +269,7 @@ static ssize_t at24c_bread(FAR struct mtd_dev_s *dev, off_t startblock, for (;;) { + unsigned tries = 50; perf_begin(priv->perf_reads); ret = I2C_TRANSFER(priv->dev, &msgv[0], 2); @@ -274,9 +277,20 @@ static ssize_t at24c_bread(FAR struct mtd_dev_s *dev, off_t startblock, if (ret >= 0) break; - /* XXX probably want a bus reset in here and an eventual timeout */ fvdbg("read stall"); usleep(1000); + perf_count(priv->perf_read_retries); + + /* + * Kick the bus in case it's stuck. + */ + if (--tries == 0) + { + tries = 50; + up_i2creset(priv->dev); + perf_count(priv->perf_resets); + } + } startblock++; @@ -480,6 +494,8 @@ FAR struct mtd_dev_s *at24c_initialize(FAR struct i2c_dev_s *dev) priv->perf_reads = perf_alloc(PC_ELAPSED, "EEPROM read"); priv->perf_writes = perf_alloc(PC_ELAPSED, "EEPROM write"); + priv->perf_resets = perf_alloc(PC_COUNT, "EEPROM reset"); + priv->perf_read_retries = perf_alloc(PC_COUNT, "EEPROM read retries"); } /* Return the implementation-specific state structure as the MTD device */ diff --git a/apps/systemcmds/eeprom/eeprom.c b/apps/systemcmds/eeprom/eeprom.c index 6d1037a13d..a0b15f77b7 100644 --- a/apps/systemcmds/eeprom/eeprom.c +++ b/apps/systemcmds/eeprom/eeprom.c @@ -67,13 +67,16 @@ __EXPORT int eeprom_main(int argc, char *argv[]); +static void eeprom_attach(void); static void eeprom_start(void); static void eeprom_erase(void); static void eeprom_ioctl(unsigned operation); static void eeprom_save(const char *name); static void eeprom_load(const char *name); +static bool attached = false; static bool started = false; +static struct mtd_dev_s *eeprom_mtd; int eeprom_main(int argc, char *argv[]) { @@ -105,13 +108,8 @@ int eeprom_main(int argc, char *argv[]) static void -eeprom_start(void) +eeprom_attach(void) { - int ret; - - if (started) - errx(1, "EEPROM service already started"); - /* find the right I2C */ struct i2c_dev_s *i2c = up_i2cinitialize(PX4_I2C_BUS_ONBOARD); /* this resets the I2C bus, set correct bus speed again */ @@ -121,16 +119,27 @@ eeprom_start(void) errx(1, "failed to locate I2C bus"); /* start the MTD driver */ - struct mtd_dev_s *mtd = at24c_initialize(i2c); + eeprom_mtd = at24c_initialize(i2c); - if (mtd == NULL) + if (eeprom_mtd == NULL) errx(1, "failed to initialize EEPROM driver"); - /* driver is started, even if NXFFS fails to mount */ - started = true; + attached = true; +} + +static void +eeprom_start(void) +{ + int ret; + + if (started) + errx(1, "EEPROM already mounted"); + + if (!attached) + eeprom_attach(); /* start NXFFS */ - ret = nxffs_initialize(mtd); + ret = nxffs_initialize(eeprom_mtd); if (ret < 0) errx(1, "failed to initialize NXFFS - erase EEPROM to reformat"); @@ -141,7 +150,9 @@ eeprom_start(void) if (ret < 0) errx(1, "failed to mount /eeprom - erase EEPROM to reformat"); - errx(0, "mounted EEPROM at /eeprom"); + started = true; + warnx("mounted EEPROM at /eeprom"); + exit(0); } extern int at24c_nuke(void); @@ -149,8 +160,8 @@ extern int at24c_nuke(void); static void eeprom_erase(void) { - if (!started) - errx(1, "must be started first"); + if (!attached) + eeprom_attach(); if (at24c_nuke()) errx(1, "erase failed"); From a9c4fabda6cccb15912348ac5061827a6cb38304 Mon Sep 17 00:00:00 2001 From: px4dev Date: Sun, 9 Sep 2012 11:14:54 -0700 Subject: [PATCH 2/2] Change the EEPROM read/write timeout behavior so that we can get actual errors rather than just hanging forever. --- apps/systemcmds/eeprom/24xxxx_mtd.c | 39 ++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/apps/systemcmds/eeprom/24xxxx_mtd.c b/apps/systemcmds/eeprom/24xxxx_mtd.c index 1b8c59f77e..c83362ca8b 100644 --- a/apps/systemcmds/eeprom/24xxxx_mtd.c +++ b/apps/systemcmds/eeprom/24xxxx_mtd.c @@ -119,6 +119,13 @@ # define CONFIG_AT24XX_MTD_BLOCKSIZE AT24XX_PAGESIZE #endif +/* The AT24 does not respond on the bus during write cycles, so we depend on a long + * timeout to detect problems. The max program time is typically ~5ms. + */ +#ifndef CONFIG_AT24XX_WRITE_TIMEOUT_MS +# define CONFIG_AT24XX_WRITE_TIMEOUT_MS 20 +#endif + /************************************************************************************ * Private Types ************************************************************************************/ @@ -140,6 +147,8 @@ struct at24c_dev_s perf_counter_t perf_writes; perf_counter_t perf_resets; perf_counter_t perf_read_retries; + perf_counter_t perf_read_errors; + perf_counter_t perf_write_errors; }; /************************************************************************************ @@ -262,6 +271,7 @@ static ssize_t at24c_bread(FAR struct mtd_dev_s *dev, off_t startblock, while (blocksleft-- > 0) { uint16_t offset = startblock * priv->pagesize; + unsigned tries = CONFIG_AT24XX_WRITE_TIMEOUT_MS; addr[1] = offset & 0xff; addr[0] = (offset >> 8) & 0xff; @@ -269,7 +279,6 @@ static ssize_t at24c_bread(FAR struct mtd_dev_s *dev, off_t startblock, for (;;) { - unsigned tries = 50; perf_begin(priv->perf_reads); ret = I2C_TRANSFER(priv->dev, &msgv[0], 2); @@ -279,18 +288,19 @@ static ssize_t at24c_bread(FAR struct mtd_dev_s *dev, off_t startblock, fvdbg("read stall"); usleep(1000); - perf_count(priv->perf_read_retries); - /* - * Kick the bus in case it's stuck. + /* We should normally only be here on the first read after + * a write. + * + * XXX maybe do special first-read handling with optional + * bus reset as well? */ + perf_count(priv->perf_read_retries); if (--tries == 0) { - tries = 50; - up_i2creset(priv->dev); - perf_count(priv->perf_resets); + perf_count(priv->perf_read_errors); + return ERROR; } - } startblock++; @@ -350,6 +360,7 @@ static ssize_t at24c_bwrite(FAR struct mtd_dev_s *dev, off_t startblock, size_t while (blocksleft-- > 0) { uint16_t offset = startblock * priv->pagesize; + unsigned tries = CONFIG_AT24XX_WRITE_TIMEOUT_MS; buf[1] = offset & 0xff; buf[0] = (offset >> 8) & 0xff; @@ -365,9 +376,17 @@ static ssize_t at24c_bwrite(FAR struct mtd_dev_s *dev, off_t startblock, size_t if (ret >= 0) break; - /* XXX probably want a bus reset in here and an eventual timeout */ fvdbg("write stall"); usleep(1000); + + /* We expect to see a number of retries per write cycle as we + * poll for write completion. + */ + if (--tries == 0) + { + perf_count(priv->perf_write_errors); + return ERROR; + } } startblock++; @@ -496,6 +515,8 @@ FAR struct mtd_dev_s *at24c_initialize(FAR struct i2c_dev_s *dev) priv->perf_writes = perf_alloc(PC_ELAPSED, "EEPROM write"); priv->perf_resets = perf_alloc(PC_COUNT, "EEPROM reset"); priv->perf_read_retries = perf_alloc(PC_COUNT, "EEPROM read retries"); + priv->perf_read_errors = perf_alloc(PC_COUNT, "EEPROM read errors"); + priv->perf_write_errors = perf_alloc(PC_COUNT, "EEPROM write errors"); } /* Return the implementation-specific state structure as the MTD device */