From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 02FB8C4321E for ; Thu, 30 Sep 2021 21:22:00 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 47F3861A3F for ; Thu, 30 Sep 2021 21:21:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 47F3861A3F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=walle.cc Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3D48280F5F; Thu, 30 Sep 2021 23:21:55 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=walle.cc Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=walle.cc header.i=@walle.cc header.b="tibRiDUe"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id DCE1A80F6A; Thu, 30 Sep 2021 23:21:52 +0200 (CEST) Received: from ssl.serverraum.org (ssl.serverraum.org [IPv6:2a01:4f8:151:8464::1:2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 93A6580F1A for ; Thu, 30 Sep 2021 23:21:49 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=walle.cc Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=michael@walle.cc Received: from ssl.serverraum.org (web.serverraum.org [172.16.0.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ssl.serverraum.org (Postfix) with ESMTPSA id E3D6322172; Thu, 30 Sep 2021 23:21:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=walle.cc; s=mail2016061301; t=1633036909; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=mWhZOUIGG5GEdU2O8KkigjBmomPRITvTjQV7qMiDlXM=; b=tibRiDUe0a2ug7gJKPVfk/QN4t0dqkDAY5PF7AhNasERVa/F0x5A23HAsohHaT0t/qnREJ nXm50rSxDuw0wk9lSBMS82ZVQq8Uut+tdd0r6iVi1TzLz4r3aRUC2mi8JbfjsH2l1J8f8s wSyvnVzj2pQ52EG+natMvY5V8k9B0Kw= MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 30 Sep 2021 23:21:48 +0200 From: Michael Walle To: Frieder Schrempf Cc: u-boot@lists.denx.de, Heiko Thiery , Frieder Schrempf , Bin Meng , Jagan Teki , Pratyush Yadav , Sean Anderson , Simon Glass Subject: Re: [PATCH] cmd: sf: Support unaligned flash updates with 'sf update' In-Reply-To: <20210930161926.2748887-1-frieder@fris.de> References: <20210930161926.2748887-1-frieder@fris.de> User-Agent: Roundcube Webmail/1.4.11 Message-ID: <23600cdcfd4bf7f6b925efea28b2db1a@walle.cc> X-Sender: michael@walle.cc X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Am 2021-09-30 18:19, schrieb Frieder Schrempf: > From: Frieder Schrempf > > Currently 'sf update' supports only offsets that are aligned to the > erase block size of the serial flash. Unaligned offsets result in > something like: > > => sf update ${kernel_addr_r} 0x400 ${filesize} > device 0 offset 0x400, size 0x11f758 > SPI flash failed in erase step > > In order to support unaligned updates, we simply read the first full > block and check only the requested part of the block for changes. If > necessary, the block is erased, the first (unchanged) part of the block > is written back together with the second part of the block (updated > data). > > Signed-off-by: Frieder Schrempf > --- > cmd/sf.c | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/cmd/sf.c b/cmd/sf.c > index eac27ed2d7..c54b4b10bb 100644 > --- a/cmd/sf.c > +++ b/cmd/sf.c > @@ -171,30 +171,32 @@ static int do_spi_flash_probe(int argc, char > *const argv[]) > static const char *spi_flash_update_block(struct spi_flash *flash, u32 > offset, > size_t len, const char *buf, char *cmp_buf, size_t *skipped) > { > + u32 offset_aligned = ALIGN_DOWN(offset, flash->sector_size); > + u32 sector_offset = offset - offset_aligned; > char *ptr = (char *)buf; > > debug("offset=%#x, sector_size=%#x, len=%#zx\n", > offset, flash->sector_size, len); > /* Read the entire sector so to allow for rewriting */ > - if (spi_flash_read(flash, offset, flash->sector_size, cmp_buf)) > + if (spi_flash_read(flash, offset_aligned, flash->sector_size, > cmp_buf)) > return "read"; > /* Compare only what is meaningful (len) */ > - if (memcmp(cmp_buf, buf, len) == 0) { > + if (memcmp(cmp_buf + sector_offset, buf, len) == 0) { > debug("Skip region %x size %zx: no change\n", > offset, len); > *skipped += len; > return NULL; > } > /* Erase the entire sector */ > - if (spi_flash_erase(flash, offset, flash->sector_size)) > + if (spi_flash_erase(flash, offset_aligned, flash->sector_size)) > return "erase"; > /* If it's a partial sector, copy the data into the temp-buffer */ > if (len != flash->sector_size) { > - memcpy(cmp_buf, buf, len); > + memcpy(cmp_buf + sector_offset, buf, len); > ptr = cmp_buf; > } > /* Write one complete sector */ > - if (spi_flash_write(flash, offset, flash->sector_size, ptr)) > + if (spi_flash_write(flash, offset_aligned, flash->sector_size, ptr)) > return "write"; > > return NULL; > @@ -230,7 +232,10 @@ static int spi_flash_update(struct spi_flash > *flash, u32 offset, > ulong last_update = get_timer(0); > > for (; buf < end && !err_oper; buf += todo, offset += todo) { > - todo = min_t(size_t, end - buf, flash->sector_size); > + if (offset % flash->sector_size) do_div() to avoid linking errors on some archs, I guess. > + todo = flash->sector_size - (offset % flash->sector_size); This is missing the end-buf calculation, no? I.e. if you update just one sector at an offset and the data you write is smaller than "sector_size - offset". > + else > + todo = min_t(size_t, end - buf, flash->sector_size); > if (get_timer(last_update) > 100) { > printf(" \rUpdating, %zu%% %lu B/s", > 100 - (end - buf) / scale, -michael