[PATCH 0/2] fix regressions in DoC G4 driver

[PATCH 0/2] fix regressions in DoC G4 driver

[Sergey Larin]

docg4 driver became broken and these patches will make it work again.

Sergey Larin (2):
  mtd: rawnand: docg4: fix NULL deref while probing
  mtd: rawnand: docg4: specify bits_per_cell

 drivers/mtd/nand/raw/docg4.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.18.0

[PATCH 1/2] mtd: rawnand: docg4: fix NULL deref while probing

[Sergey Larin]

nand_scan_tail() invokes nand_chip->scan_bbt() at the end, which is not set
by the driver. Use the default nand_default_bbt() function to avoid NULL
dereferncing.

Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>
---
 drivers/mtd/nand/raw/docg4.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
index bb96cb33cd6b..bbed8ea7858c 100644
--- a/drivers/mtd/nand/raw/docg4.c
+++ b/drivers/mtd/nand/raw/docg4.c
@@ -1269,6 +1269,7 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
 	nand->read_buf = docg4_read_buf;
 	nand->write_buf = docg4_write_buf16;
 	nand->erase = docg4_erase_block;
+	nand->scan_bbt = nand_default_bbt;
 	nand->set_features = nand_get_set_features_notsupp;
 	nand->get_features = nand_get_set_features_notsupp;
 	nand->ecc.read_page = docg4_read_page;
-- 
2.18.0

Re: [PATCH 1/2] mtd: rawnand: docg4: fix NULL deref while probing

[Miquel Raynal]

Hi Sergey,

Sergey Larin <cerg2010cerg2010@mail.ru> wrote on Sun,  8 Jul 2018
14:29:23 +0300:

> nand_scan_tail() invokes nand_chip->scan_bbt() at the end, which is not set
> by the driver. Use the default nand_default_bbt() function to avoid NULL
> dereferncing.

dereferencing

> 
> Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>

Maybe a Fixes/stable tags would be appropriate here if this is a real
runtime error?

> ---
>  drivers/mtd/nand/raw/docg4.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> index bb96cb33cd6b..bbed8ea7858c 100644
> --- a/drivers/mtd/nand/raw/docg4.c
> +++ b/drivers/mtd/nand/raw/docg4.c
> @@ -1269,6 +1269,7 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
>  	nand->read_buf = docg4_read_buf;
>  	nand->write_buf = docg4_write_buf16;
>  	nand->erase = docg4_erase_block;
> +	nand->scan_bbt = nand_default_bbt;
>  	nand->set_features = nand_get_set_features_notsupp;
>  	nand->get_features = nand_get_set_features_notsupp;
>  	nand->ecc.read_page = docg4_read_page;



-- 
Miquel Raynal, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/2] mtd: rawnand: docg4: fix NULL deref while probing

[Boris Brezillon]

On Sun,  8 Jul 2018 14:29:23 +0300
Sergey Larin <cerg2010cerg2010@mail.ru> wrote:

> nand_scan_tail() invokes nand_chip->scan_bbt() at the end, which is not set
> by the driver. Use the default nand_default_bbt() function to avoid NULL
> dereferncing.

Wow! For how long has this driver been broken? The ->scan_bbt() hook
has been there for a very long time, and nand_scan_tail() is calling
it when NAND_SKIP_BBTSCAN is not set. 

> 
> Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>

Missing Fixes and Cc stable tags.

> ---
>  drivers/mtd/nand/raw/docg4.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> index bb96cb33cd6b..bbed8ea7858c 100644
> --- a/drivers/mtd/nand/raw/docg4.c
> +++ b/drivers/mtd/nand/raw/docg4.c
> @@ -1269,6 +1269,7 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
>  	nand->read_buf = docg4_read_buf;
>  	nand->write_buf = docg4_write_buf16;
>  	nand->erase = docg4_erase_block;
> +	nand->scan_bbt = nand_default_bbt;

Are you sure that's really what you want. My experience with docg4 code
is that it's not really fitting in the raw NAND framework, so I
wouldn't be surprised if the default bad block table scan function does
not match how the docg4 NAND works.

>  	nand->set_features = nand_get_set_features_notsupp;
>  	nand->get_features = nand_get_set_features_notsupp;
>  	nand->ecc.read_page = docg4_read_page;

