mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/3] e1000: access to flash on i210
@ 2017-10-09  9:36 Uwe Kleine-König
  2017-10-09  9:36 ` [PATCH 1/3] e1000: implement register mapping for E1000_{EERD, FLSW{CTL, DATA, CNT}} Uwe Kleine-König
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2017-10-09  9:36 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Hello,

this series allows me to read the flash chip connected to an i210 network
adapter. I didn't test much yet (e.g. writing) but I think the first two
patches are OK anyhow. Maybe Andrey can comment about the third patch while I'm
out for lunch :-)

Best regards
Uwe

Uwe Kleine-König (3):
  e1000: implement register mapping for E1000_{EERD,FLSW{CTL,DATA,CNT}}
  e1000: implement support for smaller flash chips
  HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle

 drivers/net/e1000/e1000.h  | 12 +++++-------
 drivers/net/e1000/eeprom.c | 18 +++++-------------
 drivers/net/e1000/regio.c  | 12 ++++++++++++
 3 files changed, 22 insertions(+), 20 deletions(-)

-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] e1000: implement register mapping for E1000_{EERD, FLSW{CTL, DATA, CNT}}
  2017-10-09  9:36 [PATCH 0/3] e1000: access to flash on i210 Uwe Kleine-König
@ 2017-10-09  9:36 ` Uwe Kleine-König
  2017-10-09 18:22   ` Andrey Smirnov
  2017-10-09  9:36 ` [PATCH 2/3] e1000: implement support for smaller flash chips Uwe Kleine-König
  2017-10-09  9:36 ` [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle Uwe Kleine-König
  2 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2017-10-09  9:36 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

Fixes: 4ff3269a70b5 ("e1000: Expose i210's external flash as MTD")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/net/e1000/e1000.h |  9 +++++----
 drivers/net/e1000/regio.c | 12 ++++++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index e6b493c84cc1..2a29ef47e818 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -410,7 +410,8 @@ struct e1000_tx_desc {
 #define E1000_CTRL     0x00000	/* Device Control - RW */
 #define E1000_STATUS   0x00008	/* Device Status - RO */
 #define E1000_EECD     0x00010	/* EEPROM/Flash Control - RW */
-#define E1000_EERD     0x00014	/* EEPROM Read - RW */
+#define E1000_EERD     (E1000_MIGHT_BE_REMAPPED | 0x00014)	/* EEPROM Read - RW */
+#define E1000_I210_EERD     0x12014	/* EEPROM Read - RW */
 #define E1000_CTRL_EXT 0x00018	/* Extended Device Control - RW */
 #define E1000_MDIC     0x00020	/* MDI Control - RW */
 #define E1000_FCAL     0x00028	/* Flow Control Address Low - RW */
@@ -447,11 +448,11 @@ struct e1000_tx_desc {
 #define E1000_FLASHT   0x01028  /* FLASH Timer Register */
 #define E1000_EEWR     (E1000_MIGHT_BE_REMAPPED | 0x0102C)  /* EEPROM Write Register - RW */
 #define E1000_I210_EEWR     0x12018  /* EEPROM Write Register - RW */
-#define E1000_FLSWCTL  0x01030  /* FLASH control register */
+#define E1000_FLSWCTL  (E1000_MIGHT_BE_REMAPPED | 0x01030)  /* FLASH control register */
 #define E1000_I210_FLSWCTL 0x12048  /* FLASH control register */
-#define E1000_FLSWDATA 0x01034  /* FLASH data register */
+#define E1000_FLSWDATA (E1000_MIGHT_BE_REMAPPED | 0x01034)  /* FLASH data register */
 #define E1000_I210_FLSWDATA 0x1204C  /* FLASH data register */
-#define E1000_FLSWCNT  0x01038  /* FLASH Access Counter */
+#define E1000_FLSWCNT  (E1000_MIGHT_BE_REMAPPED | 0x01038)  /* FLASH Access Counter */
 #define E1000_I210_FLSWCNT  0x12050  /* FLASH Access Counter */
 #define E1000_FLOP     0x0103C  /* FLASH Opcode Register */
 #define E1000_ERT      0x02008  /* Early Rx Threshold - RW */
diff --git a/drivers/net/e1000/regio.c b/drivers/net/e1000/regio.c
index 1610d5851f05..9ef325fb8581 100644
--- a/drivers/net/e1000/regio.c
+++ b/drivers/net/e1000/regio.c
@@ -7,6 +7,9 @@ static uint32_t e1000_true_offset(struct e1000_hw *hw, uint32_t reg)
 	if (reg & E1000_MIGHT_BE_REMAPPED) {
 		if (hw->mac_type == e1000_igb) {
 			switch (reg) {
+			case E1000_EERD:
+				reg = E1000_I210_EERD;
+				break;
 			case E1000_EEWR:
 				reg = E1000_I210_EEWR;
 				break;
@@ -16,6 +19,15 @@ static uint32_t e1000_true_offset(struct e1000_hw *hw, uint32_t reg)
 			case E1000_EEMNGCTL:
 				reg = E1000_I210_EEMNGCTL;
 				break;
+			case E1000_FLSWCTL:
+				reg = E1000_I210_FLSWCTL;
+				break;
+			case E1000_FLSWCNT:
+				reg = E1000_I210_FLSWCNT;
+				break;
+			case E1000_FLSWDATA:
+				reg = E1000_I210_FLSWDATA;
+				break;
 			}
 		}
 		reg &= ~E1000_MIGHT_BE_REMAPPED;
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/3] e1000: implement support for smaller flash chips
  2017-10-09  9:36 [PATCH 0/3] e1000: access to flash on i210 Uwe Kleine-König
  2017-10-09  9:36 ` [PATCH 1/3] e1000: implement register mapping for E1000_{EERD, FLSW{CTL, DATA, CNT}} Uwe Kleine-König
