[PATCH 0/6] net/e1000: more cleanups and ways to access i210 storage

[PATCH 0/6] net/e1000: more cleanups and ways to access i210 storage

[Uwe Kleine-König]

Hello,

these are a few patches that cumulated here related to the i210 network
controller.

These are nearly good enough to populate the attached flash with a valid
firmware (which you still need to have yourself) on all machines I have
access to.

There is still one (to me) known problem: When the flash doesn't contain
a valid image, accessing the eeprom (which is a simulated device on i210
that is backed up by shadow RAM and the two first pages of flash) gives
the contents of iNVM instead of the expected default eeprom. That
results in a few wrong turns in the code but they are not grave enough
to stop being able to write a valid firmware image.

A bit that is still missing is that a customer machine uses an flash
that is write protected at power on. An i210 with an programmed flash is
clever enough to remove the write protection before writing to it, but
that doesn't help when the flash is not programmed (tin opener inside).
I have a prototype that can remove the write protection with a
cryptic command but that's not ready for prime time yet.

Best regards
Uwe

Uwe Kleine-König (6):
  net/e1000: fix coding style at a few locations
  net/e1000: don't use the eeprom word size as timeout to get a
    semaphore
  net/e1000: reorder functions
  net/e1000: provide access to iNVM even if a flash is present
  net/e1000: fix size of invm device
  net/e1000: provide device for accessing emulated eeprom

 drivers/net/e1000/e1000.h  |   8 +-
 drivers/net/e1000/eeprom.c | 324 ++++++++++++++++++++++++++-------------------
 drivers/net/e1000/main.c   |   6 +-
 3 files changed, 196 insertions(+), 142 deletions(-)

-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/6] net/e1000: fix coding style at a few locations

[Uwe Kleine-König]

While working on the e1000 driver I noticed a few coding style misdeeds.
These are fixed here.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/eeprom.c | 15 ++++++++++++---
 drivers/net/e1000/main.c   |  4 ++--
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 2a71fb1b159c..150e4147c469 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -366,7 +366,8 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 	case e1000_82543:
 	case e1000_82544:
 		e1000_eeprom_uses_microwire(eeprom, 0);
-	break;
+		break;
+
 	case e1000_82540:
 	case e1000_82545:
 	case e1000_82545_rev_3:
@@ -374,6 +375,7 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 	case e1000_82546_rev_3:
 		e1000_eeprom_uses_microwire(eeprom, eecd);
 		break;
+
 	case e1000_82541:
 	case e1000_82541_rev_2:
 	case e1000_82547:
@@ -383,10 +385,12 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 		else
 			e1000_eeprom_uses_microwire(eeprom, eecd);
 		break;
+
 	case e1000_82571:
 	case e1000_82572:
 		e1000_eeprom_uses_spi(eeprom, eecd);
 		break;
+
 	case e1000_82573:
 	case e1000_82574:
 		if (e1000_is_onboard_nvm_eeprom(hw)) {
@@ -396,16 +400,20 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 			eeprom->type = e1000_eeprom_flash;
 			eeprom->word_size = 2048;
 
-		/* Ensure that the Autonomous FLASH update bit is cleared due to
-		 * Flash update issue on parts which use a FLASH for NVM. */
+			/*
+			 * Ensure that the Autonomous FLASH update bit is cleared due to
+			 * Flash update issue on parts which use a FLASH for NVM.
+			 */
 			eecd &= ~E1000_EECD_AUPDEN;
 			e1000_write_reg(hw, E1000_EECD, eecd);
 		}
 		break;
+
 	case e1000_80003es2lan:
 		eeprom->type = e1000_eeprom_spi;
 		eeprom->read = e1000_read_eeprom_eerd;
 		break;
+
 	case e1000_igb:
 		if (eecd & E1000_EECD_I210_FLASH_DETECTED) {
 			uint32_t fla;
@@ -431,6 +439,7 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 
 		eeprom->read = e1000_read_eeprom_eerd;
 		break;
+
 	default:
 		break;
 	}
diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index 00d18adff111..6491623104c3 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -142,8 +142,8 @@ static int32_t e1000_set_phy_mode(struct e1000_hw *hw)
  ***************************************************************************/
 static int32_t e1000_get_software_semaphore(struct e1000_hw *hw)
 {
-	 int32_t timeout = hw->eeprom.word_size + 1;
-	 uint32_t swsm;
+	int32_t timeout = hw->eeprom.word_size + 1;
+	uint32_t swsm;
 
 	DEBUGFUNC();
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/6] net/e1000: don't use the eeprom word size as timeout to get a semaphore

[Uwe Kleine-König]

