Re: [PATCH] mtd: rawnand: Add a check in of_get_nand_secure_regions()

[Martin Kaiser <martin@kaiser.cx>]

Hi all,

Thus wrote Miquel Raynal (miquel.raynal@bootlin.com):

> On Thu, 2021-06-17 at 13:37:25 UTC, Dan Carpenter wrote:
> > Check for whether of_property_count_elems_of_size() returns a negative
> > error code.

> > Fixes: 13b89768275d ("mtd: rawnand: Add support for secure regions in NAND memory")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

I'm running linux-next on an imx25 system with the following flash chip

[    1.997539] nand: device found, Manufacturer ID: 0x98, Chip ID: 0xaa
[    2.004134] nand: Toshiba NAND 256MiB 1,8V 8-bit
[    2.008917] nand: 256 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 128

The system is using the drivers/mtd/nand/raw/mxc_nand.c driver.

Since this commit appeared in linux-next, mxc_nand's probe function fails
with -EINVAL, taking this path

mxcnd_probe
   nand_scan
      nand_scan_with_ids
         nand_scan_tail
            of_get_nand_secure_regions

nr_elem = of_property_count_elems_of_size(dn, "secure-regions", sizeof(u64));
returns -EINVAL as there's no secure-regions property in my device tree.

We should certainly handle negative error codes before we calculate
chip->nr_secure_regions = nr_elem / 2
but a missing secure-regions property is a valid case and should not make
the probe fail.

If the property exists, but the device-tree entry is incorrect
and of_property_count_elems_of_size returns -ENODATA, we might print a
warning and ignore the entry.

What do you think?

Thanks,

   Martin