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 E1E7EC433EF for ; Mon, 17 Jan 2022 17:56:38 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9B64F830F4; Mon, 17 Jan 2022 18:56:36 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com 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; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="QsU1fWlq"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 71A218128D; Mon, 17 Jan 2022 18:56:35 +0100 (CET) Received: from mail-qv1-xf34.google.com (mail-qv1-xf34.google.com [IPv6:2607:f8b0:4864:20::f34]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 184258128D for ; Mon, 17 Jan 2022 18:56:32 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qv1-xf34.google.com with SMTP id l13so17586076qvz.10 for ; Mon, 17 Jan 2022 09:56:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=zmLa+hMIngVz4f8QdbmhUuthRUs+X1L3y6uJxpn0r2Y=; b=QsU1fWlqTgDyGtlOwG5m7gcTr6bi2z9IoCFIahGu74Oq++zKTPCotWbomsdq8SRO57 xRqHcL4p+FSvIeAK8oGxNFcpiisqLrn3S/qgRs1K4iOeIjPw9BxYbHvyXLe6RtiX9lnW oXK7cZ1NzeDR3jdxb0mzQKjHcBaO9W9n5DM+g= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=zmLa+hMIngVz4f8QdbmhUuthRUs+X1L3y6uJxpn0r2Y=; b=I64wkIu4ydQmTNEKSfL0eK2xVAEbKpO5oK6qHYDmXg58oVju5Ee8GwKwdIMxAif0ZK 9SU8ppYngHVBxKLO7hXB2FdAJFIXs/Rrgp30bzqLTnS6hPfCoGxMowssdM3yvb6PQ8vh qJvw4PWNXehjuAaCkQGluYSNx/MNb+aJxQX1GhBr/bA7rUSmZoToamWDbxv1JwQSbEIW lmhLSoyXUTfuUJOdrpWt1z2N5jsoVyZkU8pOjbJPzqnNRcD/lesBnaJXdBk055Y/imej l0ro7Eh//P80Za9thNBQkA5Z8WEW8QdsK170N4rCZDLokEY8t/eRpW0wHGEXAJsLjVC6 3/3g== X-Gm-Message-State: AOAM532y3erOB1uJfTX0uYZadj5OC6nqFZTZzTvICJoTK3mLCl9SwQ0m jGHNNMh7yWSMFAeP+PeUArnswQ== X-Google-Smtp-Source: ABdhPJx+ZXgg9qTrC1nBrBBmvrJ7n9RHVAE9UUbVFC7OqBKSbT+P1ceJQ6zTBnQXeghUzHrUKxDXLw== X-Received: by 2002:a05:6214:3019:: with SMTP id ke25mr17613293qvb.90.1642442190940; Mon, 17 Jan 2022 09:56:30 -0800 (PST) Received: from bill-the-cat (2603-6081-7b01-cbda-8dc3-db57-8e4a-695a.res6.spectrum.com. [2603:6081:7b01:cbda:8dc3:db57:8e4a:695a]) by smtp.gmail.com with ESMTPSA id j2sm1708908qko.117.2022.01.17.09.56.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Jan 2022 09:56:30 -0800 (PST) Date: Mon, 17 Jan 2022 12:56:28 -0500 From: Tom Rini To: Jan Kiszka Cc: Jagan Teki , Michael Walle , 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 V3] sf: Querying write-protect status before operating the flash Message-ID: <20220117175628.GQ2631111@bill-the-cat> References: <20211117024831.45586-1-chaochao2021666@163.com> <1122e19f1f4d0fc7b5cef0a02cb4439a@walle.cc> <20211117115934.GW24579@bill-the-cat> <55006ef5-40ca-4a6a-7648-2c4d27446085@siemens.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="OGLMwEELQbPC02lM" Content-Disposition: inline In-Reply-To: <55006ef5-40ca-4a6a-7648-2c4d27446085@siemens.com> X-Clacks-Overhead: GNU Terry Pratchett 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.2 at phobos.denx.de X-Virus-Status: Clean --OGLMwEELQbPC02lM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jan 13, 2022 at 08:38:04AM +0100, Jan Kiszka wrote: > On 17.11.21 12:59, Tom Rini wrote: > > On Wed, Nov 17, 2021 at 01:43:28PM +0530, Jagan Teki wrote: > > > On Wed, Nov 17, 2021 at 1:33 PM Michael Walle wrot= e: > > > >=20 > > > > Hi, > > > >=20 > > > > Am 2021-11-17 03:48, schrieb chaochao2021666@163.com: > > > > > From: chao zeng > > > > >=20 > > > > > When operating the write-protection flash,spi_flash_std_write() a= nd > > > > > spi_flash_std_erase() would return wrong result.The flash is prot= ected, > > > > > but write or erase the flash would show "OK". > > > > >=20 > > > > > Check the flash write protection state before operating the flash > > > > > and give a prompt to show it has been locked if the write-protect= ion > > > > > has enbale > > > > >=20 > > > > > Signed-off-by: chao zeng > > > > >=20 > > > > > --- > > > > >=20 > > > > > Changes for V2: > > > > > - Return 0 not ENOPROTOOPT to refelect the flash feature > > > > > - Output prompt information > > > > > Changes for V3: > > > > > - Modify output information > > > > > - Delete return statement > > > > > --- > > > > > drivers/mtd/spi/sf_probe.c | 6 ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > >=20 > > > > > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_prob= e.c > > > > > index f461082e03..f9e879aec5 100644 > > > > > --- a/drivers/mtd/spi/sf_probe.c > > > > > +++ b/drivers/mtd/spi/sf_probe.c > > > > > @@ -109,6 +109,9 @@ static int spi_flash_std_write(struct udevice > > > > > *dev, u32 offset, size_t len, > > > > > struct mtd_info *mtd =3D &flash->mtd; > > > > > size_t retlen; > > > > >=20 > > > > > + if (flash->flash_is_locked && flash->flash_is_locked(flash,= offset, > > > > > len)) > > > > > + printf("SF: Operate on the protected area.Writes wi= ll be > > > > > ignored\n"); > > > >=20 > > > > I don't think this is the correct place for this output. This could > > > > also be called from a board file programmatically and then it might > > > > display this error, which is annoying. > > > >=20 > > > > Also, this is issuing an additional command "read SR" for every wri= te. > > > >=20 > > > > What is your intention here? To make the user aware that he is going > > > > to write to a write-protected region when he is using the "sf" comm= and? > > > > If that is the case, this should be added to that command instead. > > > >=20 > > > > > + > > > > > return mtd->_write(mtd, offset, len, &retlen, buf); > > > > > } > > > > >=20 > > > > > @@ -127,6 +130,9 @@ static int spi_flash_std_erase(struct udevice > > > > > *dev, u32 offset, size_t len) > > > > > instr.addr =3D offset; > > > > > instr.len =3D len; > > > > >=20 > > > > > + if (flash->flash_is_locked && flash->flash_is_locked(flash,= offset, > > > > > len)) > > > > > + printf("SF: Operate on the protected area.Erase wil= l be ignored\n"); > > >=20 > > > My fundamental question, why cannot we use 'sf protect' then 'sf writ= e'? > >=20 > > Where do we tell people to always run "sf protect" before "sf write" and > > why is that at all user friendly? No, we shouldn't run this test more > > than once per time we're told to write an image. But silently failing > > in cases we can detect a problem is also not correct. If it's possible > > to spot this easily with "sf protect" why not just do that as part of > > "sf write" and add a flag to skip the check if you know it's not needed? > > I assume it's a fairly cheap/quick operation to do this check. > >=20 >=20 > What's the status here? Who should propose/implement what now? Good question. Re-reading the quoted part here, the (valid!) concern I see on the one hand is that today you can "sf write", see "OK" and have had nothing written because the flash was protected, and that's something we could have known at the start of "sf write". The change as written is within the write API, rather than the CLI API, so could we move that check to cmd/sf.c instead? --=20 Tom --OGLMwEELQbPC02lM Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmHlrcwACgkQFHw5/5Y0 tyyvaAwAnDLYGqpbQaKnsrlrayYBI2w9spSMgHwjxv95jaEggbqqbzgSFT8BY0m9 ssQS56u2kktzrA19nEBXfuzQmIZA+23lfEDFQQsSsjKcueMRfSpJvdfKZTpQSfI1 T6rVz7d9nSVj1dzSq0yWX95fDaAONDB8SAHinQvYMeQS5xVC6bOSsIfk4NdXvYgI szPBukNmoXvNDvGWe1emfLjpPBt4B9Yzb4pTh1IQVYUojc3OcGhQwyzjTPESjb7f zsyyo58j/lJ1EE4sQf8PRCVikcUgl56sX53NKYqPsuiDJNqjpd4J9M2HzgGVavDj 2y77EhAYMf7CArpd9fmjEUL5KCq6AL5zlWvpMS0jSRMJdLdJwbWowys5KUcPi5y5 BB1Qiup35mxyr2jnKZeDJoleWbV6vaXekwlcv8MExxWAqGUHzyBnedNweTmE05tB qXOQwRbqmXOLzg5Jj/exjrOTopNCSq/qZcPmn5s3u8MnWWo4WwiqMzuwo/97TNDv +EYsiwP+ =fNbl -----END PGP SIGNATURE----- --OGLMwEELQbPC02lM--