* [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
* 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 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
* [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
* 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
* [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 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