[PATCH 3/4] mtd: spinand: Enabled support to detect parameter page

[PATCH 3/4] mtd: spinand: Enabled support to detect parameter page

[Shivamurthy Shastri (sshivamurthy)]

Some of the SPI NAND devices has parameter page which is similar to ONFI
table.

But, it may not be self sufficient to propagate all the required
parameters. Fixup function has been added in struct manufacturer to
accommodate this.

Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
---
 drivers/mtd/nand/spi/core.c | 113 +++++++++++++++++++++++++++++++++++-
 include/linux/mtd/spinand.h |   5 ++
 2 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index 985ad52cdaa7..40882a1d2bc1 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -574,6 +574,108 @@ static int spinand_lock_block(struct spinand_device *spinand, u8 lock)
 	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
 }
 
+/**
+ * spinand_read_param_page_op - Read parameter page operation
+ * @spinand: the spinand device
+ * @page: page number where parameter page tables can be found
+ * @parameters: buffer used to store the parameter page
+ * @len: length of the buffer
+ *
+ * Read parameter page
+ *
+ * Returns 0 on success, a negative error code otherwise.
+ */
+static int spinand_parameter_page_read(struct nand_device *base,
+				       u8 page, void *buf, unsigned int len)
+{
+	struct spinand_device *spinand = nand_to_spinand(base);
+	struct spi_mem_op pread_op = SPINAND_PAGE_READ_OP(page);
+	struct spi_mem_op pread_cache_op =
+				SPINAND_PAGE_READ_FROM_CACHE_OP(false,
+								0,
+								1,
+								buf,
+								len);
+	u8 feature;
+	u8 status;
+	int ret;
+
+	if (len && !buf)
+		return -EINVAL;
+
+	ret = spinand_read_reg_op(spinand, REG_CFG,
+				  &feature);
+	if (ret)
+		return ret;
+
+	/* CFG_OTP_ENABLE is used to enable parameter page access */
+	feature |= CFG_OTP_ENABLE;
+
+	spinand_write_reg_op(spinand, REG_CFG, feature);
+
+	ret = spi_mem_exec_op(spinand->spimem, &pread_op);
+	if (ret)
+		return ret;
+
+	ret = spinand_wait(spinand, &status);
+	if (ret < 0)
+		return ret;
+
+	ret = spi_mem_exec_op(spinand->spimem, &pread_cache_op);
+	if (ret)
+		return ret;
+
+	ret = spinand_read_reg_op(spinand, REG_CFG,
+				  &feature);
+	if (ret)
+		return ret;
+
+	feature &= ~CFG_OTP_ENABLE;
+
+	spinand_write_reg_op(spinand, REG_CFG, feature);
+
+	return 1;
+}
+
+static int check_version(struct nand_device *base,
+			 struct nand_onfi_params *p, int *onfi_version)
+{
+	/**
+	 * SPI NAND do not support ONFI standard
+	 * But, parameter page looks same as ONFI table
+	 */
+	if (!le16_to_cpu(p->revision))
+		*onfi_version = 0;
+
+	return 1;
+}
+
+static int spinand_intf_data(struct nand_device *base,
+			     struct nand_onfi_params *p)
+{
+	struct spinand_device *spinand = nand_to_spinand(base);
+
+	/**
+	 *	Manufacturers may interpret the parameter page differently
+	 */
+	if (spinand->manufacturer->ops->fixup_param_page)
+		spinand->manufacturer->ops->fixup_param_page(spinand, p);
+
+	return 1;
+}
+
+static int spinand_param_page_detect(struct spinand_device *spinand)
+{
+	struct nand_device *base = spinand_to_nand(spinand);
+
+	base->helper.page = 0x01;
+	base->helper.check_revision = check_version;
+	base->helper.parameter_page_read = spinand_parameter_page_read;
+	base->helper.init_intf_data = spinand_intf_data;
+
+	return nand_onfi_detect(base);
+}
+
 static int spinand_read_page(struct spinand_device *spinand,
 			     const struct nand_page_io_req *req)
 {
@@ -896,7 +998,7 @@ static void spinand_manufacturer_cleanup(struct spinand_device *spinand)
 		return spinand->manufacturer->ops->cleanup(spinand);
 }
 
