[PATCH 0/2] Uniform way to pass a NAND ID table to the core

[PATCH 0/2] Uniform way to pass a NAND ID table to the core

[Miquel Raynal]

Hello,

As part of the work of migrating all the drivers to nand_scan(), and
because nand_scan() does not provide a way to pass any ID table, create
a field in NAND chip so a controller driver can expose its ID table to
the core through it and stop using the last parameter of
nand_scan_ident() for that.

Apply this to the sm_common driver which is the only one that actually
provides its own ID table, so there is no remaining driver using this
third parameter, so all can be moved easily to nand_scan() in a next
series.

Thanks,
Miquèl

Miquel Raynal (2):
  mtd: rawnand: add a field in nand_chip to fill an array of IDs
  mtd: rawnand: sm_common: make use of the new flash_ids table entry

 drivers/mtd/nand/raw/nand_base.c | 8 ++++++--
 drivers/mtd/nand/raw/sm_common.c | 5 +++--
 include/linux/mtd/rawnand.h      | 3 +++
 3 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.14.1

[PATCH 1/2] mtd: rawnand: add a field in nand_chip to fill an array of IDs

[Miquel Raynal]

Add an alternate way to give to the core another ID table to later
remove the one given in nand_scan_ident() as part of the migration to
nand_scan().

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/nand_base.c | 8 ++++++--
 include/linux/mtd/rawnand.h      | 3 +++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 9e151e4c7570..d652c692bb1b 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5598,8 +5598,12 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
 	manufacturer = nand_get_manufacturer(maf_id);
 	chip->manufacturer.desc = manufacturer;
 
-	if (!type)
-		type = nand_flash_ids;
+	if (!type) {
+		if (chip->flash_ids)
+			type = chip->flash_ids;
+		else
+			type = nand_flash_ids;
+	}
 
 	/*
 	 * Save the NAND_BUSWIDTH_16 flag before letting auto-detection logic
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 89675cb9c79f..04b6b99cfb99 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1215,6 +1215,7 @@ int nand_op_parser_exec_op(struct nand_chip *chip,
  *			  chipnr is set to %NAND_DATA_IFACE_CHECK_ONLY this
  *			  means the configuration should not be applied but
  *			  only checked.
+ * @flash_ids:		[OPTIONAL] local NAND flash ID table
  * @bbt:		[INTERN] bad block table pointer
  * @bbt_td:		[REPLACEABLE] bad block table descriptor for flash
  *			lookup.
@@ -1301,6 +1302,8 @@ struct nand_chip {
 	unsigned long buf_align;
 	struct nand_hw_control hwcontrol;
 
+	struct nand_flash_dev *flash_ids;
+
 	uint8_t *bbt;
 	struct nand_bbt_descr *bbt_td;
 	struct nand_bbt_descr *bbt_md;
-- 
2.14.1

[PATCH 2/2] mtd: rawnand: sm_common: make use of the new flash_ids table entry

[Miquel Raynal]

Use the new entry in nand_chip to fill a local ID table instead of the
traditional way through nand_scan_ident() as part of the transition to
use nand_scan() in every driver.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/raw/sm_common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/sm_common.c b/drivers/mtd/nand/raw/sm_common.c
index 7f5044a79f01..ac93fae5ed27 100644
--- a/drivers/mtd/nand/raw/sm_common.c
+++ b/drivers/mtd/nand/raw/sm_common.c
@@ -168,8 +168,9 @@ int sm_register_device(struct mtd_info *mtd, int smartmedia)
 	chip->options |= NAND_SKIP_BBTSCAN;
 
 	/* Scan for card properties */
-	ret = nand_scan_ident(mtd, 1, smartmedia ?
-		nand_smartmedia_flash_ids : nand_xd_flash_ids);
+	chip->flash_ids = smartmedia ? nand_smartmedia_flash_ids :
+				       nand_xd_flash_ids;
+	ret = nand_scan_ident(mtd, 1, NULL);
 
 	if (ret)
 		return ret;
-- 
2.14.1

Re: [PATCH 0/2] Uniform way to pass a NAND ID table to the core

[Boris Brezillon]

Hi Miquel,

On Wed, 21 Mar 2018 14:12:33 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hello,
> 
> As part of the work of migrating all the drivers to nand_scan(), and
> because nand_scan() does not provide a way to pass any ID table, create
> a field in NAND chip so a controller driver can expose its ID table to
> the core through it and stop using the last parameter of
> nand_scan_ident() for that.

