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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id A1AC2C433F5 for ; Wed, 2 Feb 2022 09:56:11 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 198E9838A3; Wed, 2 Feb 2022 10:56:04 +0100 (CET) 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="np5by3Hp"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 45076838A4; Wed, 2 Feb 2022 10:56:02 +0100 (CET) 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 05C0183894 for ; Wed, 2 Feb 2022 10:55:58 +0100 (CET) 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 CA0DF22239; Wed, 2 Feb 2022 10:55:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=walle.cc; s=mail2016061301; t=1643795757; 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=xA0VX1us/XDGJcPhygdawuJq6arX4twelRGmdR0BF5s=; b=np5by3Hp4iqjluKJywwWeFu7V5nOzfJgvT+TgoAPD0lt0oBRU683sqmJ/QAlN9llygGh+C LNWOgwembJ5hZN4HR+ZV0X6MtzEnOo1pQh3tuxm1e2rK/7llXA8BAMRc8WT5yUp4ullEPU hvD9GxN86rmUCcvU0Fe6A1JJ0LIUhX4= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Wed, 02 Feb 2022 10:55:55 +0100 From: Michael Walle To: Jan Kiszka Cc: Tom Rini , Jagan Teki , chaochao2021666@163.com, Tudor.Ambarus@microchip.com, vigneshr@ti.com, baocheng.su@siemens.com, le.jin@siemens.com, u-boot@lists.denx.de, chao zeng Subject: Re: [PATCH v4] sf: Query write-protection status before operating the flash In-Reply-To: <1f466f11-6c3b-d1ce-800c-a171482abd2b@siemens.com> References: <6621668c-e9f0-f532-735f-f4bfbb8b00fc@siemens.com> <1f466f11-6c3b-d1ce-800c-a171482abd2b@siemens.com> User-Agent: Roundcube Webmail/1.4.12 Message-ID: <2f87f90d0af459ddb93d96264f60a3af@walle.cc> X-Sender: michael@walle.cc X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.5 at phobos.denx.de X-Virus-Status: Clean Am 2022-02-02 10:38, schrieb Jan Kiszka: > On 02.02.22 09:21, Michael Walle wrote: >> Am 2022-02-02 07:35, schrieb Jan Kiszka: >>> From: Jan Kiszka >>> >>> Do not suggest successful operation if a flash area to be changed is >>> actually locked, thus will not execute the request. Rather report an >>> error and bail out. That's way more user-friendly than asking them to >>> manually check for this case. >>> >>> Derived from original patch by Chao Zeng. >>> >>> Signed-off-by: Jan Kiszka >>> --- >>> >>> This is the successor of "[PATCH V3] sf: Querying write-protect >>> status >>> before operating the flash", moving the test into the CLI API, see >>> https://lore.kernel.org/u-boot/20220117175628.GQ2631111@bill-the-cat/. >>> >>>  cmd/sf.c | 12 ++++++++++++ >>>  1 file changed, 12 insertions(+) >>> >>> diff --git a/cmd/sf.c b/cmd/sf.c >>> index 8bdebd9fd8f..a24e04c690b 100644 >>> --- a/cmd/sf.c >>> +++ b/cmd/sf.c >>> @@ -287,6 +287,12 @@ static int do_spi_flash_read_write(int argc, >>> char >>> *const argv[]) >>>          return 1; >>>      } >>> >>> +    if (strncmp(argv[0], "read", 4) != 0 && flash->flash_is_locked >>> && >>> +        flash->flash_is_locked(flash, offset, len)) { >>> +        printf("ERROR: flash area is locked\n"); >>> +        return 1; >>> +    } >> >> Much better to handle it here. But I'm not sure if this is doing >> the right thing: >> >> Eventually, this function is called: >> >> /* >>  * Return 1 if the entire region is locked (if @locked is true) or >> unlocked (if >>  * @locked is false); 0 otherwise >>  */ >> static int stm_check_lock_status_sr(struct spi_nor *nor, loff_t ofs, >> u64 >> len, >>                                     u8 sr, bool locked) >> >> So I'd guess if you try to write to an area of the flash where only >> parts >> are locked, you still see it as unlocked and thus there will be no >> error. >> Which IMHO is even more confusing for a user. > > I suppose this is why the original patch was placed way more down the > call chain... I don't think that will help either because ultimately, you'd need to know the exact sizes and offsets of the areas which can be protected. > Back to square #1? Or can/should we split the request into > blocks? Blocks of which size? 4k? Might work but sounds hackish. You could introduce a new function which checks if *any* area is protected. -michael