Re: [PATCH 1/2] mtd: rawnand: docg4: fix NULL deref while probing

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

Hi Sergey,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mtd/nand/next]
[also build test ERROR on v4.18-rc3 next-20180706]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sergey-Larin/fix-regressions-in-DoC-G4-driver/20180708-193220
base:   git://git.infradead.org/linux-mtd.git nand/next
config: i386-randconfig-s0-201827 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "nand_default_bbt" [drivers/mtd/nand/raw/docg4.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30772 bytes --]

[PATCH 2/2] mtd: rawnand: docg4: specify bits_per_cell

[Sergey Larin]

This fixes WARN() in nand_is_slc().

DoC G4 is 2-bit MLC, so set bits_per_cell value according to this.

Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>
---
 drivers/mtd/nand/raw/docg4.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
index bbed8ea7858c..82d97a16e246 100644
--- a/drivers/mtd/nand/raw/docg4.c
+++ b/drivers/mtd/nand/raw/docg4.c
@@ -1250,6 +1250,7 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
 	nand->pagemask = 0x3ffff;
 	nand->badblockpos = NAND_LARGE_BADBLOCK_POS;
 	nand->badblockbits = 8;
+	nand->bits_per_cell = 2;
 	nand->ecc.mode = NAND_ECC_HW_SYNDROME;
 	nand->ecc.size = DOCG4_PAGE_SIZE;
 	nand->ecc.prepad = 8;
-- 
2.18.0

Re: [PATCH 2/2] mtd: rawnand: docg4: specify bits_per_cell

[Boris Brezillon]

On Sun,  8 Jul 2018 14:29:24 +0300
Sergey Larin <cerg2010cerg2010@mail.ru> wrote:

> This fixes WARN() in nand_is_slc().
> 
> DoC G4 is 2-bit MLC, so set bits_per_cell value according to this.

What you do here implies "no UBI on docg4". To be honest, I'm pretty
sure there's an FTL in there, which clearly does not make it a good
candidate for the raw NAND framework. Not to mention that it's not even
using the same instruction set which forced developers of this driver
to guess what the core was trying to do in the ->cmdfunc()
implementation and convert it to docg4 operations. This approach is
going against the recent evolution of the raw NAND framework
(deprecating ->cmdfunc() in favor of ->exec_op()).

I'd really like to see this driver moved outside of
drivers/mtd/nand/raw/. If it really has NAND constraints, you could
re-use the interface agnostic NAND framework (include/linux/nand.h) and
add a new driver under drivers/mtd/nand/diskonchip/docg4.c.

That's the very reason I asked if you if you had access to this HW. I
can help you with the development part, but I can't test.

> 
> Signed-off-by: Sergey Larin <cerg2010cerg2010@mail.ru>
> ---
>  drivers/mtd/nand/raw/docg4.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/mtd/nand/raw/docg4.c b/drivers/mtd/nand/raw/docg4.c
> index bbed8ea7858c..82d97a16e246 100644
> --- a/drivers/mtd/nand/raw/docg4.c
> +++ b/drivers/mtd/nand/raw/docg4.c
> @@ -1250,6 +1250,7 @@ static void __init init_mtd_structs(struct mtd_info *mtd)
>  	nand->pagemask = 0x3ffff;
>  	nand->badblockpos = NAND_LARGE_BADBLOCK_POS;
>  	nand->badblockbits = 8;
> +	nand->bits_per_cell = 2;
>  	nand->ecc.mode = NAND_ECC_HW_SYNDROME;
>  	nand->ecc.size = DOCG4_PAGE_SIZE;
>  	nand->ecc.prepad = 8;

Re: [PATCH 0/2] fix regressions in DoC G4 driver

[Boris Brezillon]

Hi Sergey,

On Sun,  8 Jul 2018 14:29:22 +0300
Sergey Larin <cerg2010cerg2010@mail.ru> wrote:

> docg4 driver became broken and these patches will make it work again.

I guess you have a platform with a DoC G4 on it. If that's the case, I
might have a few changes I'd need you to test if you don't mind.

Thanks,

Boris 

> 
> Sergey Larin (2):
>   mtd: rawnand: docg4: fix NULL deref while probing
>   mtd: rawnand: docg4: specify bits_per_cell
> 
>  drivers/mtd/nand/raw/docg4.c | 2 ++
>  1 file changed, 2 insertions(+)
>