I'm not a big fan of adding yet another field to the nand_chip struct.
Could we instead create a nand_scan_with_ids() function that takes an
extra ID table in argument and then make nand_scan() a wrapper around
this new function?

int nand_scan_with_ids(struct mtd_info *mtd, int max_chips,
		       struct nand_flash_dev *ids);

static inline int nand_scan(struct mtd_info *mtd, int max_chips)
{
	return nand_scan_with_ids(mtd, max_chips, NULL);
}

Regards,

Boris

> 
> Apply this to the sm_common driver which is the only one that actually
> provides its own ID table, so there is no remaining driver using this
> third parameter, so all can be moved easily to nand_scan() in a next
> series.
> 
> Thanks,
> Miquèl
> 
> Miquel Raynal (2):
>   mtd: rawnand: add a field in nand_chip to fill an array of IDs
>   mtd: rawnand: sm_common: make use of the new flash_ids table entry
> 
>  drivers/mtd/nand/raw/nand_base.c | 8 ++++++--
>  drivers/mtd/nand/raw/sm_common.c | 5 +++--
>  include/linux/mtd/rawnand.h      | 3 +++
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 



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

Re: [PATCH 0/2] Uniform way to pass a NAND ID table to the core

[Miquel Raynal]

Hi Boris,

On Tue, 27 Mar 2018 09:17:37 +0200, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> Hi Miquel,
> 
> On Wed, 21 Mar 2018 14:12:33 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hello,
> > 
> > As part of the work of migrating all the drivers to nand_scan(), and
> > because nand_scan() does not provide a way to pass any ID table, create
> > a field in NAND chip so a controller driver can expose its ID table to
> > the core through it and stop using the last parameter of
> > nand_scan_ident() for that.  
> 
> I'm not a big fan of adding yet another field to the nand_chip struct.
> Could we instead create a nand_scan_with_ids() function that takes an
> extra ID table in argument and then make nand_scan() a wrapper around
> this new function?
> 
> int nand_scan_with_ids(struct mtd_info *mtd, int max_chips,
> 		       struct nand_flash_dev *ids);
> 
> static inline int nand_scan(struct mtd_info *mtd, int max_chips)
> {
> 	return nand_scan_with_ids(mtd, max_chips, NULL);
> }

Sure I can do that, but I found too impacting the fact that we should
patch all the drivers to call nand_scan_with_ids() from your example.

I added another exported function called nand_scan_with_ids() but
nand_scan() is still visible so that I only have to patch one driver.

You'll tell me what you think.

Thanks,
Miquèl

> 
> Regards,
> 
> Boris
> 
> > 
> > Apply this to the sm_common driver which is the only one that actually
> > provides its own ID table, so there is no remaining driver using this
> > third parameter, so all can be moved easily to nand_scan() in a next
> > series.
> > 
> > Thanks,
> > Miquèl
> > 
> > Miquel Raynal (2):
> >   mtd: rawnand: add a field in nand_chip to fill an array of IDs
> >   mtd: rawnand: sm_common: make use of the new flash_ids table entry
> > 
> >  drivers/mtd/nand/raw/nand_base.c | 8 ++++++--
> >  drivers/mtd/nand/raw/sm_common.c | 5 +++--
> >  include/linux/mtd/rawnand.h      | 3 +++
> >  3 files changed, 12 insertions(+), 4 deletions(-)
> >   
> 
> 
> 



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

Re: [PATCH 0/2] Uniform way to pass a NAND ID table to the core

[Boris Brezillon]

On Sat, 21 Apr 2018 20:19:00 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> On Tue, 27 Mar 2018 09:17:37 +0200, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> 
> > Hi Miquel,
> > 
> > On Wed, 21 Mar 2018 14:12:33 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hello,
> > > 
> > > As part of the work of migrating all the drivers to nand_scan(), and
> > > because nand_scan() does not provide a way to pass any ID table, create
> > > a field in NAND chip so a controller driver can expose its ID table to
> > > the core through it and stop using the last parameter of
> > > nand_scan_ident() for that.    
> > 
> > I'm not a big fan of adding yet another field to the nand_chip struct.
> > Could we instead create a nand_scan_with_ids() function that takes an
> > extra ID table in argument and then make nand_scan() a wrapper around
> > this new function?
> > 
> > int nand_scan_with_ids(struct mtd_info *mtd, int max_chips,
> > 		       struct nand_flash_dev *ids);
> > 
> > static inline int nand_scan(struct mtd_info *mtd, int max_chips)
> > {
> > 	return nand_scan_with_ids(mtd, max_chips, NULL);
> > }  
> 
> Sure I can do that, but I found too impacting the fact that we should
> patch all the drivers to call nand_scan_with_ids() from your example.

Nope we don't have to do that, see the nand_scan() wrapper proposed
above.

> 
> I added another exported function called nand_scan_with_ids() but
> nand_scan() is still visible so that I only have to patch one driver.
> 
> You'll tell me what you think.

That's exactly what I suggested, so I think we're good ;-).

Re: [PATCH 0/2] Uniform way to pass a NAND ID table to the core

[Miquel Raynal]

Hi Boris,

On Sat, 21 Apr 2018 21:28:53 +0200, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:

> On Sat, 21 Apr 2018 20:19:00 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > On Tue, 27 Mar 2018 09:17:37 +0200, Boris Brezillon
> > <boris.brezillon@bootlin.com> wrote:
> >   
> > > Hi Miquel,
> > > 
> > > On Wed, 21 Mar 2018 14:12:33 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > Hello,
> > > > 
> > > > As part of the work of migrating all the drivers to nand_scan(), and
> > > > because nand_scan() does not provide a way to pass any ID table, create
> > > > a field in NAND chip so a controller driver can expose its ID table to
> > > > the core through it and stop using the last parameter of
> > > > nand_scan_ident() for that.      
> > > 
> > > I'm not a big fan of adding yet another field to the nand_chip struct.
> > > Could we instead create a nand_scan_with_ids() function that takes an
> > > extra ID table in argument and then make nand_scan() a wrapper around
> > > this new function?
> > > 
> > > int nand_scan_with_ids(struct mtd_info *mtd, int max_chips,
> > > 		       struct nand_flash_dev *ids);
> > > 
> > > static inline int nand_scan(struct mtd_info *mtd, int max_chips)
> > > {
> > > 	return nand_scan_with_ids(mtd, max_chips, NULL);
> > > }    
> > 
> > Sure I can do that, but I found too impacting the fact that we should
> > patch all the drivers to call nand_scan_with_ids() from your example.  
> 
> Nope we don't have to do that, see the nand_scan() wrapper proposed
> above.

Actually I was embarrassed by the 'static' keyword, I'll just drop
it off.

> 
> > 
> > I added another exported function called nand_scan_with_ids() but
> > nand_scan() is still visible so that I only have to patch one driver.
> > 
> > You'll tell me what you think.  
> 
> That's exactly what I suggested, so I think we're good ;-).

Re: [PATCH 0/2] Uniform way to pass a NAND ID table to the core

[Boris Brezillon]

On Sun, 22 Apr 2018 03:36:00 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> On Sat, 21 Apr 2018 21:28:53 +0200, Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> 
> > On Sat, 21 Apr 2018 20:19:00 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Boris,
> > > 
> > > On Tue, 27 Mar 2018 09:17:37 +0200, Boris Brezillon
> > > <boris.brezillon@bootlin.com> wrote:
> > >     
> > > > Hi Miquel,
> > > > 
> > > > On Wed, 21 Mar 2018 14:12:33 +0100
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >       
> > > > > Hello,
> > > > > 
> > > > > As part of the work of migrating all the drivers to nand_scan(), and
> > > > > because nand_scan() does not provide a way to pass any ID table, create
> > > > > a field in NAND chip so a controller driver can expose its ID table to
> > > > > the core through it and stop using the last parameter of
> > > > > nand_scan_ident() for that.        
> > > > 
> > > > I'm not a big fan of adding yet another field to the nand_chip struct.
> > > > Could we instead create a nand_scan_with_ids() function that takes an
> > > > extra ID table in argument and then make nand_scan() a wrapper around
> > > > this new function?
> > > > 
> > > > int nand_scan_with_ids(struct mtd_info *mtd, int max_chips,
> > > > 		       struct nand_flash_dev *ids);
> > > > 
> > > > static inline int nand_scan(struct mtd_info *mtd, int max_chips)
> > > > {
> > > > 	return nand_scan_with_ids(mtd, max_chips, NULL);
> > > > }      
> > > 
> > > Sure I can do that, but I found too impacting the fact that we should
> > > patch all the drivers to call nand_scan_with_ids() from your example.    
> > 
> > Nope we don't have to do that, see the nand_scan() wrapper proposed
> > above.  
> 
> Actually I was embarrassed by the 'static' keyword, I'll just drop
> it off.

It's "static inline" and that's because I was suggesting to define the
nand_scan() wrapper directly in rawnand.h instead of exporting a symbol
for a one line function.