Nothing in the documentation suggests that the time to get a semaphore
depends on the size of the (optional) eeprom and I doubt there is a
relation. Given that 2048 is the maximal value for word_size unless a
flash is used to emulate the eeprom, pick 2049 as alternative arbitrary
value that probably doesn't result in regressions.

This drops the only user of ->word_size outside of eeprom.c which allows
easier restructurings there in the future.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index 6491623104c3..01c9f39a25f4 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -142,7 +142,7 @@ static int32_t e1000_set_phy_mode(struct e1000_hw *hw)
  ***************************************************************************/
 static int32_t e1000_get_software_semaphore(struct e1000_hw *hw)
 {
-	int32_t timeout = hw->eeprom.word_size + 1;
+	int32_t timeout = 2049;
 	uint32_t swsm;
 
 	DEBUGFUNC();
@@ -214,7 +214,7 @@ static int32_t e1000_get_hw_eeprom_semaphore(struct e1000_hw *hw)
 	}
 
 	/* Get the FW semaphore. */
-	timeout = hw->eeprom.word_size + 1;
+	timeout = 2049;
 	while (timeout) {
 		swsm = e1000_read_reg(hw, E1000_SWSM);
 		swsm |= E1000_SWSM_SWESMBI;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/6] net/e1000: reorder functions

[Uwe Kleine-König]

Bring functions in a more natural order which allows to drop a few
forward declarations.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/eeprom.c | 137 ++++++++++++++++++++++-----------------------
 1 file changed, 67 insertions(+), 70 deletions(-)

diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 150e4147c469..9948a8c95275 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -6,18 +6,14 @@
 
 #include "e1000.h"
 
-static void e1000_release_eeprom_spi(struct e1000_hw *hw);
 static int32_t e1000_read_eeprom_spi(struct e1000_hw *hw, uint16_t offset,
 				     uint16_t words, uint16_t *data);
-static void e1000_release_eeprom_microwire(struct e1000_hw *hw);
 static int32_t e1000_read_eeprom_microwire(struct e1000_hw *hw, uint16_t offset,
 					   uint16_t words, uint16_t *data);
 
 static int32_t e1000_read_eeprom_eerd(struct e1000_hw *hw, uint16_t offset,
 				      uint16_t words, uint16_t *data);
 static int32_t e1000_spi_eeprom_ready(struct e1000_hw *hw);
-static void e1000_release_eeprom(struct e1000_hw *hw);
-static int32_t e1000_acquire_eeprom_flash(struct e1000_hw *hw);
 static void e1000_release_eeprom_flash(struct e1000_hw *hw);
 
 
@@ -252,6 +248,19 @@ e1000_acquire_eeprom_spi_microwire_prologue(struct e1000_hw *hw)
 	return E1000_SUCCESS;
 }
 
+static void
+e1000_release_eeprom_spi_microwire_epilogue(struct e1000_hw *hw)
+{
+	uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
+
+	/* Stop requesting EEPROM access */
+	if (hw->mac_type > e1000_82544) {
+		eecd &= ~E1000_EECD_REQ;
+		e1000_write_reg(hw, E1000_EECD, eecd);
+	}
+}
+
+
 static int32_t e1000_acquire_eeprom_spi(struct e1000_hw *hw)
 {
 	int32_t ret;
@@ -271,6 +280,19 @@ static int32_t e1000_acquire_eeprom_spi(struct e1000_hw *hw)
 	return E1000_SUCCESS;
 }
 
+static void e1000_release_eeprom_spi(struct e1000_hw *hw)
+{
+	uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
+
+	eecd |= E1000_EECD_CS;  /* Pull CS high */
+	eecd &= ~E1000_EECD_SK; /* Lower SCK */
+
+	e1000_write_reg(hw, E1000_EECD, eecd);
+	udelay(hw->eeprom.delay_usec);
+
+	e1000_release_eeprom_spi_microwire_epilogue(hw);
+}
+
 static int32_t e1000_acquire_eeprom_microwire(struct e1000_hw *hw)
 {
 	int ret;
@@ -292,11 +314,46 @@ static int32_t e1000_acquire_eeprom_microwire(struct e1000_hw *hw)
 	return E1000_SUCCESS;
 }
 
