[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account
From: Fabio Estevam fabio.estevam@freescale.com
The dfu_alt_info_spl variable allows passing a starting point for the binary to be flashed in the SPI NOR.
For example, if we have 'dfu_alt_info_spl=spl raw 0x400', this means that we want to flash the binary starting at address 0x400.
In order to do so we need to erase the entire sector and write to the the subsequent SPI NOR sectors taking such start address into account for the address calculations.
Tested by succesfully writing SPL binary into 0x400 offset and the u-boot.img at offset 64 kiB of a SPL NOR.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- drivers/dfu/dfu_sf.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c index 448d95d..844da1f 100644 --- a/drivers/dfu/dfu_sf.c +++ b/drivers/dfu/dfu_sf.c @@ -23,17 +23,25 @@ static int dfu_read_medium_sf(struct dfu_entity *dfu, u64 offset, void *buf, return spi_flash_read(dfu->data.sf.dev, offset, *len, buf); }
+static u64 find_sector(struct dfu_entity *dfu, u64 start, u64 offset) +{ + return ((start + offset) / dfu->data.sf.dev->sector_size) * + dfu->data.sf.dev->sector_size; +} + static int dfu_write_medium_sf(struct dfu_entity *dfu, u64 offset, void *buf, long *len) { int ret;
- ret = spi_flash_erase(dfu->data.sf.dev, offset, + ret = spi_flash_erase(dfu->data.sf.dev, + find_sector(dfu, dfu->data.sf.start, offset), dfu->data.sf.dev->sector_size); if (ret) return ret;
- ret = spi_flash_write(dfu->data.sf.dev, offset, *len, buf); + ret = spi_flash_write(dfu->data.sf.dev, dfu->data.sf.start + offset, + *len, buf); if (ret) return ret;
Hi Fabio,
From: Fabio Estevam fabio.estevam@freescale.com
The dfu_alt_info_spl variable allows passing a starting point for the binary to be flashed in the SPI NOR.
For example, if we have 'dfu_alt_info_spl=spl raw 0x400', this means that we want to flash the binary starting at address 0x400.
In order to do so we need to erase the entire sector and write to the the subsequent SPI NOR sectors taking such start address into account for the address calculations.
Tested by succesfully writing SPL binary into 0x400 offset and the u-boot.img at offset 64 kiB of a SPL NOR.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
drivers/dfu/dfu_sf.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/dfu/dfu_sf.c b/drivers/dfu/dfu_sf.c index 448d95d..844da1f 100644 --- a/drivers/dfu/dfu_sf.c +++ b/drivers/dfu/dfu_sf.c @@ -23,17 +23,25 @@ static int dfu_read_medium_sf(struct dfu_entity *dfu, u64 offset, void *buf, return spi_flash_read(dfu->data.sf.dev, offset, *len, buf); }
+static u64 find_sector(struct dfu_entity *dfu, u64 start, u64 offset) +{
- return ((start + offset) / dfu->data.sf.dev->sector_size) *
dfu->data.sf.dev->sector_size;+}
static int dfu_write_medium_sf(struct dfu_entity *dfu, u64 offset, void *buf, long *len) { int ret;
- ret = spi_flash_erase(dfu->data.sf.dev, offset,
- ret = spi_flash_erase(dfu->data.sf.dev,
find_sector(dfu, dfu->data.sf.start,offset), dfu->data.sf.dev->sector_size); if (ret) return ret;
- ret = spi_flash_write(dfu->data.sf.dev, offset, *len, buf);
- ret = spi_flash_write(dfu->data.sf.dev, dfu->data.sf.start +
offset,
*len, buf);
Acked-by: Lukasz Majewski l.majewski@samsung.com
Applied to u-boot-dfu.
Thanks for your patch.
On 09/22/2015 09:50 PM, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
The dfu_alt_info_spl variable allows passing a starting point for the binary to be flashed in the SPI NOR.
For example, if we have 'dfu_alt_info_spl=spl raw 0x400', this means that we want to flash the binary starting at address 0x400.
In order to do so we need to erase the entire sector and write to the the subsequent SPI NOR sectors taking such start address into account for the address calculations.
Tested by succesfully writing SPL binary into 0x400 offset and the u-boot.img at offset 64 kiB of a SPL NOR.
This feels wrong. The entries in the DFU list must already be sector-aligned, so there's no need for the code to round them. If the entries are not already aligned, then the aligned erase operations will erase other data, which will cause corruption.
On Wed, Sep 23, 2015 at 12:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
This feels wrong. The entries in the DFU list must already be sector-aligned, so there's no need for the code to round them. If the entries are not already aligned, then the aligned erase operations will erase other data, which will cause corruption.
The start address does not need to be sector-aligned.
In my case the SPL image needs to be placed at 0x400.
On Wed, Sep 23, 2015 at 12:36 PM, Fabio Estevam festevam@gmail.com wrote:
On Wed, Sep 23, 2015 at 12:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
This feels wrong. The entries in the DFU list must already be sector-aligned, so there's no need for the code to round them. If the entries are not already aligned, then the aligned erase operations will erase other data, which will cause corruption.
The start address does not need to be sector-aligned.
In my case the SPL image needs to be placed at 0x400.
Also, just to clarify: in this patch we make sure that the erase operation are always sector-aligned.
Only the write operations can be 'shifted' due to the start address passed via dfu_alt_info.
On 09/23/2015 09:44 AM, Fabio Estevam wrote:
On Wed, Sep 23, 2015 at 12:36 PM, Fabio Estevam festevam@gmail.com wrote:
On Wed, Sep 23, 2015 at 12:22 PM, Stephen Warren swarren@wwwdotorg.org wrote:
This feels wrong. The entries in the DFU list must already be sector-aligned, so there's no need for the code to round them. If the entries are not already aligned, then the aligned erase operations will erase other data, which will cause corruption.
The start address does not need to be sector-aligned.
In my case the SPL image needs to be placed at 0x400.
Also, just to clarify: in this patch we make sure that the erase operation are always sector-aligned.
Only the write operations can be 'shifted' due to the start address passed via dfu_alt_info.
So it's OK to erase the data between 0..0x400 and not replace it? Even if that's true in your case, it seems quite unlikely in general.
Instead, the DFU region should start and end at sector-aligned locations so that erasing the region doesn't negatively affect other data. If logical objects in your flash aren't sector aligned, then that simply means you need to construct a complete flash image and write the whole thing at once. I don't thin there's any other way to avoid corruption.
On Wed, Sep 23, 2015 at 12:50:39AM -0300, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
The dfu_alt_info_spl variable allows passing a starting point for the binary to be flashed in the SPI NOR.
For example, if we have 'dfu_alt_info_spl=spl raw 0x400', this means that we want to flash the binary starting at address 0x400.
In order to do so we need to erase the entire sector and write to the the subsequent SPI NOR sectors taking such start address into account for the address calculations.
Tested by succesfully writing SPL binary into 0x400 offset and the u-boot.img at offset 64 kiB of a SPL NOR.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com Acked-by: Lukasz Majewski l.majewski@samsung.com
After making this use lldiv() for the math, Applied to u-boot/master, thanks!
participants (4)
-
Fabio Estevam -
Lukasz Majewski -
Stephen Warren -
Tom Rini