Re: [U-Boot] [PATCH 3/4] sf: Add extended address access support
-Original Message-
From: Langer Thomas (LQDE RD ST PON SW)
[mailto:thomas.lan...@lantiq.com]
Sent: 24 February 2013 21:21
To: Jagannadha Sutradharudu Teki; u-boot@lists.denx.de
Cc: mic...@theia.denx.de; Jagannadha Sutradharudu Teki
Subject: AW: [U-Boot] [PATCH 3/4] sf: Add extended address access support
Hello Jagan,
Am 23.02.2013 12:39, schrieb Jagannadha Sutradharudu Teki:
This patch provides support to access an extended addressing in 3-byte
address mode.
The current implementation in spi_flash supports 3-byte address mode
due to this up to 16MB amount of flash is able to access for those
flashes which has an actual size of 16MB.
extended/bank address register contains an information to access the
4th byte addressing hence the flashes which has 16MB can be accessible.
Signed-off-by: Jagannadha Sutradharudu Teki jaga...@xilinx.com
---
drivers/mtd/spi/spi_flash.c | 81
++
drivers/mtd/spi/spi_flash_internal.h |8 +++
2 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index c168c1c..16e5f59 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -23,6 +23,30 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
cmd[3] = addr 0;
}
+static int spi_flash_check_extaddr_access(struct spi_flash *flash,
+u32 *offset) {
+ int ret;
+
+ if (*offset = 0x100) {
+ ret = spi_flash_extaddr_access(flash,
STATUS_EXTADDR_ENABLE);
Restricting this to a single bit here would give the next size limit at 32M.
Please make it future-prov as much as possible: Why not directly use the
upper byte of the offset as parameter?
I didn't get you clearly, can you please elaborate.
+ if (ret) {
+ debug(SF: fail to %s ext addr bit\n,
+ STATUS_EXTADDR_ENABLE ? set : reset);
+ return ret;
+ }
+ *offset -= 0x100;
Are you sure that manipulating the value of the caller has no side-effect?
Is it even necessary, if the callers only do 3-byte-addressing and probably
ignore the upper byte?
May be you are not getting my exact code context.
From the current implementation on spi, we have 3-byte addressing means we are
able to access
only up to 16MB-- means 0 to 0x FF
If the flash has 32MB, if the user is giving an offset 0x100 it will again
pointing to 0x0.
Because we have 3-byte addressing able to access only first 16MB of flash but
the actual flash size is 32MB.
So, from my implementation if user is giving an offset of 0x100 then we
have enable the bank register
for pointing to second 16MB area on 32 MB flash and we must subtract the offset
from 16MB as we are using 3-byte
addressing. Means we are accessing 4-byte addressing in 3-byte address mode. Ie
is the reason I am subtracting it from16MB.
Please comment and let me know if you're not clear.
+ } else {
+ ret = spi_flash_extaddr_access(flash,
STATUS_EXTADDR_DISABLE);
+ if (ret) {
+ debug(SF: fail to %s ext addr bit\n,
+ STATUS_EXTADDR_DISABLE ? set : reset);
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
static int spi_flash_read_write(struct spi_slave *spi,
const u8 *cmd, size_t cmd_len,
const u8 *data_out, u8 *data_in, @@ -73,6
+97,14 @@ int
spi_flash_cmd_write_multi(struct spi_flash *flash, u32 offset,
int ret;
u8 cmd[4];
+ if (flash-size 0x100) {
As already said in my comments to patch 1/4, this check (and the check for
manufacturer ID) should be done only once once during probing of the flash.
Then, if necessary, adapted functions for read, write and erase should be
assigned to the function-pointers.
Or use an additional function-pointer, which can be set to this or a similar
function.
Then the call of this function should be included in the loops, which all
these
functions have.
Otherwise, as with your current code, you have a problem if the current
accessed block crosses the boundary at 16M. Have you ever tried this?
Means this check should be in probe call, by adding one member in spi_flash
structure, is it?.
I am not understanding extra function pointers concept, will you please explain.
+ ret = spi_flash_check_extaddr_access(flash, offset);
+ if (ret) {
+ debug(SF: fail to acess ext_addr\n);
+ return ret;
+ }
+ }
+
page_size = flash-page_size;
page_addr = offset / page_size;
byte_addr = offset % page_size;
@@ -139,6 +171,15 @@ int spi_flash_cmd_read_fast(struct spi_flash
*flash, u32 offset,
size_t len, void *data)
{
u8 cmd[5];
+ int ret
Re: [U-Boot] [PATCH 3/4] sf: Add extended address access support
Hello Jagan,
Am 23.02.2013 12:39, schrieb Jagannadha Sutradharudu Teki:
This patch provides support to access an extended addressing
in 3-byte address mode.
The current implementation in spi_flash supports 3-byte address mode
due to this up to 16MB amount of flash is able to access for those
flashes which has an actual size of 16MB.
extended/bank address register contains an information to access the
4th byte addressing hence the flashes which has 16MB can be accessible.
Signed-off-by: Jagannadha Sutradharudu Teki jaga...@xilinx.com
---
drivers/mtd/spi/spi_flash.c | 81
++
drivers/mtd/spi/spi_flash_internal.h |8 +++
2 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index c168c1c..16e5f59 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -23,6 +23,30 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
cmd[3] = addr 0;
}
+static int spi_flash_check_extaddr_access(struct spi_flash *flash, u32
*offset)
+{
+ int ret;
+
+ if (*offset = 0x100) {
+ ret = spi_flash_extaddr_access(flash, STATUS_EXTADDR_ENABLE);
Restricting this to a single bit here would give the next size limit at 32M.
Please make it future-prov as much as possible: Why not directly use the upper
byte of the offset as parameter?
+ if (ret) {
+ debug(SF: fail to %s ext addr bit\n,
+ STATUS_EXTADDR_ENABLE ? set : reset);
+ return ret;
+ }
+ *offset -= 0x100;
Are you sure that manipulating the value of the caller has no side-effect?
Is it even necessary, if the callers only do 3-byte-addressing and probably
ignore the upper byte?
+ } else {
+ ret = spi_flash_extaddr_access(flash, STATUS_EXTADDR_DISABLE);
+ if (ret) {
+ debug(SF: fail to %s ext addr bit\n,
+ STATUS_EXTADDR_DISABLE ? set : reset);
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
static int spi_flash_read_write(struct spi_slave *spi,
const u8 *cmd, size_t cmd_len,
const u8 *data_out, u8 *data_in,
@@ -73,6 +97,14 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32
offset,
int ret;
u8 cmd[4];
+ if (flash-size 0x100) {
As already said in my comments to patch 1/4, this check (and the check for
manufacturer ID) should be done
only once once during probing of the flash.
Then, if necessary, adapted functions for read, write and erase should be
assigned to the function-pointers.
Or use an additional function-pointer, which can be set to this or a similar
function.
Then the call of this function should be included in the loops, which all these
functions have.
Otherwise, as with your current code, you have a problem if the current
accessed block crosses the boundary
at 16M. Have you ever tried this?
+ ret = spi_flash_check_extaddr_access(flash, offset);
+ if (ret) {
+ debug(SF: fail to acess ext_addr\n);
+ return ret;
+ }
+ }
+
page_size = flash-page_size;
page_addr = offset / page_size;
byte_addr = offset % page_size;
@@ -139,6 +171,15 @@ int spi_flash_cmd_read_fast(struct spi_flash *flash, u32
offset,
size_t len, void *data)
{
u8 cmd[5];
+ int ret;
+
+ if (flash-size 0x100) {
+ ret = spi_flash_check_extaddr_access(flash, offset);
+ if (ret) {
+ debug(SF: fail to acess ext_addr\n);
+ return ret;
+ }
+ }
cmd[0] = CMD_READ_ARRAY_FAST;
spi_flash_addr(offset, cmd);
@@ -196,6 +237,14 @@ int spi_flash_cmd_erase(struct spi_flash *flash, u32
offset, size_t len)
int ret;
u8 cmd[4];
+ if (flash-size 0x100) {
+ ret = spi_flash_check_extaddr_access(flash, offset);
+ if (ret) {
+ debug(SF: fail to acess ext_addr\n);
+ return ret;
+ }
+ }
+
erase_size = flash-sector_size;
if (offset % erase_size || len % erase_size) {
debug(SF: Erase offset/length not multiple of erase size\n);
@@ -333,6 +382,38 @@ int spi_flash_cmd_extaddr_read(struct spi_flash *flash,
void *data)
return spi_flash_read_common(flash, cmd, 1, data, 1);
}
+int spi_flash_extaddr_access(struct spi_flash *flash, u8 status)
+{
+ int ret, pass;
+ u8 data = 0, write_done = 0;
+
+ for (pass = 0; pass 2; pass++) {
Why this is required??
+ ret = spi_flash_cmd_extaddr_read(flash, (void *)data);
+ if (ret 0) {
+
[U-Boot] [PATCH 3/4] sf: Add extended address access support
This patch provides support to access an extended addressing
in 3-byte address mode.
The current implementation in spi_flash supports 3-byte address mode
due to this up to 16MB amount of flash is able to access for those
flashes which has an actual size of 16MB.
extended/bank address register contains an information to access the
4th byte addressing hence the flashes which has 16MB can be accessible.
Signed-off-by: Jagannadha Sutradharudu Teki jaga...@xilinx.com
---
drivers/mtd/spi/spi_flash.c | 81 ++
drivers/mtd/spi/spi_flash_internal.h |8 +++
2 files changed, 89 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index c168c1c..16e5f59 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -23,6 +23,30 @@ static void spi_flash_addr(u32 addr, u8 *cmd)
cmd[3] = addr 0;
}
+static int spi_flash_check_extaddr_access(struct spi_flash *flash, u32 *offset)
+{
+ int ret;
+
+ if (*offset = 0x100) {
+ ret = spi_flash_extaddr_access(flash, STATUS_EXTADDR_ENABLE);
+ if (ret) {
+ debug(SF: fail to %s ext addr bit\n,
+ STATUS_EXTADDR_ENABLE ? set : reset);
+ return ret;
+ }
+ *offset -= 0x100;
+ } else {
+ ret = spi_flash_extaddr_access(flash, STATUS_EXTADDR_DISABLE);
+ if (ret) {
+ debug(SF: fail to %s ext addr bit\n,
+ STATUS_EXTADDR_DISABLE ? set : reset);
+ return ret;
+ }
+ }
+
+ return ret;
+}
+
static int spi_flash_read_write(struct spi_slave *spi,
const u8 *cmd, size_t cmd_len,
const u8 *data_out, u8 *data_in,
@@ -73,6 +97,14 @@ int spi_flash_cmd_write_multi(struct spi_flash *flash, u32
offset,
int ret;
u8 cmd[4];
+ if (flash-size 0x100) {
+ ret = spi_flash_check_extaddr_access(flash, offset);
+ if (ret) {
+ debug(SF: fail to acess ext_addr\n);
+ return ret;
+ }
+ }
+
page_size = flash-page_size;
page_addr = offset / page_size;
byte_addr = offset % page_size;
@@ -139,6 +171,15 @@ int spi_flash_cmd_read_fast(struct spi_flash *flash, u32
offset,
size_t len, void *data)
{
u8 cmd[5];
+ int ret;
+
+ if (flash-size 0x100) {
+ ret = spi_flash_check_extaddr_access(flash, offset);
+ if (ret) {
+ debug(SF: fail to acess ext_addr\n);
+ return ret;
+ }
+ }
cmd[0] = CMD_READ_ARRAY_FAST;
spi_flash_addr(offset, cmd);
@@ -196,6 +237,14 @@ int spi_flash_cmd_erase(struct spi_flash *flash, u32
offset, size_t len)
int ret;
u8 cmd[4];
+ if (flash-size 0x100) {
+ ret = spi_flash_check_extaddr_access(flash, offset);
+ if (ret) {
+ debug(SF: fail to acess ext_addr\n);
+ return ret;
+ }
+ }
+
erase_size = flash-sector_size;
if (offset % erase_size || len % erase_size) {
debug(SF: Erase offset/length not multiple of erase size\n);
@@ -333,6 +382,38 @@ int spi_flash_cmd_extaddr_read(struct spi_flash *flash,
void *data)
return spi_flash_read_common(flash, cmd, 1, data, 1);
}
+int spi_flash_extaddr_access(struct spi_flash *flash, u8 status)
+{
+ int ret, pass;
+ u8 data = 0, write_done = 0;
+
+ for (pass = 0; pass 2; pass++) {
+ ret = spi_flash_cmd_extaddr_read(flash, (void *)data);
+ if (ret 0) {
+ debug(SF: fail to read ext addr register\n);
+ return ret;
+ }
+
+ if ((data != status) !write_done) {
+ debug(SF: need to %s ext addr bit\n,
+ status ? set : reset);
+
+ write_done = 1;
+ ret = spi_flash_cmd_extaddr_write(flash, status);
+ if (ret 0) {
+ debug(SF: fail to write ext addr bit\n);
+ return ret;
+ }
+ } else {
+ debug(SF: ext addr bit is %s.\n,
+ status ? set : reset);
+ return ret;
+ }
+ }
+
+ return -1;
+}
+
/*
* The following table holds all device probe functions
*
diff --git a/drivers/mtd/spi/spi_flash_internal.h
b/drivers/mtd/spi/spi_flash_internal.h
index 65960ad..a6b04d7 100644
--- a/drivers/mtd/spi/spi_flash_internal.h