+static void e1000_release_eeprom_microwire(struct e1000_hw *hw)
+{
+	uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
+
+	/* cleanup eeprom */
+
+	/* CS on Microwire is active-high */
+	eecd &= ~(E1000_EECD_CS | E1000_EECD_DI);
+
+	e1000_write_reg(hw, E1000_EECD, eecd);
+
+	/* Rising edge of clock */
+	eecd |= E1000_EECD_SK;
+	e1000_write_reg(hw, E1000_EECD, eecd);
+	e1000_write_flush(hw);
+	udelay(hw->eeprom.delay_usec);
+
+	/* Falling edge of clock */
+	eecd &= ~E1000_EECD_SK;
+	e1000_write_reg(hw, E1000_EECD, eecd);
+	e1000_write_flush(hw);
+	udelay(hw->eeprom.delay_usec);
+
+
+	e1000_release_eeprom_spi_microwire_epilogue(hw);
+}
+
 static int32_t e1000_acquire_eeprom_flash(struct e1000_hw *hw)
 {
 	return e1000_swfw_sync_acquire(hw, E1000_SWFW_EEP_SM);
 }
 
+static void e1000_release_eeprom_flash(struct e1000_hw *hw)
+{
+	if (e1000_swfw_sync_release(hw, E1000_SWFW_EEP_SM) < 0)
+		dev_warn(hw->dev,
+			 "Timeout while releasing SWFW_SYNC bits (0x%08x)\n",
+			 E1000_SWFW_EEP_SM);
+}
+
 static int32_t e1000_acquire_eeprom(struct e1000_hw *hw)
 {
 	if (hw->eeprom.acquire)
@@ -305,6 +362,12 @@ static int32_t e1000_acquire_eeprom(struct e1000_hw *hw)
 		return E1000_SUCCESS;
 }
 
+static void e1000_release_eeprom(struct e1000_hw *hw)
+{
+	if (hw->eeprom.release)
+		hw->eeprom.release(hw);
+}
+
 static void e1000_eeprom_uses_spi(struct e1000_eeprom_info *eeprom,
 				  uint32_t eecd)
 {
@@ -600,72 +663,6 @@ static int32_t e1000_read_eeprom_microwire(struct e1000_hw *hw,
 	return E1000_SUCCESS;
 }
 
-static void
-e1000_release_eeprom_spi_microwire_epilogue(struct e1000_hw *hw)
-{
-	uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
-
-	/* Stop requesting EEPROM access */
-	if (hw->mac_type > e1000_82544) {
-		eecd &= ~E1000_EECD_REQ;
-		e1000_write_reg(hw, E1000_EECD, eecd);
-	}
-}
-
-static void e1000_release_eeprom_microwire(struct e1000_hw *hw)
-{
-	uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
-
-	/* cleanup eeprom */
-
-	/* CS on Microwire is active-high */
-	eecd &= ~(E1000_EECD_CS | E1000_EECD_DI);
-
-	e1000_write_reg(hw, E1000_EECD, eecd);
-
-	/* Rising edge of clock */
-	eecd |= E1000_EECD_SK;
-	e1000_write_reg(hw, E1000_EECD, eecd);
-	e1000_write_flush(hw);
-	udelay(hw->eeprom.delay_usec);
-
-	/* Falling edge of clock */
-	eecd &= ~E1000_EECD_SK;
-	e1000_write_reg(hw, E1000_EECD, eecd);
-	e1000_write_flush(hw);
-	udelay(hw->eeprom.delay_usec);
-
-
-	e1000_release_eeprom_spi_microwire_epilogue(hw);
-}
-
-static void e1000_release_eeprom_spi(struct e1000_hw *hw)
-{
-	uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
-
-	eecd |= E1000_EECD_CS;  /* Pull CS high */
-	eecd &= ~E1000_EECD_SK; /* Lower SCK */
-
-	e1000_write_reg(hw, E1000_EECD, eecd);
-	udelay(hw->eeprom.delay_usec);
-
-	e1000_release_eeprom_spi_microwire_epilogue(hw);
-}
-
-static void e1000_release_eeprom_flash(struct e1000_hw *hw)
-{
-	if (e1000_swfw_sync_release(hw, E1000_SWFW_EEP_SM) < 0)
-		dev_warn(hw->dev,
-			 "Timeout while releasing SWFW_SYNC bits (0x%08x)\n",
-			 E1000_SWFW_EEP_SM);
-}
-
-static void e1000_release_eeprom(struct e1000_hw *hw)
-{
-	if (hw->eeprom.release)
-		hw->eeprom.release(hw);
-}
-
 /******************************************************************************
  * Reads a 16 bit word from the EEPROM.
  *
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 4/6] net/e1000: provide access to iNVM even if a flash is present

[Uwe Kleine-König]

An i210 (aka e1000_igb) supports two different non-volatile storages for
configuration. There is a built-in one-time programmable storage called
iNVM and an optional external SPI-Flash. If a flash is populated and
contains a valid configuration image the iNVM is not used (if I
understood the documentation correctly). Still the iNVM can be useful if
the flash is not configured. Also the iNVM contains manufacturing
identification information.

So it makes sense to provide the invm device even if a flash is present.

This patch also cleans up some confusion that suggests that invm is a
way to access the (simulated) eeprom structures and drops some unused
enum values.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
If you look at the diff ignoring whitespace changes (git diff -b) the patch
looks much prettier.

 drivers/net/e1000/e1000.h  |   6 +-
 drivers/net/e1000/eeprom.c | 133 ++++++++++++++++++++++++---------------------
 2 files changed, 72 insertions(+), 67 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index f2da08b4d5c1..ac68e28276c1 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -65,13 +65,9 @@ typedef enum {
 } e1000_media_type;
 
 typedef enum {
-	e1000_eeprom_uninitialized = 0,
 	e1000_eeprom_spi,
 	e1000_eeprom_microwire,
-	e1000_eeprom_flash,
-	e1000_eeprom_ich8,
-	e1000_eeprom_none, /* No NVM support */
-	e1000_eeprom_invm,
+	e1000_eeprom_flash, /* access via EERD */
 	e1000_num_eeprom_types
 } e1000_eeprom_type;
 
diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 9948a8c95275..e5d1c5921f5e 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -493,13 +493,11 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 					 "limiting access to first 4KB\n");
 			}
 
-			eeprom->type = e1000_eeprom_flash;
 			eeprom->acquire = e1000_acquire_eeprom_flash;
 			eeprom->release = e1000_release_eeprom_flash;
-		} else {
-			eeprom->type = e1000_eeprom_invm;
 		}
 
+		eeprom->type = e1000_eeprom_flash;
 		eeprom->read = e1000_read_eeprom_eerd;
 		break;
 
@@ -507,8 +505,7 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 		break;
 	}
 
-	if (eeprom->type == e1000_eeprom_spi ||
-	    eeprom->type == e1000_eeprom_invm) {
+	if (eeprom->type == e1000_eeprom_spi) {
 		/* eeprom_size will be an enum [0..8] that maps
 		 * to eeprom sizes 128B to
 		 * 32KB (incremented by powers of 2).
@@ -1385,75 +1382,87 @@ fail:
 	return ret;
 }
 
-int e1000_register_eeprom(struct e1000_hw *hw)
+int e1000_register_invm(struct e1000_hw *hw)
 {
-	int ret = E1000_SUCCESS;
+	int ret;
 	u16 word;
 	struct param_d *p;
 
-	struct e1000_eeprom_info *eeprom = &hw->eeprom;
+	ret = e1000_read_eeprom(hw, 0x0a, 1, &word);
+	if (ret < 0)
+		return ret;
 
-	switch (eeprom->type) {
-	case e1000_eeprom_invm:
-		ret = e1000_read_eeprom(hw, 0x0A, 1, &word);
-		if (ret < 0)
-			return ret;
+	if (word & (1 << 15))
+		dev_warn(hw->dev, "iNVM lockout mechanism is active\n");
 
-		if (word & (1 << 15))
-			dev_warn(hw->dev, "iNVM lockout mechanism is active\n");
+	hw->invm.cdev.dev = hw->dev;
+	hw->invm.cdev.ops = &e1000_invm_ops;
+	hw->invm.cdev.priv = hw;
+	hw->invm.cdev.name = xasprintf("e1000-invm%d", hw->dev->id);
+	hw->invm.cdev.size = 32 * E1000_INVM_DATA_MAX_N;
 
-		hw->invm.cdev.dev = hw->dev;
-		hw->invm.cdev.ops = &e1000_invm_ops;
-		hw->invm.cdev.priv = hw;
-		hw->invm.cdev.name = xasprintf("e1000-invm%d", hw->dev->id);
-		hw->invm.cdev.size = 32 * E1000_INVM_DATA_MAX_N;
+	ret = devfs_create(&hw->invm.cdev);
+	if (ret < 0)
+		return ret;
 
-		ret = devfs_create(&hw->invm.cdev);
-		if (ret < 0)
-			break;
+	strcpy(hw->invm.dev.name, "invm");
+	hw->invm.dev.parent = hw->dev;
+	ret = register_device(&hw->invm.dev);
+	if (ret < 0) {
+		devfs_remove(&hw->invm.cdev);
+		return ret;
+	}
 
-		strcpy(hw->invm.dev.name, "invm");
-		hw->invm.dev.parent = hw->dev;
-		ret = register_device(&hw->invm.dev);
-		if (ret < 0) {
-			devfs_remove(&hw->invm.cdev);
-			break;
-		}
+	p = dev_add_param_int(&hw->invm.dev, "lock", e1000_invm_set_lock,
+			      NULL, &hw->invm.line, "%u", hw);
+	if (IS_ERR(p)) {
+		unregister_device(&hw->invm.dev);
+		devfs_remove(&hw->invm.cdev);
+		ret = PTR_ERR(p);
+	}
 
-		p = dev_add_param_int(&hw->invm.dev, "lock", e1000_invm_set_lock,
-				      NULL, &hw->invm.line, "%u", hw);
-		if (IS_ERR(p)) {
-			unregister_device(&hw->invm.dev);
-			devfs_remove(&hw->invm.cdev);
-			break;
+	return ret;
+}
+
+/*
+ * This function has a wrong name for historic reasons, it doesn't add an
+ * eeprom, but the flash (if available) that is used to simulate the eeprom.
+ * Also a device that represents the invm is registered here (if available).
+ */
+int e1000_register_eeprom(struct e1000_hw *hw)
+{
+	int ret = E1000_SUCCESS;
+
+	struct e1000_eeprom_info *eeprom = &hw->eeprom;
+
+	if (hw->mac_type == e1000_igb) {
+		uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
+
+		if (eecd & E1000_EECD_I210_FLASH_DETECTED) {
+			hw->mtd.parent = hw->dev;
+			hw->mtd.read = e1000_mtd_read;
+			hw->mtd.write = e1000_mtd_write;
+			hw->mtd.erase = e1000_mtd_erase;
+			hw->mtd.size = eeprom->word_size * 2;
+			hw->mtd.writesize = 1;
+			hw->mtd.subpage_sft = 0;
+
+			hw->mtd.eraseregions = xzalloc(sizeof(struct mtd_erase_region_info));
+			hw->mtd.erasesize = SZ_4K;
+			hw->mtd.eraseregions[0].erasesize = SZ_4K;
+			hw->mtd.eraseregions[0].numblocks = hw->mtd.size / SZ_4K;
+			hw->mtd.numeraseregions = 1;
+
+			hw->mtd.flags = MTD_CAP_NORFLASH;
+			hw->mtd.type = MTD_NORFLASH;
+
+			ret = add_mtd_device(&hw->mtd, "e1000-nor",
+					     DEVICE_ID_DYNAMIC);
+			if (ret)
+				return ret;
 		}
-		break;
-	case e1000_eeprom_flash:
-		if (hw->mac_type != e1000_igb)
-			break;
 
-		hw->mtd.parent = hw->dev;
-		hw->mtd.read = e1000_mtd_read;
-		hw->mtd.write = e1000_mtd_write;
-		hw->mtd.erase = e1000_mtd_erase;
-		hw->mtd.size = eeprom->word_size * 2;
-		hw->mtd.writesize = 1;
-		hw->mtd.subpage_sft = 0;
-
-		hw->mtd.eraseregions = xzalloc(sizeof(struct mtd_erase_region_info));
-		hw->mtd.erasesize = SZ_4K;
-		hw->mtd.eraseregions[0].erasesize = SZ_4K;
-		hw->mtd.eraseregions[0].numblocks = hw->mtd.size / SZ_4K;
-		hw->mtd.numeraseregions = 1;
-
-		hw->mtd.flags = MTD_CAP_NORFLASH;
-		hw->mtd.type = MTD_NORFLASH;
-
-		ret = add_mtd_device(&hw->mtd, "e1000-nor",
-				     DEVICE_ID_DYNAMIC);
-		break;
-	default:
-		break;
+		ret = e1000_register_invm(hw);
 	}
 
 	return ret;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 5/6] net/e1000: fix size of invm device

[Uwe Kleine-König]

The iNVM storage is 2 Kib containing 64 32bit words (0-63).

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/eeprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index e5d1c5921f5e..5ee545d8fd5d 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -1399,7 +1399,7 @@ int e1000_register_invm(struct e1000_hw *hw)
 	hw->invm.cdev.ops = &e1000_invm_ops;
 	hw->invm.cdev.priv = hw;
 	hw->invm.cdev.name = xasprintf("e1000-invm%d", hw->dev->id);
-	hw->invm.cdev.size = 32 * E1000_INVM_DATA_MAX_N;
+	hw->invm.cdev.size = 4 * (E1000_INVM_DATA_MAX_N + 1);
 
 	ret = devfs_create(&hw->invm.cdev);
 	if (ret < 0)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 6/6] net/e1000: provide device for accessing emulated eeprom

[Uwe Kleine-König]

This device uses e1000_read_eeprom to provide access to the emulated
eeprom on e1000-igb. Only reading is implemented for now.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/e1000.h  |  2 ++
 drivers/net/e1000/eeprom.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index ac68e28276c1..1e2c5d7bc5e9 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -2161,6 +2161,8 @@ struct e1000_hw {
 		int line;
 	} invm;
 
+	struct cdev eepromcdev;
+
 	struct mtd_info mtd;
 
 	uint32_t phy_id;
diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 5ee545d8fd5d..e5f8c9e271e0 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -1298,6 +1298,31 @@ static struct file_operations e1000_invm_ops = {
 	.lseek	= dev_lseek_default,
 };
 
+static ssize_t e1000_eeprom_cdev_read(struct cdev *cdev, void *buf,
+				      size_t count, loff_t offset, unsigned long flags)
+{
+	struct e1000_hw *hw = container_of(cdev, struct e1000_hw, eepromcdev);
+	int32_t ret;
+
+	/*
+	 * The eeprom interface works on 16 bit words which gives a nice excuse
+	 * for being lazy and not implementing unaligned reads.
+	 */
+	if (offset & 1 || count == 1)
+		return -EIO;
+
+	ret = e1000_read_eeprom(hw, offset / 2, count / 2, buf);
+	if (ret)
+		return -EIO;
+	else
+		return (count / 2) * 2;
+};
+
+static struct file_operations e1000_eeprom_ops = {
+	.read = e1000_eeprom_cdev_read,
+	.lseek = dev_lseek_default,
+};
+
 static int e1000_mtd_read_or_write(bool read,
 				   struct mtd_info *mtd, loff_t off, size_t len,
 				   size_t *retlen, u_char *buf)
@@ -1438,6 +1463,15 @@ int e1000_register_eeprom(struct e1000_hw *hw)
 	if (hw->mac_type == e1000_igb) {
 		uint32_t eecd = e1000_read_reg(hw, E1000_EECD);
 
+		hw->eepromcdev.dev = hw->dev;
+		hw->eepromcdev.ops = &e1000_eeprom_ops;
+		hw->eepromcdev.name = xasprintf("e1000-eeprom%d", hw->dev->id);
+		hw->eepromcdev.size = 0x1000;
+
+		ret = devfs_create(&hw->eepromcdev);
+		if (ret < 0)
+			return ret;
+
 		if (eecd & E1000_EECD_I210_FLASH_DETECTED) {
 			hw->mtd.parent = hw->dev;
 			hw->mtd.read = e1000_mtd_read;
@@ -1458,11 +1492,18 @@ int e1000_register_eeprom(struct e1000_hw *hw)
 
 			ret = add_mtd_device(&hw->mtd, "e1000-nor",
 					     DEVICE_ID_DYNAMIC);
-			if (ret)
+			if (ret) {
+				devfs_remove(&hw->eepromcdev);
 				return ret;
+			}
 		}
 
 		ret = e1000_register_invm(hw);
+		if (ret < 0) {
+			if (eecd & E1000_EECD_I210_FLASH_DETECTED)
+				del_mtd_device(&hw->mtd);
+			devfs_remove(&hw->eepromcdev);
+		}
 	}
 
 	return ret;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 7/6] net/e1000: don't fail to bind on uninitialized flash