-static const struct spi_mem_op *
+const struct spi_mem_op *
 spinand_select_op_variant(struct spinand_device *spinand,
 			  const struct spinand_op_variants *variants)
 {
@@ -1012,6 +1114,15 @@ static int spinand_detect(struct spinand_device *spinand)
 		return ret;
 	}
 
+	if (!spinand->base.memorg.pagesize) {
+		ret = spinand_param_page_detect(spinand);
+		if (ret < 0) {
+			dev_err(dev, "no parameter page for %*phN\n",
+				SPINAND_MAX_ID_LEN, spinand->id.data);
+			return ret;
+		}
+	}
+
 	if (nand->memorg.ntargets > 1 && !spinand->select_target) {
 		dev_err(dev,
 			"SPI NANDs with more than one die must implement ->select_target()\n");
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index d093d237fba8..57b3b5b075f2 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -179,6 +179,8 @@ struct spinand_manufacturer_ops {
 	int (*detect)(struct spinand_device *spinand);
 	int (*init)(struct spinand_device *spinand);
 	void (*cleanup)(struct spinand_device *spinand);
+	void (*fixup_param_page)(struct spinand_device *spinand,
+				 struct nand_onfi_params *p);
 };
 
 /**
@@ -429,4 +431,7 @@ int spinand_match_and_init(struct spinand_device *dev,
 int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
 int spinand_select_target(struct spinand_device *spinand, unsigned int target);
 
+const struct spi_mem_op *spinand_select_op_variant(struct spinand_device *spinand,
+	   const struct spinand_op_variants *variants);
+
 #endif /* __LINUX_MTD_SPINAND_H */
-- 
2.17.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/4] mtd: spinand: Enabled support to detect parameter page

[Miquel Raynal]

Hi Shivamurthy,

"Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote on
Tue, 26 Mar 2019 10:52:00 +0000:

> Some of the SPI NAND devices has parameter page which is similar to ONFI
> table.
> 
> But, it may not be self sufficient to propagate all the required
> parameters. Fixup function has been added in struct manufacturer to
> accommodate this.
> 
> Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> ---
>  drivers/mtd/nand/spi/core.c | 113 +++++++++++++++++++++++++++++++++++-
>  include/linux/mtd/spinand.h |   5 ++
>  2 files changed, 117 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 985ad52cdaa7..40882a1d2bc1 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -574,6 +574,108 @@ static int spinand_lock_block(struct spinand_device *spinand, u8 lock)
>  	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
>  }
>  
> +/**
> + * spinand_read_param_page_op - Read parameter page operation
> + * @spinand: the spinand device
> + * @page: page number where parameter page tables can be found
> + * @parameters: buffer used to store the parameter page

Does not match the prototype

> + * @len: length of the buffer
> + *
> + * Read parameter page
> + *
> + * Returns 0 on success, a negative error code otherwise.
> + */
> +static int spinand_parameter_page_read(struct nand_device *base,

Please use a spinand structure as parameter, you don't need a
nand_device here (same for other spinand functions).

> +				       u8 page, void *buf, unsigned int len)
> +{
> +	struct spinand_device *spinand = nand_to_spinand(base);
> +	struct spi_mem_op pread_op = SPINAND_PAGE_READ_OP(page);
> +	struct spi_mem_op pread_cache_op =
> +				SPINAND_PAGE_READ_FROM_CACHE_OP(false,
> +								0,
> +								1,
> +								buf,
> +								len);
> +	u8 feature;
> +	u8 status;
> +	int ret;
> +
> +	if (len && !buf)
> +		return -EINVAL;
> +
> +	ret = spinand_read_reg_op(spinand, REG_CFG,
> +				  &feature);
> +	if (ret)
> +		return ret;
> +
> +	/* CFG_OTP_ENABLE is used to enable parameter page access */
> +	feature |= CFG_OTP_ENABLE;
> +
> +	spinand_write_reg_op(spinand, REG_CFG, feature);
> +
> +	ret = spi_mem_exec_op(spinand->spimem, &pread_op);
> +	if (ret)
> +		return ret;
> +
> +	ret = spinand_wait(spinand, &status);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = spi_mem_exec_op(spinand->spimem, &pread_cache_op);
> +	if (ret)
> +		return ret;
> +
> +	ret = spinand_read_reg_op(spinand, REG_CFG,
> +				  &feature);
> +	if (ret)
> +		return ret;
> +
> +	feature &= ~CFG_OTP_ENABLE;
> +
> +	spinand_write_reg_op(spinand, REG_CFG, feature);
> +
> +	return 1;

Should return 0

> +}
> +
> +static int check_version(struct nand_device *base,
> +			 struct nand_onfi_params *p, int *onfi_version)
> +{
> +	/**

I don't think you need these /** here, just use /*

> +	 * SPI NAND do not support ONFI standard

What about:

        /*
         * SPI NANDs do not necessarily support ONFI standard, but
         * parameter page looks the same as an ONFI table.
         */

> +	 * But, parameter page looks same as ONFI table
> +	 */
> +	if (!le16_to_cpu(p->revision))
> +		*onfi_version = 0;
> +
> +	return 1;

Functions should return 0 in normal state, not 1.

> +}
> +
> +static int spinand_intf_data(struct nand_device *base,
> +			     struct nand_onfi_params *p)
> +{
> +	struct spinand_device *spinand = nand_to_spinand(base);
> +
> +	/**
> +	 *	Manufacturers may interpret the parameter page differently

           ^^^^^ extra spaces

> +	 */

One-line comment should be in the form /* <comment> */

> +	if (spinand->manufacturer->ops->fixup_param_page)
> +		spinand->manufacturer->ops->fixup_param_page(spinand, p);
> +
> +	return 1;

        return 0;

> +}
> +
> +static int spinand_param_page_detect(struct spinand_device *spinand)
> +{
> +	struct nand_device *base = spinand_to_nand(spinand);
> +
> +	base->helper.page = 0x01;
> +	base->helper.check_revision = check_version;
> +	base->helper.parameter_page_read = spinand_parameter_page_read;
> +	base->helper.init_intf_data = spinand_intf_data;
> +
> +	return nand_onfi_detect(base);
> +}
> +
>  static int spinand_read_page(struct spinand_device *spinand,
>  			     const struct nand_page_io_req *req)
>  {
> @@ -896,7 +998,7 @@ static void spinand_manufacturer_cleanup(struct spinand_device *spinand)
>  		return spinand->manufacturer->ops->cleanup(spinand);
>  }
>  
> -static const struct spi_mem_op *
> +const struct spi_mem_op *
>  spinand_select_op_variant(struct spinand_device *spinand,
>  			  const struct spinand_op_variants *variants)
>  {
> @@ -1012,6 +1114,15 @@ static int spinand_detect(struct spinand_device *spinand)
>  		return ret;
>  	}
>  
> +	if (!spinand->base.memorg.pagesize) {
> +		ret = spinand_param_page_detect(spinand);
> +		if (ret < 0) {
> +			dev_err(dev, "no parameter page for %*phN\n",
> +				SPINAND_MAX_ID_LEN, spinand->id.data);
> +			return ret;
> +		}
> +	}
> +

I think this could be added in a separate patch. Is this the only
condition where we want to read the param page ?

