On Mon, Nov 15, 2021 at 6:51 PM chaochao2021...@163.com
<chaochao2021...@163.com> wrote:
>
> On 2021/11/15 13:57, tudor.amba...@microchip.com wrote:
>
> Hi,
>
> + Michael
>
> On 11/15/21 4:37 AM, chaochao2021...@163.com wrote:
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> From: chao zeng <chao.z...@siemens.com>
>
> When operating the write-protection flash,spi_flash_std_write() and
> spi_flash_std_erase() would return wrong result.The flash is protected,
> 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 <chao.z...@siemens.com>
>
> ---
>
> Changes for V2:
> - Return 0 not ENOPROTOOPT to refelect the flash feature
> - Output prompt information
> ---
> drivers/mtd/spi/sf_probe.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index f461082e03..995801817d 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -109,6 +109,11 @@ static int spi_flash_std_write(struct udevice *dev, u32
> offset, size_t len,
> struct mtd_info *mtd = &flash->mtd;
> size_t retlen;
>
> + if (flash->flash_is_locked && flash->flash_is_locked(flash, offset,
> len)) {
> + printf("SF: Flash is locked\n");
>
> I would use a debug message, it's a flash specific thing. Also, I would update
> a bit the message, something like
> "SF: Flash has protected areas in the requested length. Writes will be
> ignored on those."
>
> + return 0;
>
> Michael has suggested to drop this line. I agree with him, check the
> conversation
> on the previous email thread.
>
> Cheers,
> ta
>
> + }
> +
> return mtd->_write(mtd, offset, len, &retlen, buf);
> }
>
> @@ -127,6 +132,11 @@ static int spi_flash_std_erase(struct udevice *dev, u32
> offset, size_t len)
> instr.addr = offset;
> instr.len = len;
>
> + if (flash->flash_is_locked && flash->flash_is_locked(flash, offset,
> len)) {
> + printf("SF: Flash is locked\n");
> + return 0;
> + }
> +
> return mtd->_erase(mtd, &instr);
> }
>
> --
> 2.33.1
>
>
>
> the background is we like to use sf command to operate the flash under uboot
> shell,
>
> "sf erase" command still would show the prompt "erase ok" even though
> write-enable has enabled.
>
>
> So at the beginning I'd like to return an error ,so the sf operation would
> show "erase failed" when operating the write-enabled devices.
>
>
> I'm agree with only output information to prompt the user the operation
> unsuccessful.
>
> But It should explicitly give clear hints,so I suggest at here using printf
> not debug.
We cannot encourage sf to show non operational prints like locked or
unlocked on command line. Just check the contents via read and compare
and understand whether flash is written properly, if not written
properly user has to debug on his own.
Jagan.
