* [PATCH v2] sf: Querying write-protect status before operating the flash @ 2021-11-15 2:37 chaochao2021666 2021-11-15 5:57 ` Tudor.Ambarus 0 siblings, 1 reply; 8+ messages in thread From: chaochao2021666 @ 2021-11-15 2:37 UTC (permalink / raw) To: jagan, vigneshr, tudor.ambarus Cc: chao.zeng, jan.kiszka, trini, u-boot, baocheng.su, le.jin From: chao zeng <chao.zeng@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.zeng@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"); + return 0; + } + 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sf: Querying write-protect status before operating the flash 2021-11-15 2:37 [PATCH v2] sf: Querying write-protect status before operating the flash chaochao2021666 @ 2021-11-15 5:57 ` Tudor.Ambarus 2021-11-15 13:20 ` chaochao2021666 0 siblings, 1 reply; 8+ messages in thread From: Tudor.Ambarus @ 2021-11-15 5:57 UTC (permalink / raw) To: chaochao2021666, jagan, vigneshr, michael Cc: chao.zeng, jan.kiszka, trini, u-boot, baocheng.su, le.jin Hi, + Michael On 11/15/21 4:37 AM, chaochao2021666@163.com wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > From: chao zeng <chao.zeng@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.zeng@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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sf: Querying write-protect status before operating the flash 2021-11-15 5:57 ` Tudor.Ambarus @ 2021-11-15 13:20 ` chaochao2021666 2021-11-15 14:02 ` Jagan Teki 0 siblings, 1 reply; 8+ messages in thread From: chaochao2021666 @ 2021-11-15 13:20 UTC (permalink / raw) To: Tudor.Ambarus, jagan, vigneshr, michael Cc: chao.zeng, jan.kiszka, trini, u-boot, baocheng.su, le.jin, 464759471 On 2021/11/15 13:57, Tudor.Ambarus@microchip.com wrote: > Hi, > > + Michael > > On 11/15/21 4:37 AM,chaochao2021666@163.com wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> From: chao zeng<chao.zeng@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.zeng@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 onlyoutput information to prompt the user the operation unsuccessful. But It should explicitly giveclear hints,so I suggest at here using printf not debug. BRs Chao ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sf: Querying write-protect status before operating the flash 2021-11-15 13:20 ` chaochao2021666 @ 2021-11-15 14:02 ` Jagan Teki 2021-11-15 14:18 ` Tom Rini 2021-11-16 8:41 ` chaochao 0 siblings, 2 replies; 8+ messages in thread From: Jagan Teki @ 2021-11-15 14:02 UTC (permalink / raw) To: chaochao2021666 Cc: Tudor.Ambarus, vigneshr, michael, chao.zeng, jan.kiszka, trini, u-boot, baocheng.su, le.jin, 464759471 On Mon, Nov 15, 2021 at 6:51 PM chaochao2021666@163.com <chaochao2021666@163.com> wrote: > > On 2021/11/15 13:57, Tudor.Ambarus@microchip.com wrote: > > Hi, > > + Michael > > On 11/15/21 4:37 AM, chaochao2021666@163.com wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > From: chao zeng <chao.zeng@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.zeng@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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sf: Querying write-protect status before operating the flash 2021-11-15 14:02 ` Jagan Teki @ 2021-11-15 14:18 ` Tom Rini 2021-11-15 14:49 ` Jagan Teki 2021-11-16 8:41 ` chaochao 1 sibling, 1 reply; 8+ messages in thread From: Tom Rini @ 2021-11-15 14:18 UTC (permalink / raw) To: Jagan Teki Cc: chaochao2021666, Tudor.Ambarus, vigneshr, michael, chao.zeng, jan.kiszka, u-boot, baocheng.su, le.jin, 464759471 [-- Attachment #1: Type: text/plain, Size: 3597 bytes --] On Mon, Nov 15, 2021 at 07:32:29PM +0530, Jagan Teki wrote: > On Mon, Nov 15, 2021 at 6:51 PM chaochao2021666@163.com > <chaochao2021666@163.com> wrote: > > > > On 2021/11/15 13:57, Tudor.Ambarus@microchip.com wrote: > > > > Hi, > > > > + Michael > > > > On 11/15/21 4:37 AM, chaochao2021666@163.com wrote: > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > From: chao zeng <chao.zeng@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.zeng@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. Wait, what? Is there a separate subcommand to use, or that needs to be written perhaps, to query if a given area/chip is locked? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sf: Querying write-protect status before operating the flash 2021-11-15 14:18 ` Tom Rini @ 2021-11-15 14:49 ` Jagan Teki 0 siblings, 0 replies; 8+ messages in thread From: Jagan Teki @ 2021-11-15 14:49 UTC (permalink / raw) To: Tom Rini Cc: chaochao2021666, Tudor.Ambarus, vigneshr, michael, chao.zeng, jan.kiszka, u-boot, baocheng.su, le.jin, 464759471 On Mon, Nov 15, 2021 at 7:48 PM Tom Rini <trini@konsulko.com> wrote: > > On Mon, Nov 15, 2021 at 07:32:29PM +0530, Jagan Teki wrote: > > On Mon, Nov 15, 2021 at 6:51 PM chaochao2021666@163.com > > <chaochao2021666@163.com> wrote: > > > > > > On 2021/11/15 13:57, Tudor.Ambarus@microchip.com wrote: > > > > > > Hi, > > > > > > + Michael > > > > > > On 11/15/21 4:37 AM, chaochao2021666@163.com wrote: > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > > > From: chao zeng <chao.zeng@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.zeng@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. > > Wait, what? Is there a separate subcommand to use, or that needs to be > written perhaps, to query if a given area/chip is locked? 'sf protect' ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sf: Querying write-protect status before operating the flash 2021-11-15 14:02 ` Jagan Teki 2021-11-15 14:18 ` Tom Rini @ 2021-11-16 8:41 ` chaochao 2021-11-16 20:10 ` Tom Rini 1 sibling, 1 reply; 8+ messages in thread From: chaochao @ 2021-11-16 8:41 UTC (permalink / raw) To: Jagan Teki Cc: Tudor.Ambarus, vigneshr, michael, chao.zeng, jan.kiszka, trini, u-boot, baocheng.su, le.jin, 464759471 On 2021/11/15 22:02, Jagan Teki wrote: > On Mon, Nov 15, 2021 at 6:51 PM chaochao2021666@163.com > <chaochao2021666@163.com> wrote: >> >> On 2021/11/15 13:57, Tudor.Ambarus@microchip.com wrote: >> >> Hi, >> >> + Michael >> >> On 11/15/21 4:37 AM, chaochao2021666@163.com wrote: >> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> From: chao zeng <chao.zeng@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.zeng@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. > I think it’s not user friendly at all. Using debug is not a problem for developers, but it is not so good for users who use the sf command. BRs Chao ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sf: Querying write-protect status before operating the flash 2021-11-16 8:41 ` chaochao @ 2021-11-16 20:10 ` Tom Rini 0 siblings, 0 replies; 8+ messages in thread From: Tom Rini @ 2021-11-16 20:10 UTC (permalink / raw) To: chaochao Cc: Jagan Teki, Tudor.Ambarus, vigneshr, michael, chao.zeng, jan.kiszka, u-boot, baocheng.su, le.jin, 464759471 [-- Attachment #1: Type: text/plain, Size: 4270 bytes --] On Tue, Nov 16, 2021 at 04:41:46PM +0800, chaochao wrote: > > On 2021/11/15 22:02, Jagan Teki wrote: > > On Mon, Nov 15, 2021 at 6:51 PM chaochao2021666@163.com > > <chaochao2021666@163.com> wrote: > > > > > > On 2021/11/15 13:57, Tudor.Ambarus@microchip.com wrote: > > > > > > Hi, > > > > > > + Michael > > > > > > On 11/15/21 4:37 AM, chaochao2021666@163.com wrote: > > > > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > > > From: chao zeng <chao.zeng@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.zeng@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. > > I think it’s not user friendly at all. Using debug is not a problem for > developers, but it is not so good for users who use the sf command. Yes, I think I agree that this is the kind of information that since it is easily available to us at run time should be printed to the user when things fail. We don't document and expect people to start with running "sf protect" to check the status of writing to a specific area nor do we tell people that all writes should be read back and verified in the normal case. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-11-16 20:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-15 2:37 [PATCH v2] sf: Querying write-protect status before operating the flash chaochao2021666 2021-11-15 5:57 ` Tudor.Ambarus 2021-11-15 13:20 ` chaochao2021666 2021-11-15 14:02 ` Jagan Teki 2021-11-15 14:18 ` Tom Rini 2021-11-15 14:49 ` Jagan Teki 2021-11-16 8:41 ` chaochao 2021-11-16 20:10 ` Tom Rini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.