RE: [UBOOT PATCH v3] mtd: nand: arasan: Print warning for unsupported ecc modes

2024-03-01 Thread Abbarapu, Venkatesh
Hi Michal,

> -Original Message-
> From: Simek, Michal 
> Sent: Friday, March 1, 2024 4:55 PM
> To: Abbarapu, Venkatesh ; u-
> b...@lists.denx.de
> Cc: dario.binac...@amarulasolutions.com; mich...@amarulasolutions.com;
> git (AMD-Xilinx) ; Ashok Reddy Soma
> 
> Subject: Re: [UBOOT PATCH v3] mtd: nand: arasan: Print warning for
> unsupported ecc modes
> 
> 
> 
> On 3/1/24 12:12, Venkatesh Yadav Abbarapu wrote:
> > Currently only hw ecc is supported in U-Boot. If any other ecc mode is
> > given in DT, it simply ignores and switches to hw ecc. So better print
> > what is being done.
> >
> > Revert this patch once soft ecc support is fixed in future.
> >
> > Signed-off-by: Ashok Reddy Soma 
> > Signed-off-by: Venkatesh Yadav Abbarapu
> 
> > ---
> > Changes in v2:
> > - Added the ecc mode check in the arasan driver itself.
> > Changes in v3:
> > - Skip the driver probe when sw-ecc mode is present in the device tree.
> > ---
> >   drivers/mtd/nand/raw/arasan_nfc.c | 7 +++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/arasan_nfc.c
> > b/drivers/mtd/nand/raw/arasan_nfc.c
> > index 14766401bf..426160c384 100644
> > --- a/drivers/mtd/nand/raw/arasan_nfc.c
> > +++ b/drivers/mtd/nand/raw/arasan_nfc.c
> > @@ -1233,6 +1233,7 @@ static int arasan_probe(struct udevice *dev)
> > struct mtd_info *mtd;
> > ofnode child;
> > int err = -1;
> > +   const char *str;
> >
> > info->reg = dev_read_addr_ptr(dev);
> > mtd = nand_to_mtd(nand_chip);
> > @@ -1263,6 +1264,12 @@ static int arasan_probe(struct udevice *dev)
> > goto fail;
> > }
> >
> > +   str = ofnode_read_string(nand_chip->flash_node, "nand-ecc-mode");
> > +   if (strcmp(str, "hw")) {
> > +   printf("%s ecc is not supported, switch to hw ecc\n", str);
> 
> you are returning below. It means switch to HW ecc is not happening right?
 Yes. We are not switching to HW ecc, just returning from the probe 
if it is other that HW ecc.

Thanks
Venkatesh
> 
> M


Re: [UBOOT PATCH v3] mtd: nand: arasan: Print warning for unsupported ecc modes

2024-03-01 Thread Dan Carpenter
On Fri, Mar 01, 2024 at 04:42:54PM +0530, Venkatesh Yadav Abbarapu wrote:
> @@ -1263,6 +1264,12 @@ static int arasan_probe(struct udevice *dev)
>   goto fail;
>   }
>  
> + str = ofnode_read_string(nand_chip->flash_node, "nand-ecc-mode");
> + if (strcmp(str, "hw")) {
> + printf("%s ecc is not supported, switch to hw ecc\n", str);

I'm a newbie to device trees, but the two other callers assume that it's
possible for ofnode_read_string(nand->flash_node, "nand-ecc-mode") to
return NULL so probably we should add a NULL check here too.

if (!str || strcmp(str, "hw") != 0) {

regards,
dan carpenter



Re: [UBOOT PATCH v3] mtd: nand: arasan: Print warning for unsupported ecc modes

2024-03-01 Thread Michal Simek




On 3/1/24 12:12, Venkatesh Yadav Abbarapu wrote:

Currently only hw ecc is supported in U-Boot. If any other ecc mode is
given in DT, it simply ignores and switches to hw ecc. So better print
what is being done.

Revert this patch once soft ecc support is fixed in future.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
Changes in v2:
- Added the ecc mode check in the arasan driver itself.
Changes in v3:
- Skip the driver probe when sw-ecc mode is present in the device tree.
---
  drivers/mtd/nand/raw/arasan_nfc.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/nand/raw/arasan_nfc.c 
b/drivers/mtd/nand/raw/arasan_nfc.c
index 14766401bf..426160c384 100644
--- a/drivers/mtd/nand/raw/arasan_nfc.c
+++ b/drivers/mtd/nand/raw/arasan_nfc.c
@@ -1233,6 +1233,7 @@ static int arasan_probe(struct udevice *dev)
struct mtd_info *mtd;
ofnode child;
int err = -1;
+   const char *str;
  
  	info->reg = dev_read_addr_ptr(dev);

mtd = nand_to_mtd(nand_chip);
@@ -1263,6 +1264,12 @@ static int arasan_probe(struct udevice *dev)
goto fail;
}
  
+	str = ofnode_read_string(nand_chip->flash_node, "nand-ecc-mode");

+   if (strcmp(str, "hw")) {
+   printf("%s ecc is not supported, switch to hw ecc\n", str);


you are returning below. It means switch to HW ecc is not happening right?

M