[Uwe Kleine-König]

When the flash doesn't hold a valid firmware image there are still
some things (like accessing the flash) possible. Don't return an error
code in this case but return 0 instead. This way the driver is
regularily bound and just doesn't provide a network interface.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index 01c9f39a25f4..bb6ab4eb0360 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -3598,7 +3598,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	if (e1000_validate_eeprom_checksum(hw))
-		return -EINVAL;
+		return 0;
 
 	e1000_get_ethaddr(edev, edev->ethaddr);
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 8/6] net/e1000: log flash/invm status at probe time

[Uwe Kleine-König]

Provide some info about flash/eeprom state at boot up

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/eeprom.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index e5f8c9e271e0..cb92deee3b44 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -1472,6 +1472,24 @@ int e1000_register_eeprom(struct e1000_hw *hw)
 		if (ret < 0)
 			return ret;
 
+		if (eecd & E1000_EECD_AUTO_RD) {
+			if (eecd & E1000_EECD_EE_PRES) {
+				if (eecd & E1000_EECD_FLASH_IN_USE)
+					dev_info(hw->dev, "Hardware programmed from flash\n");
+				else
+					dev_info(hw->dev, "Hardware programmed from iNVM\n");
+			} else {
+				dev_warn(hw->dev, "Shadow RAM invalid\n");
+			}
+		} else {
+			/*
+			 * I never saw this case in practise and I'm unsure how
+			 * to handle that. Maybe just wait until the hardware is
+			 * up enough that this bit is set?
+			 */
+			dev_err(hw->dev, "Flash Auto-Read not done\n");
+		}
+
 		if (eecd & E1000_EECD_I210_FLASH_DETECTED) {
 			hw->mtd.parent = hw->dev;
 			hw->mtd.read = e1000_mtd_read;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 9/6] net/e1000: don't access the (simulated) eeprom when it is invalid

