[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account

[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account

[Fabio Estevam]

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;
 
-- 
1.9.1

[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account

[Lukasz Majewski]

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);
>  	if (ret)
>  		return ret;
>  

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

Applied to u-boot-dfu.

Thanks for your patch.

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account

[Stephen Warren]

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.

[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account

[Fabio Estevam]

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.

[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account

[Fabio Estevam]

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.

[U-Boot] [PATCH] dfu: dfu_sf: Take the start address into account

[Stephen Warren]

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.

[U-Boot] dfu: dfu_sf: Take the start address into account

[Tom Rini]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151019/1544f083/attachment.sig>