@ 2017-10-09  9:36 ` Uwe Kleine-König
  2017-10-09 18:24   ` Andrey Smirnov
  2017-10-09  9:36 ` [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle Uwe Kleine-König
  2 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2017-10-09  9:36 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

In older revisions of the i210 data sheet (rev 2.8) M25PE80 (1 MiB) is still
listed as supported. So check the full range of FLA.FL_SIZE which also
simplifies the code.

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

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 2a29ef47e818..1bc57bf2760c 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -2106,9 +2106,6 @@ struct e1000_eeprom_info {
 #define E1000_FLA			0x1201C
 #define E1000_FLA_FL_SIZE_SHIFT		17
 #define E1000_FLA_FL_SIZE_MASK		(0b111 << E1000_FLA_FL_SIZE_SHIFT) /* EEprom Size */
-#define E1000_FLA_FL_SIZE_2MB		0b101
-#define E1000_FLA_FL_SIZE_4MB		0b110
-#define E1000_FLA_FL_SIZE_8MB		0b111
 
 
 #define E1000_FLSWCTL_ADDR(a)		((a) & 0x00FFFFFF)
diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 1a0c6e15abef..739bc17a519e 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -414,17 +414,9 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
 			fla &= E1000_FLA_FL_SIZE_MASK;
 			fla >>= E1000_FLA_FL_SIZE_SHIFT;
 
-			switch (fla) {
-			case E1000_FLA_FL_SIZE_8MB:
-				eeprom->word_size = SZ_8M / 2;
-				break;
-			case E1000_FLA_FL_SIZE_4MB:
-				eeprom->word_size = SZ_4M / 2;
-				break;
-			case E1000_FLA_FL_SIZE_2MB:
-				eeprom->word_size = SZ_2M / 2;
-				break;
-			default:
+			if (fla) {
+				eeprom->word_size = (SZ_64K << fla) / 2;
+			} else {
 				eeprom->word_size = 2048;
 				dev_info(hw->dev, "Unprogrammed Flash detected, "
 					 "limiting access to first 4KB\n");
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle
  2017-10-09  9:36 [PATCH 0/3] e1000: access to flash on i210 Uwe Kleine-König
  2017-10-09  9:36 ` [PATCH 1/3] e1000: implement register mapping for E1000_{EERD, FLSW{CTL, DATA, CNT}} Uwe Kleine-König
  2017-10-09  9:36 ` [PATCH 2/3] e1000: implement support for smaller flash chips Uwe Kleine-König
@ 2017-10-09  9:36 ` Uwe Kleine-König
  2017-10-09 14:24   ` Uwe Kleine-König
  2 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2017-10-09  9:36 UTC (permalink / raw)
  To: barebox; +Cc: Andrey Smirnov

I don't understand all the consequences of this patch yet, but this makes reading
out the flash chip connected to an i210 work for me.
---
 drivers/net/e1000/eeprom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
index 739bc17a519e..482a969f8d56 100644
--- a/drivers/net/e1000/eeprom.c
+++ b/drivers/net/e1000/eeprom.c
@@ -709,8 +709,8 @@ static int e1000_flash_mode_wait_for_idle(struct e1000_hw *hw)
 	 * execution by polling only FLSWCTL.DONE */
 
 	const int ret = e1000_poll_reg(hw, E1000_FLSWCTL,
-				       E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
-				       E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
+				       E1000_FLSWCTL_DONE,
+				       E1000_FLSWCTL_DONE,
 				       SECOND);
 	if (ret < 0)
 		dev_err(hw->dev,
-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle
  2017-10-09  9:36 ` [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle Uwe Kleine-König
@ 2017-10-09 14:24   ` Uwe Kleine-König
  2017-10-09 18:15     ` Andrey Smirnov
  0 siblings, 1 reply; 10+ messages in thread
From: Uwe Kleine-König @ 2017-10-09 14:24 UTC (permalink / raw)
  To: barebox, Andrey Smirnov

On Mon, Oct 09, 2017 at 11:36:16AM +0200, Uwe Kleine-König wrote:
> I don't understand all the consequences of this patch yet, but this makes reading
> out the flash chip connected to an i210 work for me.
> ---
>  drivers/net/e1000/eeprom.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
> index 739bc17a519e..482a969f8d56 100644
> --- a/drivers/net/e1000/eeprom.c
> +++ b/drivers/net/e1000/eeprom.c
> @@ -709,8 +709,8 @@ static int e1000_flash_mode_wait_for_idle(struct e1000_hw *hw)
>  	 * execution by polling only FLSWCTL.DONE */
>  
>  	const int ret = e1000_poll_reg(hw, E1000_FLSWCTL,
> -				       E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
> -				       E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
> +				       E1000_FLSWCTL_DONE,
> +				       E1000_FLSWCTL_DONE,

I tested a bit with and without this change at it seems as long as
nothing "strange" happens, testing for both FLSWCTL.DONE and
FLSWCTL.GLDONE (i.e. not applying my HACK patch) works fine.

Still I think only testing for FLSWCTL.DONE is better because it works
well even if the state machine is in the middle of a read request and
then changing the command (which is always done after
e1000_flash_mode_wait_for_idle()) should work well.

I'll resend with a better commit log once I tested this.

Alexey: I didn't understand the comment above the patched line, maybe
I'm missing something?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle
  2017-10-09 14:24   ` Uwe Kleine-König
@ 2017-10-09 18:15     ` Andrey Smirnov
  2017-10-09 18:41       ` Uwe Kleine-König
  0 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2017-10-09 18:15 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox

On Mon, Oct 9, 2017 at 7:24 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, Oct 09, 2017 at 11:36:16AM +0200, Uwe Kleine-König wrote:
>> I don't understand all the consequences of this patch yet, but this makes reading
>> out the flash chip connected to an i210 work for me.
>> ---
>>  drivers/net/e1000/eeprom.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
>> index 739bc17a519e..482a969f8d56 100644
>> --- a/drivers/net/e1000/eeprom.c
>> +++ b/drivers/net/e1000/eeprom.c
>> @@ -709,8 +709,8 @@ static int e1000_flash_mode_wait_for_idle(struct e1000_hw *hw)
>>        * execution by polling only FLSWCTL.DONE */
>>
>>       const int ret = e1000_poll_reg(hw, E1000_FLSWCTL,
>> -                                    E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
>> -                                    E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
>> +                                    E1000_FLSWCTL_DONE,
>> +                                    E1000_FLSWCTL_DONE,
>
> I tested a bit with and without this change at it seems as long as
> nothing "strange" happens, testing for both FLSWCTL.DONE and
> FLSWCTL.GLDONE (i.e. not applying my HACK patch) works fine.
>
> Still I think only testing for FLSWCTL.DONE is better because it works
> well even if the state machine is in the middle of a read request and
> then changing the command (which is always done after
> e1000_flash_mode_wait_for_idle()) should work well.
>
> I'll resend with a better commit log once I tested this.
>
> Alexey: I didn't understand the comment above the patched line, maybe
> I'm missing something?
>

Assuming this question is for me: what I meant by that comment is that
all of the flash related operations (read, write, erase) already poll
for E1000_FLSWCTL_DONE as their last step, so the only time that the
state of that bit would be unknown would be right after reset, the
first time any of those functions is executed.

As for GLDONE bit, I tried to stick to algorithms described in
"3.3.5.5 Software Flash Program Flow via the Flash-Mode Interface" and
"3.3.5.6 Software Flash Read Flow via the Flash-Mode Interface" of Rev
2.8 of the datasheet and that's where that "requrement" is coming
from.

I haven't touched this particular HW/area in more than a year, so,
since you actually work with it and can actually test things out I'll
defer to you to judge if certain HW checks are needed or not. My code
is by no means complete or exhaustively tested, so I have no problem
believing that I got some of the parts wrong.

Thanks,
Andrey Smirnov

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] e1000: implement register mapping for E1000_{EERD, FLSW{CTL, DATA, CNT}}
  2017-10-09  9:36 ` [PATCH 1/3] e1000: implement register mapping for E1000_{EERD, FLSW{CTL, DATA, CNT}} Uwe Kleine-König
@ 2017-10-09 18:22   ` Andrey Smirnov
  2017-10-09 18:33     ` [PATCH 1/3] e1000: implement register mapping for E1000_{EERD,FLSW{CTL,DATA,CNT}} Uwe Kleine-König
  0 siblings, 1 reply; 10+ messages in thread
From: Andrey Smirnov @ 2017-10-09 18:22 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox

On Mon, Oct 9, 2017 at 2:36 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> Fixes: 4ff3269a70b5 ("e1000: Expose i210's external flash as MTD")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/net/e1000/e1000.h |  9 +++++----
>  drivers/net/e1000/regio.c | 12 ++++++++++++
>  2 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
> index e6b493c84cc1..2a29ef47e818 100644
> --- a/drivers/net/e1000/e1000.h
> +++ b/drivers/net/e1000/e1000.h
> @@ -410,7 +410,8 @@ struct e1000_tx_desc {
>  #define E1000_CTRL     0x00000 /* Device Control - RW */
>  #define E1000_STATUS   0x00008 /* Device Status - RO */
>  #define E1000_EECD     0x00010 /* EEPROM/Flash Control - RW */
> -#define E1000_EERD     0x00014 /* EEPROM Read - RW */
> +#define E1000_EERD     (E1000_MIGHT_BE_REMAPPED | 0x00014)     /* EEPROM Read - RW */
> +#define E1000_I210_EERD     0x12014    /* EEPROM Read - RW */
>  #define E1000_CTRL_EXT 0x00018 /* Extended Device Control - RW */
>  #define E1000_MDIC     0x00020 /* MDI Control - RW */
>  #define E1000_FCAL     0x00028 /* Flow Control Address Low - RW */
> @@ -447,11 +448,11 @@ struct e1000_tx_desc {
>  #define E1000_FLASHT   0x01028  /* FLASH Timer Register */
>  #define E1000_EEWR     (E1000_MIGHT_BE_REMAPPED | 0x0102C)  /* EEPROM Write Register - RW */
>  #define E1000_I210_EEWR     0x12018  /* EEPROM Write Register - RW */
> -#define E1000_FLSWCTL  0x01030  /* FLASH control register */
> +#define E1000_FLSWCTL  (E1000_MIGHT_BE_REMAPPED | 0x01030)  /* FLASH control register */
>  #define E1000_I210_FLSWCTL 0x12048  /* FLASH control register */
> -#define E1000_FLSWDATA 0x01034  /* FLASH data register */
> +#define E1000_FLSWDATA (E1000_MIGHT_BE_REMAPPED | 0x01034)  /* FLASH data register */
>  #define E1000_I210_FLSWDATA 0x1204C  /* FLASH data register */
> -#define E1000_FLSWCNT  0x01038  /* FLASH Access Counter */
> +#define E1000_FLSWCNT  (E1000_MIGHT_BE_REMAPPED | 0x01038)  /* FLASH Access Counter */
>  #define E1000_I210_FLSWCNT  0x12050  /* FLASH Access Counter */
>  #define E1000_FLOP     0x0103C  /* FLASH Opcode Register */
>  #define E1000_ERT      0x02008  /* Early Rx Threshold - RW */
> diff --git a/drivers/net/e1000/regio.c b/drivers/net/e1000/regio.c
> index 1610d5851f05..9ef325fb8581 100644
> --- a/drivers/net/e1000/regio.c
> +++ b/drivers/net/e1000/regio.c
> @@ -7,6 +7,9 @@ static uint32_t e1000_true_offset(struct e1000_hw *hw, uint32_t reg)
>         if (reg & E1000_MIGHT_BE_REMAPPED) {
>                 if (hw->mac_type == e1000_igb) {
>                         switch (reg) {
> +                       case E1000_EERD:
> +                               reg = E1000_I210_EERD;
> +                               break;

Rev. 2.8 lists 0x00014 as alias for 0x12014 (that's why I didn't do
any "re-mapping" here) is it not true?

>                         case E1000_EEWR:
>                                 reg = E1000_I210_EEWR;
>                                 break;
> @@ -16,6 +19,15 @@ static uint32_t e1000_true_offset(struct e1000_hw *hw, uint32_t reg)
>                         case E1000_EEMNGCTL:
>                                 reg = E1000_I210_EEMNGCTL;
>                                 break;
> +                       case E1000_FLSWCTL:
> +                               reg = E1000_I210_FLSWCTL;
> +                               break;
> +                       case E1000_FLSWCNT:
> +                               reg = E1000_I210_FLSWCNT;
> +                               break;
> +                       case E1000_FLSWDATA:
> +                               reg = E1000_I210_FLSWDATA;
> +                               break;

Somehow missed these guys, sorry about that.

Acked-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Thanks,
Andrey Smirnov

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] e1000: implement support for smaller flash chips
  2017-10-09  9:36 ` [PATCH 2/3] e1000: implement support for smaller flash chips Uwe Kleine-König
@ 2017-10-09 18:24   ` Andrey Smirnov
  0 siblings, 0 replies; 10+ messages in thread
From: Andrey Smirnov @ 2017-10-09 18:24 UTC (permalink / raw)
  To: Uwe Kleine-König; +Cc: barebox

On Mon, Oct 9, 2017 at 2:36 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> In older revisions of the i210 data sheet (rev 2.8) M25PE80 (1 MiB) is still
> listed as supported. So check the full range of FLA.FL_SIZE which also
> simplifies the code.
>

Looks good to me:

Acked-by: Andrey Smirnov <andrew.smirnov@gmail.com>

> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/net/e1000/e1000.h  |  3 ---
>  drivers/net/e1000/eeprom.c | 14 +++-----------
>  2 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
> index 2a29ef47e818..1bc57bf2760c 100644
> --- a/drivers/net/e1000/e1000.h
> +++ b/drivers/net/e1000/e1000.h
> @@ -2106,9 +2106,6 @@ struct e1000_eeprom_info {
>  #define E1000_FLA                      0x1201C
>  #define E1000_FLA_FL_SIZE_SHIFT                17
>  #define E1000_FLA_FL_SIZE_MASK         (0b111 << E1000_FLA_FL_SIZE_SHIFT) /* EEprom Size */
> -#define E1000_FLA_FL_SIZE_2MB          0b101
> -#define E1000_FLA_FL_SIZE_4MB          0b110
> -#define E1000_FLA_FL_SIZE_8MB          0b111
>
>
>  #define E1000_FLSWCTL_ADDR(a)          ((a) & 0x00FFFFFF)
> diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
> index 1a0c6e15abef..739bc17a519e 100644
> --- a/drivers/net/e1000/eeprom.c
> +++ b/drivers/net/e1000/eeprom.c
> @@ -414,17 +414,9 @@ int32_t e1000_init_eeprom_params(struct e1000_hw *hw)
>                         fla &= E1000_FLA_FL_SIZE_MASK;
>                         fla >>= E1000_FLA_FL_SIZE_SHIFT;
>
> -                       switch (fla) {
> -                       case E1000_FLA_FL_SIZE_8MB:
> -                               eeprom->word_size = SZ_8M / 2;
> -                               break;
> -                       case E1000_FLA_FL_SIZE_4MB:
> -                               eeprom->word_size = SZ_4M / 2;
> -                               break;
> -                       case E1000_FLA_FL_SIZE_2MB:
> -                               eeprom->word_size = SZ_2M / 2;
> -                               break;
> -                       default:
> +                       if (fla) {
> +                               eeprom->word_size = (SZ_64K << fla) / 2;
> +                       } else {
>                                 eeprom->word_size = 2048;
>                                 dev_info(hw->dev, "Unprogrammed Flash detected, "
>                                          "limiting access to first 4KB\n");
> --
> 2.11.0
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] e1000: implement register mapping for E1000_{EERD,FLSW{CTL,DATA,CNT}}
  2017-10-09 18:22   ` Andrey Smirnov
@ 2017-10-09 18:33     ` Uwe Kleine-König
  0 siblings, 0 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2017-10-09 18:33 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

On Mon, Oct 09, 2017 at 11:22:34AM -0700, Andrey Smirnov wrote:
> On Mon, Oct 9, 2017 at 2:36 AM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > Fixes: 4ff3269a70b5 ("e1000: Expose i210's external flash as MTD")
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> >  drivers/net/e1000/e1000.h |  9 +++++----
> >  drivers/net/e1000/regio.c | 12 ++++++++++++
> >  2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
> > index e6b493c84cc1..2a29ef47e818 100644
> > --- a/drivers/net/e1000/e1000.h
> > +++ b/drivers/net/e1000/e1000.h
> > @@ -410,7 +410,8 @@ struct e1000_tx_desc {
> >  #define E1000_CTRL     0x00000 /* Device Control - RW */
> >  #define E1000_STATUS   0x00008 /* Device Status - RO */
> >  #define E1000_EECD     0x00010 /* EEPROM/Flash Control - RW */
> > -#define E1000_EERD     0x00014 /* EEPROM Read - RW */
> > +#define E1000_EERD     (E1000_MIGHT_BE_REMAPPED | 0x00014)     /* EEPROM Read - RW */
> > +#define E1000_I210_EERD     0x12014    /* EEPROM Read - RW */
> >  #define E1000_CTRL_EXT 0x00018 /* Extended Device Control - RW */
> >  #define E1000_MDIC     0x00020 /* MDI Control - RW */
> >  #define E1000_FCAL     0x00028 /* Flow Control Address Low - RW */
> > @@ -447,11 +448,11 @@ struct e1000_tx_desc {
> >  #define E1000_FLASHT   0x01028  /* FLASH Timer Register */
> >  #define E1000_EEWR     (E1000_MIGHT_BE_REMAPPED | 0x0102C)  /* EEPROM Write Register - RW */
> >  #define E1000_I210_EEWR     0x12018  /* EEPROM Write Register - RW */
> > -#define E1000_FLSWCTL  0x01030  /* FLASH control register */
> > +#define E1000_FLSWCTL  (E1000_MIGHT_BE_REMAPPED | 0x01030)  /* FLASH control register */
> >  #define E1000_I210_FLSWCTL 0x12048  /* FLASH control register */
> > -#define E1000_FLSWDATA 0x01034  /* FLASH data register */
> > +#define E1000_FLSWDATA (E1000_MIGHT_BE_REMAPPED | 0x01034)  /* FLASH data register */
> >  #define E1000_I210_FLSWDATA 0x1204C  /* FLASH data register */
> > -#define E1000_FLSWCNT  0x01038  /* FLASH Access Counter */
> > +#define E1000_FLSWCNT  (E1000_MIGHT_BE_REMAPPED | 0x01038)  /* FLASH Access Counter */
> >  #define E1000_I210_FLSWCNT  0x12050  /* FLASH Access Counter */
> >  #define E1000_FLOP     0x0103C  /* FLASH Opcode Register */
> >  #define E1000_ERT      0x02008  /* Early Rx Threshold - RW */
> > diff --git a/drivers/net/e1000/regio.c b/drivers/net/e1000/regio.c
> > index 1610d5851f05..9ef325fb8581 100644
> > --- a/drivers/net/e1000/regio.c
> > +++ b/drivers/net/e1000/regio.c
> > @@ -7,6 +7,9 @@ static uint32_t e1000_true_offset(struct e1000_hw *hw, uint32_t reg)
> >         if (reg & E1000_MIGHT_BE_REMAPPED) {
> >                 if (hw->mac_type == e1000_igb) {
> >                         switch (reg) {
> > +                       case E1000_EERD:
> > +                               reg = E1000_I210_EERD;
> > +                               break;
> 
> Rev. 2.8 lists 0x00014 as alias for 0x12014 (that's why I didn't do
> any "re-mapping" here) is it not true?

I have 3.1 and didn't see the alias, but indeed it's there. So this hunk
can be dropped. Will send a v2.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle
  2017-10-09 18:15     ` Andrey Smirnov
@ 2017-10-09 18:41       ` Uwe Kleine-König
  0 siblings, 0 replies; 10+ messages in thread
From: Uwe Kleine-König @ 2017-10-09 18:41 UTC (permalink / raw)
  To: Andrey Smirnov; +Cc: barebox

On Mon, Oct 09, 2017 at 11:15:09AM -0700, Andrey Smirnov wrote:
> On Mon, Oct 9, 2017 at 7:24 AM, Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> > On Mon, Oct 09, 2017 at 11:36:16AM +0200, Uwe Kleine-König wrote:
> >> I don't understand all the consequences of this patch yet, but this makes reading
> >> out the flash chip connected to an i210 work for me.
> >> ---
> >>  drivers/net/e1000/eeprom.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/e1000/eeprom.c b/drivers/net/e1000/eeprom.c
> >> index 739bc17a519e..482a969f8d56 100644
> >> --- a/drivers/net/e1000/eeprom.c
> >> +++ b/drivers/net/e1000/eeprom.c
> >> @@ -709,8 +709,8 @@ static int e1000_flash_mode_wait_for_idle(struct e1000_hw *hw)
> >>        * execution by polling only FLSWCTL.DONE */
> >>
> >>       const int ret = e1000_poll_reg(hw, E1000_FLSWCTL,
> >> -                                    E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
> >> -                                    E1000_FLSWCTL_DONE | E1000_FLSWCTL_GLDONE,
> >> +                                    E1000_FLSWCTL_DONE,
> >> +                                    E1000_FLSWCTL_DONE,
> >
> > I tested a bit with and without this change at it seems as long as
> > nothing "strange" happens, testing for both FLSWCTL.DONE and
> > FLSWCTL.GLDONE (i.e. not applying my HACK patch) works fine.
> >
> > Still I think only testing for FLSWCTL.DONE is better because it works
> > well even if the state machine is in the middle of a read request and
> > then changing the command (which is always done after
> > e1000_flash_mode_wait_for_idle()) should work well.
> >
> > I'll resend with a better commit log once I tested this.
> >
> > Alexey: I didn't understand the comment above the patched line, maybe
> > I'm missing something?
> >
> 
> Assuming this question is for me: what I meant by that comment is that

Ah yes, sorry. It seems I have to participate in the cut'n'paste
seminar, too.

> all of the flash related operations (read, write, erase) already poll
> for E1000_FLSWCTL_DONE as their last step, so the only time that the
> state of that bit would be unknown would be right after reset, the
> first time any of those functions is executed.

Right after reset or when someone poked in the hardware using mw/md, or
if a previous transaction was not completed (nor sure this can happen
though).
 
> As for GLDONE bit, I tried to stick to algorithms described in
> "3.3.5.5 Software Flash Program Flow via the Flash-Mode Interface" and
> "3.3.5.6 Software Flash Read Flow via the Flash-Mode Interface" of Rev
> 2.8 of the datasheet and that's where that "requrement" is coming
> from.

3.3.5.5 and 3.3.5.6 in Rev 3.1 only check .DONE at the start and for the
last step tell (for 3.3.5.5): 

	FLSWCTL.GLDONE bit is set by hardware when the last byte
	programmed has been written.

So I'd not use that to check if I can start. Still more as there is:

	But software can stop the transaction in the middle as long as
	it got the DONE bit read as 1b.

> I haven't touched this particular HW/area in more than a year, so,
> since you actually work with it and can actually test things out I'll
> defer to you to judge if certain HW checks are needed or not. My code
> is by no means complete or exhaustively tested, so I have no problem
> believing that I got some of the parts wrong.

OK. So I will send a v2 that drops checking for GLDONE.

Thanks for your replies.

Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-10-09 18:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-09  9:36 [PATCH 0/3] e1000: access to flash on i210 Uwe Kleine-König
2017-10-09  9:36 ` [PATCH 1/3] e1000: implement register mapping for E1000_{EERD, FLSW{CTL, DATA, CNT}} Uwe Kleine-König
2017-10-09 18:22   ` Andrey Smirnov
2017-10-09 18:33     ` [PATCH 1/3] e1000: implement register mapping for E1000_{EERD,FLSW{CTL,DATA,CNT}} Uwe Kleine-König
2017-10-09  9:36 ` [PATCH 2/3] e1000: implement support for smaller flash chips Uwe Kleine-König
2017-10-09 18:24   ` Andrey Smirnov
2017-10-09  9:36 ` [PATCH 3/3] HACK: e1000: don't check for FLSWCTL.GLDONE when waiting for idle Uwe Kleine-König
2017-10-09 14:24   ` Uwe Kleine-König
2017-10-09 18:15     ` Andrey Smirnov
2017-10-09 18:41       ` Uwe Kleine-König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox