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 99F6BC433F5 for ; Wed, 17 Nov 2021 11:59:49 +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 D3FDA61BD4 for ; Wed, 17 Nov 2021 11:59:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org D3FDA61BD4 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com 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 6EB5F82A66; Wed, 17 Nov 2021 12:59:46 +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="XmvpvqyO"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id ABF8681545; Wed, 17 Nov 2021 12:59:44 +0100 (CET) Received: from mail-qt1-x82b.google.com (mail-qt1-x82b.google.com [IPv6:2607:f8b0:4864:20::82b]) (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 9E66482F5E for ; Wed, 17 Nov 2021 12:59:38 +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-qt1-x82b.google.com with SMTP id q14so2285905qtx.10 for ; Wed, 17 Nov 2021 03:59:38 -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=T7WP/zHcBNwVObX2puJRlXx5Cd6GVlM30Tr3enN2kws=; b=XmvpvqyObrHQhv8vDblJE12GUIy2yPZKOSt0QZPXzfv3o0A1AOVeuKriyZBnXHuX7y 4UcZiKjVISKX25zECPLoku63H0mq2GJc774S5UpMEUsFJ0X2Y6XHL61yXXRH4El1R/YJ RMTuO5hpzKYOLsu3/0h80CP5QEbuNwEwQmfGI= 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=T7WP/zHcBNwVObX2puJRlXx5Cd6GVlM30Tr3enN2kws=; b=NjwNjiwrdE0sjtbCPHhXP8R54paxrJlq9mGGFP9OMH8HYJFjjnI2QUfWLYV4YDqnwE XMc9kEtUOY8mTwhQjzE7nmVpY2L1Kd/pi7Vx4zsO4UUgCqENUtU3yUuQ3hiGrwxr+R1C llvlV5rUcYwF4InnIcMSNuJyLi2E5+Uw2qD5eQV6iRu10Wb/brpQRYkaayTwgEkl4Fef uiF4GpHMzpDHOgqkPacm1zoTxc4yFMyBZloiBISZOnqbJ6a+aPKcC0GSWJn3Z5PU6Q/p E0iVVffGkQ2sVVWnhLddXDcMYJVQrWAiXIHu1qFBpYgLDRH8q6yuwkHXsRWGCpcOUlwa qfsA== X-Gm-Message-State: AOAM531Plfw/pXgCiiaLjZX828UdwVOtOww4CmqUyj/VT7DcVcmWEvl3 /oWQ9xa5iEYfVQe3y9GCyCf9rg== X-Google-Smtp-Source: ABdhPJzUNtInEtJ3VPTIFl1Zu8W9Yrz8SE8CLn7wPeFSioJ1sDYtqZouVDumVhafWn2WOBgslfO2jQ== X-Received: by 2002:a05:622a:153:: with SMTP id v19mr16044522qtw.144.1637150377308; Wed, 17 Nov 2021 03:59:37 -0800 (PST) Received: from bill-the-cat (2603-6081-7b01-cbda-c5ad-ec44-3eaa-133f.res6.spectrum.com. [2603:6081:7b01:cbda:c5ad:ec44:3eaa:133f]) by smtp.gmail.com with ESMTPSA id p10sm7287635qtw.97.2021.11.17.03.59.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Nov 2021 03:59:36 -0800 (PST) Date: Wed, 17 Nov 2021 06:59:34 -0500 From: Tom Rini To: Jagan Teki Cc: Michael Walle , chaochao2021666@163.com, Tudor.Ambarus@microchip.com, vigneshr@ti.com, jan.kiszka@siemens.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: <20211117115934.GW24579@bill-the-cat> References: <20211117024831.45586-1-chaochao2021666@163.com> <1122e19f1f4d0fc7b5cef0a02cb4439a@walle.cc> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="DzKDwAC8m/33A2me" Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.35 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 --DzKDwAC8m/33A2me Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Nov 17, 2021 at 01:43:28PM +0530, Jagan Teki wrote: > On Wed, Nov 17, 2021 at 1:33 PM Michael Walle wrote: > > > > Hi, > > > > Am 2021-11-17 03:48, schrieb chaochao2021666@163.com: > > > From: chao zeng > > > > > > When operating the write-protection flash,spi_flash_std_write() and > > > spi_flash_std_erase() would return wrong result.The flash is protecte= d, > > > but write or erase the flash would show "OK". > > > > > > Check the flash write protection state before operating the flash > > > and give a prompt to show it has been locked if the write-protection > > > has enbale > > > > > > Signed-off-by: chao zeng > > > > > > --- > > > > > > 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(+) > > > > > > diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.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; > > > > > > + if (flash->flash_is_locked && flash->flash_is_locked(flash, off= set, > > > len)) > > > + printf("SF: Operate on the protected area.Writes will be > > > ignored\n"); > > > > 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. > > > > Also, this is issuing an additional command "read SR" for every write. > > > > 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" command? > > If that is the case, this should be added to that command instead. > > > > > + > > > return mtd->_write(mtd, offset, len, &retlen, buf); > > > } > > > > > > @@ -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; > > > > > > + if (flash->flash_is_locked && flash->flash_is_locked(flash, off= set, > > > len)) > > > + printf("SF: Operate on the protected area.Erase will be= ignored\n"); >=20 > My fundamental question, why cannot we use 'sf protect' then 'sf write'? 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 Tom --DzKDwAC8m/33A2me Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmGU7qYACgkQFHw5/5Y0 tyyZMQv+LT8Ff7dCVjz66RIhs+3/kuxBxOk8GWDzvQKTYoelAV98BXlX3hJQjwr7 LPcXyy7b2ytuPF2/GcFNowkXN5ACSfrvtwEKupmBA0t0DFuWgw6F95YUU8XKjR7L gX37Bk02kVTCUgrDM+Ytoy3Ium+VzWGkqxLJIdY6dfE2mhTHdp6ZMWBF3UNkaNKw BtgFYsOAPMEOKUsAfwAOZem6oqjiD3XnFHVl/9/d500fnQvv8pBU47NXEsutOGiv C/3MHmWovUmei0DlwOG5qQPo7ZcrJEIJCzcTmFG2PAu30t9wV79K7B/CpN2dpcR+ RIlibPWoBmukq3/FnuQHkl+pKl2osM+RJ8Msm3zrdrA/LwPgSyNtcn1Z3lb394En 37DagnAGrUBO9LN0v8hsJgryO+hhgPyPcJL8N6OLIuWzskqNa+DVAEFntnIc1vno me2TObLZQ3kdZG3qkr035aH8g2hOYFrSShw0H/i3eHh3S+As+O07U0niPaNeWlYU x37BvP5W =FnIt -----END PGP SIGNATURE----- --DzKDwAC8m/33A2me--