[Uwe Kleine-König]

The shadow RAM that is used to serve read requests from the eeprom
interface isn't valid in all cases. Catch these by returning an error in
the eeprom read function and make eeprom validation dependant on
working access.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/e1000.h  |  3 +++
 drivers/net/e1000/eeprom.c | 36 +++++++++++++++++++++++++++++++-----
 drivers/net/e1000/main.c   |  2 +-
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 1e2c5d7bc5e9..b49e1198160a 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -794,6 +794,8 @@ struct e1000_eeprom_info {
 #ifndef E1000_EEPROM_GRANT_ATTEMPTS
 #define E1000_EEPROM_GRANT_ATTEMPTS 1000 /* EEPROM # attempts to gain grant */
 #endif
+#define E1000_EECD_FLASH_IN_USE     0x00000100  /* Flash is present with a valid signature */
+#define E1000_EECD_EE_PRES          0x00000100
 #define E1000_EECD_AUTO_RD          0x00000200  /* EEPROM Auto Read done */
 #define E1000_EECD_SIZE_EX_MASK     0x00007800  /* EEprom Size */
 #define E1000_EECD_SIZE_EX_SHIFT    11
@@ -2195,6 +2197,7 @@ void e1000_write_reg_array(struct e1000_hw *hw, uint32_t base,
 void e1000_write_flush(struct e1000_hw *hw);
 
 int32_t e1000_init_eeprom_params(struct e1000_hw *hw);
+int e1000_eeprom_valid(struct e1000_hw *hw);
 int e1000_validate_eeprom_checksum(struct e1000_hw *hw);
 int32_t e1000_read_eeprom(struct e1000_hw *hw, uint16_t offset,
 		uint16_t words,
diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index cb92deee3b44..afd129b79fab 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -961,6 +961,9 @@ int32_t e1000_read_eeprom(struct e1000_hw *hw, uint16_t offset,
 
 	DEBUGFUNC();
 
+	if (!e1000_eeprom_valid(hw))
+		return -EINVAL;
+
 	/* A check for invalid values:  offset too large, too many words,
 	 * and not enough words.
 	 */
@@ -1413,12 +1416,14 @@ int e1000_register_invm(struct e1000_hw *hw)
 	u16 word;
 	struct param_d *p;
 
-	ret = e1000_read_eeprom(hw, 0x0a, 1, &word);
-	if (ret < 0)
-		return ret;
+	if (e1000_eeprom_valid(hw)) {
+		ret = e1000_read_eeprom(hw, 0x0a, 1, &word);
+		if (ret < 0)
+			return ret;
 
-	if (word & (1 << 15))
-		dev_warn(hw->dev, "iNVM lockout mechanism is active\n");
+		if (word & (1 << 15))
+			dev_warn(hw->dev, "iNVM lockout mechanism is active\n");
+	}
 
 	hw->invm.cdev.dev = hw->dev;
 	hw->invm.cdev.ops = &e1000_invm_ops;
@@ -1449,6 +1454,27 @@ int e1000_register_invm(struct e1000_hw *hw)
 	return ret;
 }
 
+int e1000_eeprom_valid(struct e1000_hw *hw)
+{
+	uint32_t eecd;
+
+	if (hw->mac_type != e1000_igb)
+		return 1;
+
+	/*
+	 * if AUTO_RD or EE_PRES are not set in EECD, the shadow RAM is invalid
+	 * (and in practise seems to contain the contents of iNVM).
+	 */
+	eecd = e1000_read_reg(hw, E1000_EECD);
+	if (!(eecd & E1000_EECD_AUTO_RD))
+		return 0;
+
+	if (!(eecd & E1000_EECD_EE_PRES))
+		return 0;
+
+	return 1;
+}
+
 /*
  * This function has a wrong name for historic reasons, it doesn't add an
  * eeprom, but the flash (if available) that is used to simulate the eeprom.
diff --git a/drivers/net/e1000/main.c b/drivers/net/e1000/main.c
index bb6ab4eb0360..0139c4a6d758 100644
--- a/drivers/net/e1000/main.c
+++ b/drivers/net/e1000/main.c
@@ -3597,7 +3597,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		}
 	}
 
-	if (e1000_validate_eeprom_checksum(hw))
+	if (!e1000_eeprom_valid(hw) || e1000_validate_eeprom_checksum(hw))
 		return 0;
 
 	e1000_get_ethaddr(edev, edev->ethaddr);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 10/6] net/e1000: indicate in error messages where the failure occured

[Uwe Kleine-König]

There used to be four places that all emit the same error message. Even
if in a given context not all four of them can be relevant, there are
always two possible locations where the message can origin from.

So make the output slightly different in all places to ease future
debugging.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/eeprom.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index afd129b79fab..0d91a88bd74f 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -708,7 +708,7 @@ static int e1000_flash_mode_wait_for_idle(struct e1000_hw *hw)
 				       E1000_FLSWCTL_DONE, SECOND);
 	if (ret < 0)
 		dev_err(hw->dev,
-			"Timeout waiting for FLSWCTL.DONE to be set\n");
+			"Timeout waiting for FLSWCTL.DONE to be set (wait)\n");
 	return ret;
 }
 
@@ -764,7 +764,7 @@ static int e1000_flash_mode_read_chunk(struct e1000_hw *hw, loff_t offset,
 				     SECOND);
 		if (ret < 0) {
 			dev_err(hw->dev,
-				"Timeout waiting for FLSWCTL.DONE to be set\n");
+				"Timeout waiting for FLSWCTL.DONE to be set (read)\n");
 			return ret;
 		}
 
@@ -820,7 +820,7 @@ static int e1000_flash_mode_write_chunk(struct e1000_hw *hw, loff_t offset,
 				     SECOND);
 		if (ret < 0) {
 			dev_err(hw->dev,
-				"Timeout waiting for FLSWCTL.DONE to be set\n");
+				"Timeout waiting for FLSWCTL.DONE to be set (write)\n");
 			return ret;
 		}
 
@@ -857,7 +857,7 @@ static int e1000_flash_mode_erase_chunk(struct e1000_hw *hw, loff_t offset,
 			     SECOND);
 	if (ret < 0) {
 		dev_err(hw->dev,
-			"Timeout waiting for FLSWCTL.DONE to be set\n");
+			"Timeout waiting for FLSWCTL.DONE to be set (erase)\n");
 		return ret;
 	}
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 11/6] net/e1000: expand timeout for flash erasure

[Uwe Kleine-König]

Erasing the whole flash takes approximately 7s on one of my test
machines. Expand the timeout accordingly.

Note however that it is in general not allowed to hold the flash
semaphore for so long and "firmware might implement a timeout
mechanism and take ownership of the relevant [lock]" after one second.
So a chip erase should better only be done when firmware doesn't make
use of the flash.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/eeprom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 0d91a88bd74f..0f1305c8499b 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -854,7 +854,7 @@ static int e1000_flash_mode_erase_chunk(struct e1000_hw *hw, loff_t offset,
 	ret = e1000_poll_reg(hw, E1000_FLSWCTL,
 			     E1000_FLSWCTL_DONE | E1000_FLSWCTL_FLBUSY,
 			     E1000_FLSWCTL_DONE,
-			     SECOND);
+			     10 * SECOND);
 	if (ret < 0) {
 		dev_err(hw->dev,
 			"Timeout waiting for FLSWCTL.DONE to be set (erase)\n");
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/6] net/e1000: more cleanups and ways to access i210 storage

[Sascha Hauer]

On Wed, Nov 22, 2017 at 11:22:25AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> these are a few patches that cumulated here related to the i210 network
> controller.
> 
> These are nearly good enough to populate the attached flash with a valid
> firmware (which you still need to have yourself) on all machines I have
> access to.
> 
> There is still one (to me) known problem: When the flash doesn't contain
> a valid image, accessing the eeprom (which is a simulated device on i210
> that is backed up by shadow RAM and the two first pages of flash) gives
> the contents of iNVM instead of the expected default eeprom. That
> results in a few wrong turns in the code but they are not grave enough
> to stop being able to write a valid firmware image.
> 
> A bit that is still missing is that a customer machine uses an flash
> that is write protected at power on. An i210 with an programmed flash is
> clever enough to remove the write protection before writing to it, but
> that doesn't help when the flash is not programmed (tin opener inside).
> I have a prototype that can remove the write protection with a
> cryptic command but that's not ready for prime time yet.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (6):
>   net/e1000: fix coding style at a few locations
>   net/e1000: don't use the eeprom word size as timeout to get a
>     semaphore
>   net/e1000: reorder functions
>   net/e1000: provide access to iNVM even if a flash is present
>   net/e1000: fix size of invm device
>   net/e1000: provide device for accessing emulated eeprom
> 
>  drivers/net/e1000/e1000.h  |   8 +-
>  drivers/net/e1000/eeprom.c | 324 ++++++++++++++++++++++++++-------------------
>  drivers/net/e1000/main.c   |   6 +-
>  3 files changed, 196 insertions(+), 142 deletions(-)

Applied, thanks

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox