diff mbox series

[3/4] mtd: spinand: Add M70A series Micron SPI NAND devices

Message ID 20200119145432.10405-4-sshivamurthy@micron.com
State Changes Requested
Delegated to: Miquel Raynal
Headers show
Series Add new series Micron SPI NAND devices | expand

Commit Message

Shivamurthy Shastri Jan. 19, 2020, 2:54 p.m. UTC
From: Shivamurthy Shastri <sshivamurthy@micron.com>

Add device table for M70A series Micron SPI NAND devices.

While at it, disable the Continuous Read feature which is enabled by
default.

Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
---
 drivers/mtd/nand/spi/micron.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

Comments

Miquel Raynal Jan. 20, 2020, 10:16 a.m. UTC | #1
Hi Shiva,

This is remark common to the four patches: you miss the 'v2' prefix in
the object.

shiva.linuxworks@gmail.com wrote on Sun, 19 Jan 2020 15:54:31 +0100:

> From: Shivamurthy Shastri <sshivamurthy@micron.com>
> 
> Add device table for M70A series Micron SPI NAND devices.
> 
> While at it, disable the Continuous Read feature which is enabled by
> default.

Can you please give us more detail on why this is an issue?

Shall we backport it to stable?

As a rule of thumb, when you start a sentence by "while at it" in a
commit message and this is not a trivial change : split the patch,
please. Unless this is really related and in this case explain how and
why in the commit message.

> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>  drivers/mtd/nand/spi/micron.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
> index 5fd1f921ef12..45fc37c58f8a 100644
> --- a/drivers/mtd/nand/spi/micron.c
> +++ b/drivers/mtd/nand/spi/micron.c
> @@ -131,6 +131,26 @@ static const struct spinand_info micron_spinand_table[] = {
>  		     0,
>  		     SPINAND_ECCINFO(&micron_8_ooblayout,
>  				     micron_8_ecc_get_status)),
> +	/* M70A 4Gb 3.3V */
> +	SPINAND_INFO("MT29F4G01ABAFD", 0x34,
> +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> +				     micron_8_ecc_get_status)),
> +	/* M70A 4Gb 1.8V */
> +	SPINAND_INFO("MT29F4G01ABBFD", 0x35,
> +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> +		     NAND_ECCREQ(8, 512),
> +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> +					      &write_cache_variants,
> +					      &update_cache_variants),
> +		     0,
> +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> +				     micron_8_ecc_get_status)),
>  };
>  
>  static int micron_spinand_detect(struct spinand_device *spinand)
> @@ -153,8 +173,19 @@ static int micron_spinand_detect(struct spinand_device *spinand)
>  	return 1;
>  }
>  
> +static int micron_spinand_init(struct spinand_device *spinand)
> +{
> +	/*
> +	 * M70A device series enable Continuous Read feature at Power-up,
> +	 * which is not supported. Disable this bit to avoid any possible
> +	 * failure.
> +	 */
> +	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE, 0);
> +}
> +
>  static const struct spinand_manufacturer_ops micron_spinand_manuf_ops = {
>  	.detect = micron_spinand_detect,
> +	.init = micron_spinand_init,
>  };
>  
>  const struct spinand_manufacturer micron_spinand_manufacturer = {

Thanks,
Miquèl
Shivamurthy Shastri (sshivamurthy) Jan. 21, 2020, 12:23 p.m. UTC | #2
Hi Miquel,

> 
> Hi Shiva,
> 
> This is remark common to the four patches: you miss the 'v2' prefix in
> the object.
> 

Sorry for this mistake.
I recognized this after sending out the patches.

> shiva.linuxworks@gmail.com wrote on Sun, 19 Jan 2020 15:54:31 +0100:
> 
> > From: Shivamurthy Shastri <sshivamurthy@micron.com>
> >
> > Add device table for M70A series Micron SPI NAND devices.
> >
> > While at it, disable the Continuous Read feature which is enabled by
> > default.
> 
> Can you please give us more detail on why this is an issue?

"Continuous Read" is the new feature added by the Micron for 
M70A series devices. If this feature is enabled, the READ command 
doesn't output the OOB area. The following short description
describes this feature.

Description:
If the Continuous Read feature is enabled, the device provides 
the capability to read the whole block with a single command.
However, the read command doesn't output the OOB area.

Read command behavior (if Continuous Read enabled):
The READ CACHE command doesn't require the starting column address.
The device always output the data starting from the first column of the
cache register, and once the end of the cache register reached, the data
output continues through the next page. With the continuous read mode,
it is possible to read out the entire block using a single READ command, and
once the end of the block reached, the output pins become High-Z state.

> 
> Shall we backport it to stable?

This is not a bug fix and applicable only to M70A series devices, there is no
need to backport.
(FYI, the previously enabled device was M79A series)

> 
> As a rule of thumb, when you start a sentence by "while at it" in a
> commit message and this is not a trivial change : split the patch,
> please. Unless this is really related and in this case explain how and
> why in the commit message.

Okay, I will explain in my next version.

> 
> >
> > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> > ---
> >  drivers/mtd/nand/spi/micron.c | 31
> +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/spi/micron.c
> b/drivers/mtd/nand/spi/micron.c
> > index 5fd1f921ef12..45fc37c58f8a 100644
> > --- a/drivers/mtd/nand/spi/micron.c
> > +++ b/drivers/mtd/nand/spi/micron.c
> > @@ -131,6 +131,26 @@ static const struct spinand_info
> micron_spinand_table[] = {
> >  		     0,
> >  		     SPINAND_ECCINFO(&micron_8_ooblayout,
> >  				     micron_8_ecc_get_status)),
> > +	/* M70A 4Gb 3.3V */
> > +	SPINAND_INFO("MT29F4G01ABAFD", 0x34,
> > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > +		     NAND_ECCREQ(8, 512),
> > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > +					      &write_cache_variants,
> > +					      &update_cache_variants),
> > +		     0,
> > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > +				     micron_8_ecc_get_status)),
> > +	/* M70A 4Gb 1.8V */
> > +	SPINAND_INFO("MT29F4G01ABBFD", 0x35,
> > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > +		     NAND_ECCREQ(8, 512),
> > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > +					      &write_cache_variants,
> > +					      &update_cache_variants),
> > +		     0,
> > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > +				     micron_8_ecc_get_status)),
> >  };
> >
> >  static int micron_spinand_detect(struct spinand_device *spinand)
> > @@ -153,8 +173,19 @@ static int micron_spinand_detect(struct
> spinand_device *spinand)
> >  	return 1;
> >  }
> >
> > +static int micron_spinand_init(struct spinand_device *spinand)
> > +{
> > +	/*
> > +	 * M70A device series enable Continuous Read feature at Power-up,
> > +	 * which is not supported. Disable this bit to avoid any possible
> > +	 * failure.
> > +	 */
> > +	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE, 0);
> > +}
> > +
> >  static const struct spinand_manufacturer_ops
> micron_spinand_manuf_ops = {
> >  	.detect = micron_spinand_detect,
> > +	.init = micron_spinand_init,
> >  };
> >
> >  const struct spinand_manufacturer micron_spinand_manufacturer = {
> 
> Thanks,
> Miquèl

Thanks,
Shiva
Miquel Raynal Jan. 21, 2020, 1:40 p.m. UTC | #3
Hi Shivamurthy,

"Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote on
Tue, 21 Jan 2020 12:23:20 +0000:

> Hi Miquel,
> 
> > 
> > Hi Shiva,
> > 
> > This is remark common to the four patches: you miss the 'v2' prefix in
> > the object.
> >   
> 
> Sorry for this mistake.
> I recognized this after sending out the patches.
> 
> > shiva.linuxworks@gmail.com wrote on Sun, 19 Jan 2020 15:54:31 +0100:
> >   
> > > From: Shivamurthy Shastri <sshivamurthy@micron.com>
> > >
> > > Add device table for M70A series Micron SPI NAND devices.
> > >
> > > While at it, disable the Continuous Read feature which is enabled by
> > > default.  
> > 
> > Can you please give us more detail on why this is an issue?  
> 
> "Continuous Read" is the new feature added by the Micron for 
> M70A series devices. If this feature is enabled, the READ command 
> doesn't output the OOB area. The following short description
> describes this feature.
> 
> Description:
> If the Continuous Read feature is enabled, the device provides 
> the capability to read the whole block with a single command.
> However, the read command doesn't output the OOB area.
> 
> Read command behavior (if Continuous Read enabled):
> The READ CACHE command doesn't require the starting column address.
> The device always output the data starting from the first column of the
> cache register, and once the end of the cache register reached, the data
> output continues through the next page. With the continuous read mode,
> it is possible to read out the entire block using a single READ command, and
> once the end of the block reached, the output pins become High-Z state.

Ok I understand better. In this case there is no need to split this
commit, instead just reword the commit log to something like:

--->8---
Add device table for M70A series Micron SPI-NAND devices.

As opposed to the M60A series already supported, M70A parts have the
"Continuous Read" feature enabled by default which does not fit the
subsystem needs.

<here explain the feature>.

Hence, we disable the feature at probe time.
---8<---

However, below, you disable this bit for all the parts. Is this really
ok? Souldn't we make it more specific to this series?

> 
> > 
> > Shall we backport it to stable?  
> 
> This is not a bug fix and applicable only to M70A series devices, there is no
> need to backport.
> (FYI, the previously enabled device was M79A series)
> 
> > 
> > As a rule of thumb, when you start a sentence by "while at it" in a
> > commit message and this is not a trivial change : split the patch,
> > please. Unless this is really related and in this case explain how and
> > why in the commit message.  
> 
> Okay, I will explain in my next version.
> 
> >   
> > >
> > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> > > ---
> > >  drivers/mtd/nand/spi/micron.c | 31  
> > +++++++++++++++++++++++++++++++  
> > >  1 file changed, 31 insertions(+)
> > >
> > > diff --git a/drivers/mtd/nand/spi/micron.c  
> > b/drivers/mtd/nand/spi/micron.c  
> > > index 5fd1f921ef12..45fc37c58f8a 100644
> > > --- a/drivers/mtd/nand/spi/micron.c
> > > +++ b/drivers/mtd/nand/spi/micron.c
> > > @@ -131,6 +131,26 @@ static const struct spinand_info  
> > micron_spinand_table[] = {  
> > >  		     0,
> > >  		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > >  				     micron_8_ecc_get_status)),
> > > +	/* M70A 4Gb 3.3V */
> > > +	SPINAND_INFO("MT29F4G01ABAFD", 0x34,
> > > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > > +		     NAND_ECCREQ(8, 512),
> > > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > > +					      &write_cache_variants,
> > > +					      &update_cache_variants),
> > > +		     0,
> > > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > > +				     micron_8_ecc_get_status)),
> > > +	/* M70A 4Gb 1.8V */
> > > +	SPINAND_INFO("MT29F4G01ABBFD", 0x35,
> > > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > > +		     NAND_ECCREQ(8, 512),
> > > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > > +					      &write_cache_variants,
> > > +					      &update_cache_variants),
> > > +		     0,
> > > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > > +				     micron_8_ecc_get_status)),
> > >  };
> > >
> > >  static int micron_spinand_detect(struct spinand_device *spinand)
> > > @@ -153,8 +173,19 @@ static int micron_spinand_detect(struct  
> > spinand_device *spinand)  
> > >  	return 1;
> > >  }
> > >
> > > +static int micron_spinand_init(struct spinand_device *spinand)
> > > +{
> > > +	/*
> > > +	 * M70A device series enable Continuous Read feature at Power-up,
> > > +	 * which is not supported. Disable this bit to avoid any possible
> > > +	 * failure.
> > > +	 */
> > > +	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE, 0);
> > > +}
> > > +
> > >  static const struct spinand_manufacturer_ops  
> > micron_spinand_manuf_ops = {  
> > >  	.detect = micron_spinand_detect,
> > > +	.init = micron_spinand_init,
> > >  };
> > >
> > >  const struct spinand_manufacturer micron_spinand_manufacturer = {  
> > 
> > Thanks,
> > Miquèl  
> 
> Thanks,
> Shiva




Thanks,
Miquèl
Shivamurthy Shastri (sshivamurthy) Jan. 21, 2020, 2:59 p.m. UTC | #4
Hi Miquel,

> 
> Hi Shivamurthy,
> 
> "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote
> on
> Tue, 21 Jan 2020 12:23:20 +0000:
> 
> > Hi Miquel,
> >
> > >
> > > Hi Shiva,
> > >
> > > This is remark common to the four patches: you miss the 'v2' prefix in
> > > the object.
> > >
> >
> > Sorry for this mistake.
> > I recognized this after sending out the patches.
> >
> > > shiva.linuxworks@gmail.com wrote on Sun, 19 Jan 2020 15:54:31 +0100:
> > >
> > > > From: Shivamurthy Shastri <sshivamurthy@micron.com>
> > > >
> > > > Add device table for M70A series Micron SPI NAND devices.
> > > >
> > > > While at it, disable the Continuous Read feature which is enabled by
> > > > default.
> > >
> > > Can you please give us more detail on why this is an issue?
> >
> > "Continuous Read" is the new feature added by the Micron for
> > M70A series devices. If this feature is enabled, the READ command
> > doesn't output the OOB area. The following short description
> > describes this feature.
> >
> > Description:
> > If the Continuous Read feature is enabled, the device provides
> > the capability to read the whole block with a single command.
> > However, the read command doesn't output the OOB area.
> >
> > Read command behavior (if Continuous Read enabled):
> > The READ CACHE command doesn't require the starting column address.
> > The device always output the data starting from the first column of the
> > cache register, and once the end of the cache register reached, the data
> > output continues through the next page. With the continuous read mode,
> > it is possible to read out the entire block using a single READ command, and
> > once the end of the block reached, the output pins become High-Z state.
> 
> Ok I understand better. In this case there is no need to split this
> commit, instead just reword the commit log to something like:
> 
> --->8---
> Add device table for M70A series Micron SPI-NAND devices.
> 
> As opposed to the M60A series already supported, M70A parts have the
> "Continuous Read" feature enabled by default which does not fit the
> subsystem needs.
> 
> <here explain the feature>.
> 
> Hence, we disable the feature at probe time.
> ---8<---
> 

Sure, I will change as per your suggestion.

> However, below, you disable this bit for all the parts. Is this really
> ok? Souldn't we make it more specific to this series?

It is ok because this bit is unused in other series.

> 
> >
> > >
> > > Shall we backport it to stable?
> >
> > This is not a bug fix and applicable only to M70A series devices, there is no
> > need to backport.
> > (FYI, the previously enabled device was M79A series)
> >
> > >
> > > As a rule of thumb, when you start a sentence by "while at it" in a
> > > commit message and this is not a trivial change : split the patch,
> > > please. Unless this is really related and in this case explain how and
> > > why in the commit message.
> >
> > Okay, I will explain in my next version.
> >
> > >
> > > >
> > > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> > > > ---
> > > >  drivers/mtd/nand/spi/micron.c | 31
> > > +++++++++++++++++++++++++++++++
> > > >  1 file changed, 31 insertions(+)
> > > >
> > > > diff --git a/drivers/mtd/nand/spi/micron.c
> > > b/drivers/mtd/nand/spi/micron.c
> > > > index 5fd1f921ef12..45fc37c58f8a 100644
> > > > --- a/drivers/mtd/nand/spi/micron.c
> > > > +++ b/drivers/mtd/nand/spi/micron.c
> > > > @@ -131,6 +131,26 @@ static const struct spinand_info
> > > micron_spinand_table[] = {
> > > >  		     0,
> > > >  		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > > >  				     micron_8_ecc_get_status)),
> > > > +	/* M70A 4Gb 3.3V */
> > > > +	SPINAND_INFO("MT29F4G01ABAFD", 0x34,
> > > > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > > > +		     NAND_ECCREQ(8, 512),
> > > > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > > > +					      &write_cache_variants,
> > > > +					      &update_cache_variants),
> > > > +		     0,
> > > > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > > > +				     micron_8_ecc_get_status)),
> > > > +	/* M70A 4Gb 1.8V */
> > > > +	SPINAND_INFO("MT29F4G01ABBFD", 0x35,
> > > > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > > > +		     NAND_ECCREQ(8, 512),
> > > > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > > > +					      &write_cache_variants,
> > > > +					      &update_cache_variants),
> > > > +		     0,
> > > > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > > > +				     micron_8_ecc_get_status)),
> > > >  };
> > > >
> > > >  static int micron_spinand_detect(struct spinand_device *spinand)
> > > > @@ -153,8 +173,19 @@ static int micron_spinand_detect(struct
> > > spinand_device *spinand)
> > > >  	return 1;
> > > >  }
> > > >
> > > > +static int micron_spinand_init(struct spinand_device *spinand)
> > > > +{
> > > > +	/*
> > > > +	 * M70A device series enable Continuous Read feature at Power-up,
> > > > +	 * which is not supported. Disable this bit to avoid any possible
> > > > +	 * failure.
> > > > +	 */
> > > > +	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE, 0);
> > > > +}
> > > > +
> > > >  static const struct spinand_manufacturer_ops
> > > micron_spinand_manuf_ops = {
> > > >  	.detect = micron_spinand_detect,
> > > > +	.init = micron_spinand_init,
> > > >  };
> > > >
> > > >  const struct spinand_manufacturer micron_spinand_manufacturer = {
> > >

Thanks,
Shiva
Miquel Raynal Jan. 21, 2020, 3:27 p.m. UTC | #5
Hi Shivamurthy,

"Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote on
Tue, 21 Jan 2020 14:59:11 +0000:

> Hi Miquel,
> 
> > 
> > Hi Shivamurthy,
> > 
> > "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote
> > on
> > Tue, 21 Jan 2020 12:23:20 +0000:
> >   
> > > Hi Miquel,
> > >  
> > > >
> > > > Hi Shiva,
> > > >
> > > > This is remark common to the four patches: you miss the 'v2' prefix in
> > > > the object.
> > > >  
> > >
> > > Sorry for this mistake.
> > > I recognized this after sending out the patches.
> > >  
> > > > shiva.linuxworks@gmail.com wrote on Sun, 19 Jan 2020 15:54:31 +0100:
> > > >  
> > > > > From: Shivamurthy Shastri <sshivamurthy@micron.com>
> > > > >
> > > > > Add device table for M70A series Micron SPI NAND devices.
> > > > >
> > > > > While at it, disable the Continuous Read feature which is enabled by
> > > > > default.  
> > > >
> > > > Can you please give us more detail on why this is an issue?  
> > >
> > > "Continuous Read" is the new feature added by the Micron for
> > > M70A series devices. If this feature is enabled, the READ command
> > > doesn't output the OOB area. The following short description
> > > describes this feature.
> > >
> > > Description:
> > > If the Continuous Read feature is enabled, the device provides
> > > the capability to read the whole block with a single command.
> > > However, the read command doesn't output the OOB area.
> > >
> > > Read command behavior (if Continuous Read enabled):
> > > The READ CACHE command doesn't require the starting column address.
> > > The device always output the data starting from the first column of the
> > > cache register, and once the end of the cache register reached, the data
> > > output continues through the next page. With the continuous read mode,
> > > it is possible to read out the entire block using a single READ command, and
> > > once the end of the block reached, the output pins become High-Z state.  
> > 
> > Ok I understand better. In this case there is no need to split this
> > commit, instead just reword the commit log to something like:
> >   
> > --->8---  
> > Add device table for M70A series Micron SPI-NAND devices.
> > 
> > As opposed to the M60A series already supported, M70A parts have the
> > "Continuous Read" feature enabled by default which does not fit the
> > subsystem needs.
> > 
> > <here explain the feature>.
> > 
> > Hence, we disable the feature at probe time.
> > ---8<---
> >   
> 
> Sure, I will change as per your suggestion.
> 
> > However, below, you disable this bit for all the parts. Is this really
> > ok? Souldn't we make it more specific to this series?  
> 
> It is ok because this bit is unused in other series.

I would rather prefer to avoid this kind of arrangement. Please change
this bit only for the series which has it.


Thanks,
Miquèl
Shivamurthy Shastri (sshivamurthy) Jan. 23, 2020, 9:01 a.m. UTC | #6
Hi Miquel,

> 
> Hi Shivamurthy,
> 
> "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote
> on
> Tue, 21 Jan 2020 14:59:11 +0000:
> 
> > Hi Miquel,
> >
> > >
> > > Hi Shivamurthy,
> > >
> > > "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com>
> wrote
> > > on
> > > Tue, 21 Jan 2020 12:23:20 +0000:
> > >
> > > > Hi Miquel,
> > > >
> > > > >
> > > > > Hi Shiva,
> > > > >
> > > > > This is remark common to the four patches: you miss the 'v2' prefix in
> > > > > the object.
> > > > >
> > > >
> > > > Sorry for this mistake.
> > > > I recognized this after sending out the patches.
> > > >
> > > > > shiva.linuxworks@gmail.com wrote on Sun, 19 Jan 2020 15:54:31
> +0100:
> > > > >
> > > > > > From: Shivamurthy Shastri <sshivamurthy@micron.com>
> > > > > >
> > > > > > Add device table for M70A series Micron SPI NAND devices.
> > > > > >
> > > > > > While at it, disable the Continuous Read feature which is enabled by
> > > > > > default.
> > > > >
> > > > > Can you please give us more detail on why this is an issue?
> > > >
> > > > "Continuous Read" is the new feature added by the Micron for
> > > > M70A series devices. If this feature is enabled, the READ command
> > > > doesn't output the OOB area. The following short description
> > > > describes this feature.
> > > >
> > > > Description:
> > > > If the Continuous Read feature is enabled, the device provides
> > > > the capability to read the whole block with a single command.
> > > > However, the read command doesn't output the OOB area.
> > > >
> > > > Read command behavior (if Continuous Read enabled):
> > > > The READ CACHE command doesn't require the starting column
> address.
> > > > The device always output the data starting from the first column of the
> > > > cache register, and once the end of the cache register reached, the
> data
> > > > output continues through the next page. With the continuous read
> mode,
> > > > it is possible to read out the entire block using a single READ command,
> and
> > > > once the end of the block reached, the output pins become High-Z
> state.
> > >
> > > Ok I understand better. In this case there is no need to split this
> > > commit, instead just reword the commit log to something like:
> > >
> > > --->8---
> > > Add device table for M70A series Micron SPI-NAND devices.
> > >
> > > As opposed to the M60A series already supported, M70A parts have the
> > > "Continuous Read" feature enabled by default which does not fit the
> > > subsystem needs.
> > >
> > > <here explain the feature>.
> > >
> > > Hence, we disable the feature at probe time.
> > > ---8<---
> > >
> >
> > Sure, I will change as per your suggestion.
> >
> > > However, below, you disable this bit for all the parts. Is this really
> > > ok? Souldn't we make it more specific to this series?
> >
> > It is ok because this bit is unused in other series.
> 
> I would rather prefer to avoid this kind of arrangement. Please change
> this bit only for the series which has it.

There is no other way to identify the series of the device except by using the
"flags" from the structure spinand_info. So I will proceed with this approach.

1. In spinand.h --> I will create macro

#define SPINAND_HAS_CR_FEAT_BIT         BIT(1)

2. Use this flag inside micron_spinand_init function to identify the device.


> 
> 
> Thanks,
> Miquèl

Thanks,
Shiva
diff mbox series

Patch

diff --git a/drivers/mtd/nand/spi/micron.c b/drivers/mtd/nand/spi/micron.c
index 5fd1f921ef12..45fc37c58f8a 100644
--- a/drivers/mtd/nand/spi/micron.c
+++ b/drivers/mtd/nand/spi/micron.c
@@ -131,6 +131,26 @@  static const struct spinand_info micron_spinand_table[] = {
 		     0,
 		     SPINAND_ECCINFO(&micron_8_ooblayout,
 				     micron_8_ecc_get_status)),
+	/* M70A 4Gb 3.3V */
+	SPINAND_INFO("MT29F4G01ABAFD", 0x34,
+		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&micron_8_ooblayout,
+				     micron_8_ecc_get_status)),
+	/* M70A 4Gb 1.8V */
+	SPINAND_INFO("MT29F4G01ABBFD", 0x35,
+		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
+		     NAND_ECCREQ(8, 512),
+		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
+					      &write_cache_variants,
+					      &update_cache_variants),
+		     0,
+		     SPINAND_ECCINFO(&micron_8_ooblayout,
+				     micron_8_ecc_get_status)),
 };
 
 static int micron_spinand_detect(struct spinand_device *spinand)
@@ -153,8 +173,19 @@  static int micron_spinand_detect(struct spinand_device *spinand)
 	return 1;
 }
 
+static int micron_spinand_init(struct spinand_device *spinand)
+{
+	/*
+	 * M70A device series enable Continuous Read feature at Power-up,
+	 * which is not supported. Disable this bit to avoid any possible
+	 * failure.
+	 */
+	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE, 0);
+}
+
 static const struct spinand_manufacturer_ops micron_spinand_manuf_ops = {
 	.detect = micron_spinand_detect,
+	.init = micron_spinand_init,
 };
 
 const struct spinand_manufacturer micron_spinand_manufacturer = {