>  	if (nand->memorg.ntargets > 1 && !spinand->select_target) {
>  		dev_err(dev,
>  			"SPI NANDs with more than one die must implement ->select_target()\n");
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index d093d237fba8..57b3b5b075f2 100644
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -179,6 +179,8 @@ struct spinand_manufacturer_ops {
>  	int (*detect)(struct spinand_device *spinand);
>  	int (*init)(struct spinand_device *spinand);
>  	void (*cleanup)(struct spinand_device *spinand);
> +	void (*fixup_param_page)(struct spinand_device *spinand,
> +				 struct nand_onfi_params *p);

You could probably add this hook in a separate patch.

>  };
>  
>  /**
> @@ -429,4 +431,7 @@ int spinand_match_and_init(struct spinand_device *dev,
>  int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
>  int spinand_select_target(struct spinand_device *spinand, unsigned int target);
>  
> +const struct spi_mem_op *spinand_select_op_variant(struct spinand_device *spinand,
> +	   const struct spinand_op_variants *variants);

What is it for?

> +
>  #endif /* __LINUX_MTD_SPINAND_H */




Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [EXT] Re: [PATCH 3/4] mtd: spinand: Enabled support to detect parameter page

[Shivamurthy Shastri (sshivamurthy)]

Hi Miquel,

> 
> Hi Shivamurthy,
> 
> "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote
> on
> Tue, 26 Mar 2019 10:52:00 +0000:
> 
> > Some of the SPI NAND devices has parameter page which is similar to ONFI
> > table.
> >
> > But, it may not be self sufficient to propagate all the required
> > parameters. Fixup function has been added in struct manufacturer to
> > accommodate this.
> >
> > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> > ---
> >  drivers/mtd/nand/spi/core.c | 113
> +++++++++++++++++++++++++++++++++++-
> >  include/linux/mtd/spinand.h |   5 ++
> >  2 files changed, 117 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > index 985ad52cdaa7..40882a1d2bc1 100644
> > --- a/drivers/mtd/nand/spi/core.c
> > +++ b/drivers/mtd/nand/spi/core.c
> > @@ -574,6 +574,108 @@ static int spinand_lock_block(struct
> spinand_device *spinand, u8 lock)
> >  	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
> >  }
> >
> > +/**
> > + * spinand_read_param_page_op - Read parameter page operation
> > + * @spinand: the spinand device
> > + * @page: page number where parameter page tables can be found
> > + * @parameters: buffer used to store the parameter page
> 
> Does not match the prototype

I will fix this in next version.

> 
> > + * @len: length of the buffer
> > + *
> > + * Read parameter page
> > + *
> > + * Returns 0 on success, a negative error code otherwise.
> > + */
> > +static int spinand_parameter_page_read(struct nand_device *base,
> 
> Please use a spinand structure as parameter, you don't need a
> nand_device here (same for other spinand functions).

This function is helper function for generic ONFI layer.
From generic ONFI layer, I can get only nand_device.

> 
> > +				       u8 page, void *buf, unsigned int len)
> > +{
> > +	struct spinand_device *spinand = nand_to_spinand(base);
> > +	struct spi_mem_op pread_op = SPINAND_PAGE_READ_OP(page);
> > +	struct spi_mem_op pread_cache_op =
> > +
> 	SPINAND_PAGE_READ_FROM_CACHE_OP(false,
> > +								0,
> > +								1,
> > +								buf,
> > +								len);
> > +	u8 feature;
> > +	u8 status;
> > +	int ret;
> > +
> > +	if (len && !buf)
> > +		return -EINVAL;
> > +
> > +	ret = spinand_read_reg_op(spinand, REG_CFG,
> > +				  &feature);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/* CFG_OTP_ENABLE is used to enable parameter page access */
> > +	feature |= CFG_OTP_ENABLE;
> > +
> > +	spinand_write_reg_op(spinand, REG_CFG, feature);
> > +
> > +	ret = spi_mem_exec_op(spinand->spimem, &pread_op);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = spinand_wait(spinand, &status);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = spi_mem_exec_op(spinand->spimem, &pread_cache_op);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = spinand_read_reg_op(spinand, REG_CFG,
> > +				  &feature);
> > +	if (ret)
> > +		return ret;
> > +
> > +	feature &= ~CFG_OTP_ENABLE;
> > +
> > +	spinand_write_reg_op(spinand, REG_CFG, feature);
> > +
> > +	return 1;
> 
> Should return 0

I will fix this in next version.

> 
> > +}
> > +
> > +static int check_version(struct nand_device *base,
> > +			 struct nand_onfi_params *p, int *onfi_version)
> > +{
> > +	/**
> 
> I don't think you need these /** here, just use /*

I will fix this in next version.

> 
> > +	 * SPI NAND do not support ONFI standard
> 
> What about:
> 
>         /*
>          * SPI NANDs do not necessarily support ONFI standard, but
>          * parameter page looks the same as an ONFI table.
>          */
> 

This looks better, I will use this.

> > +	 * But, parameter page looks same as ONFI table
> > +	 */
> > +	if (!le16_to_cpu(p->revision))
> > +		*onfi_version = 0;
> > +
> > +	return 1;
> 
> Functions should return 0 in normal state, not 1.
> 

I will fix this in next version.

> > +}
> > +
> > +static int spinand_intf_data(struct nand_device *base,
> > +			     struct nand_onfi_params *p)
> > +{
> > +	struct spinand_device *spinand = nand_to_spinand(base);
> > +
> > +	/**
> > +	 *	Manufacturers may interpret the parameter page differently
> 
>            ^^^^^ extra spaces
> 

I will fix this in next version.

> > +	 */
> 
> One-line comment should be in the form /* <comment> */
> 
> > +	if (spinand->manufacturer->ops->fixup_param_page)
> > +		spinand->manufacturer->ops->fixup_param_page(spinand,
> p);
> > +
> > +	return 1;
> 
>         return 0;

I will fix this in next version.

> 
> > +}
> > +
> > +static int spinand_param_page_detect(struct spinand_device *spinand)
> > +{
> > +	struct nand_device *base = spinand_to_nand(spinand);
> > +
> > +	base->helper.page = 0x01;
> > +	base->helper.check_revision = check_version;
> > +	base->helper.parameter_page_read =
> spinand_parameter_page_read;
> > +	base->helper.init_intf_data = spinand_intf_data;
> > +
> > +	return nand_onfi_detect(base);
> > +}
> > +
> >  static int spinand_read_page(struct spinand_device *spinand,
> >  			     const struct nand_page_io_req *req)
> >  {
> > @@ -896,7 +998,7 @@ static void spinand_manufacturer_cleanup(struct
> spinand_device *spinand)
> >  		return spinand->manufacturer->ops->cleanup(spinand);
> >  }
> >
> > -static const struct spi_mem_op *
> > +const struct spi_mem_op *
> >  spinand_select_op_variant(struct spinand_device *spinand,
> >  			  const struct spinand_op_variants *variants)
> >  {
> > @@ -1012,6 +1114,15 @@ static int spinand_detect(struct spinand_device
> *spinand)
> >  		return ret;
> >  	}
> >
> > +	if (!spinand->base.memorg.pagesize) {
> > +		ret = spinand_param_page_detect(spinand);
> > +		if (ret < 0) {
> > +			dev_err(dev, "no parameter page for %*phN\n",
> > +				SPINAND_MAX_ID_LEN, spinand->id.data);
> > +			return ret;
> > +		}
> > +	}
> > +
> 
> I think this could be added in a separate patch. Is this the only
> condition where we want to read the param page ?
> 

I will make separate patch if possible.
spinand_manufacturer_detect will not initialize memorg, I think it is 
enough to check memorg.pagesize is initialized or not.
Suggest me if you have any other thoughts.

> >  	if (nand->memorg.ntargets > 1 && !spinand->select_target) {
> >  		dev_err(dev,
> >  			"SPI NANDs with more than one die must implement
> ->select_target()\n");
> > diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> > index d093d237fba8..57b3b5b075f2 100644
> > --- a/include/linux/mtd/spinand.h
> > +++ b/include/linux/mtd/spinand.h
> > @@ -179,6 +179,8 @@ struct spinand_manufacturer_ops {
> >  	int (*detect)(struct spinand_device *spinand);
> >  	int (*init)(struct spinand_device *spinand);
> >  	void (*cleanup)(struct spinand_device *spinand);
> > +	void (*fixup_param_page)(struct spinand_device *spinand,
> > +				 struct nand_onfi_params *p);
> 
> You could probably add this hook in a separate patch.

I will fix this in new version.

> 
> >  };
> >
> >  /**
> > @@ -429,4 +431,7 @@ int spinand_match_and_init(struct spinand_device
> *dev,
> >  int spinand_upd_cfg(struct spinand_device *spinand, u8 mask, u8 val);
> >  int spinand_select_target(struct spinand_device *spinand, unsigned int
> target);
> >
> > +const struct spi_mem_op *spinand_select_op_variant(struct
> spinand_device *spinand,
> > +	   const struct spinand_op_variants *variants);
> 
> What is it for?

Ah, it should be in next patch no here.
 
> 
> > +
> >  #endif /* __LINUX_MTD_SPINAND_H */
> 
> 
> 
> 
> Thanks,
> Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [EXT] Re: [PATCH 3/4] mtd: spinand: Enabled support to detect parameter page

[Miquel Raynal]

Hi Shiva,

"Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote on
Thu, 2 May 2019 11:37:10 +0000:

> Hi Miquel,
> 
> > 
> > Hi Shivamurthy,
> > 
> > "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote
> > on
> > Tue, 26 Mar 2019 10:52:00 +0000:
> >   
> > > Some of the SPI NAND devices has parameter page which is similar to ONFI
> > > table.
> > >
> > > But, it may not be self sufficient to propagate all the required
> > > parameters. Fixup function has been added in struct manufacturer to
> > > accommodate this.
> > >
> > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> > > ---
> > >  drivers/mtd/nand/spi/core.c | 113  
> > +++++++++++++++++++++++++++++++++++-  
> > >  include/linux/mtd/spinand.h |   5 ++
> > >  2 files changed, 117 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > > index 985ad52cdaa7..40882a1d2bc1 100644
> > > --- a/drivers/mtd/nand/spi/core.c
> > > +++ b/drivers/mtd/nand/spi/core.c
> > > @@ -574,6 +574,108 @@ static int spinand_lock_block(struct  
> > spinand_device *spinand, u8 lock)  
> > >  	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
> > >  }
> > >
> > > +/**
> > > + * spinand_read_param_page_op - Read parameter page operation
> > > + * @spinand: the spinand device
> > > + * @page: page number where parameter page tables can be found
> > > + * @parameters: buffer used to store the parameter page  
> > 
> > Does not match the prototype  
> 
> I will fix this in next version.
> 
> >   
> > > + * @len: length of the buffer
> > > + *
> > > + * Read parameter page
> > > + *
> > > + * Returns 0 on success, a negative error code otherwise.
> > > + */
> > > +static int spinand_parameter_page_read(struct nand_device *base,  
> > 
> > Please use a spinand structure as parameter, you don't need a
> > nand_device here (same for other spinand functions).  
> 
> This function is helper function for generic ONFI layer.
> From generic ONFI layer, I can get only nand_device.

How do you handle if the SPI NAND core is not compiled-in?


Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: [EXT] Re: [PATCH 3/4] mtd: spinand: Enabled support to detect parameter page

[Shivamurthy Shastri (sshivamurthy)]

Hi Miquel,

> > >
> > > > Some of the SPI NAND devices has parameter page which is similar to
> ONFI
> > > > table.
> > > >
> > > > But, it may not be self sufficient to propagate all the required
> > > > parameters. Fixup function has been added in struct manufacturer to
> > > > accommodate this.
> > > >
> > > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com>
> > > > ---
> > > >  drivers/mtd/nand/spi/core.c | 113
> > > +++++++++++++++++++++++++++++++++++-
> > > >  include/linux/mtd/spinand.h |   5 ++
> > > >  2 files changed, 117 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> > > > index 985ad52cdaa7..40882a1d2bc1 100644
> > > > --- a/drivers/mtd/nand/spi/core.c
> > > > +++ b/drivers/mtd/nand/spi/core.c
> > > > @@ -574,6 +574,108 @@ static int spinand_lock_block(struct
> > > spinand_device *spinand, u8 lock)
> > > >  	return spinand_write_reg_op(spinand, REG_BLOCK_LOCK, lock);
> > > >  }
> > > >
> > > > +/**
> > > > + * spinand_read_param_page_op - Read parameter page operation
> > > > + * @spinand: the spinand device
> > > > + * @page: page number where parameter page tables can be found
> > > > + * @parameters: buffer used to store the parameter page
> > >
> > > Does not match the prototype
> >
> > I will fix this in next version.
> >
> > >
> > > > + * @len: length of the buffer
> > > > + *
> > > > + * Read parameter page
> > > > + *
> > > > + * Returns 0 on success, a negative error code otherwise.
> > > > + */
> > > > +static int spinand_parameter_page_read(struct nand_device *base,
> > >
> > > Please use a spinand structure as parameter, you don't need a
> > > nand_device here (same for other spinand functions).
> >
> > This function is helper function for generic ONFI layer.
> > From generic ONFI layer, I can get only nand_device.
> 
> How do you handle if the SPI NAND core is not compiled-in?
> 

Both raw NAND and SPI NAND define parameter_page_read function,
which will be called in nand_onfi_detect.

Rightly you pointed, I will add the following lines in nand_onfi_detect
to tackle if those functions are not compiled-in.

        /* return 0, if ONFI helper functions are not defined */                 
        if (!base->helper.parameter_page_read &&                                 
            !base->helper.check_revision &&                                      
            !base->helper.init_intf_data)                                        
                return 0;

I hope this answers your point.

Thanks,
Shiva
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/