[PATCH 0/3] Add support for page scope read

[PATCH 0/3] Add support for page scope read

[Md Sadre Alam]

These series of patches to support page scope read.

In QPIC v1, SW is needed to write EXEC_CMD register
for each Code word and collect any Status related to
that CW before issueing EXEC_CMD for next CW.

Page scope command, currently supported only for read
commands in bam mode, is truly a page mode command where
SW is required to issue EXEC_CMD only once for a page.
Controller HW takes care of Codeword specific details
and automatically returns status associated with each
CW to BAM pipe, dedicated for status deposition.

Md Sadre Alam (3):
  mtd: rawnand: qcom: Add support for status pipe
  mtd: rawnand: qcom: Add sg list to handle status pipe request
  mtd: rawnand: qcom: Add support for page scope read

 drivers/mtd/nand/raw/qcom_nandc.c | 125 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 119 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH 1/3] mtd: rawnand: qcom: Add support for status pipe

[Md Sadre Alam]

From QPIC V2.0 onwards there is a separate pipe
to read status of each code word, called "status" pipe.

"status" pipe will use to read CW status in case of
enhanced read mode like page scope read, multi page read.

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 04e6f7b..42c6291 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -389,6 +389,7 @@ struct qcom_nand_controller {
 			struct dma_chan *tx_chan;
 			struct dma_chan *rx_chan;
 			struct dma_chan *cmd_chan;
+			struct dma_chan *sts_chan;
 		};
 
 		/* will be used only by EBI2 for ADM DMA */
@@ -2737,6 +2738,11 @@ static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc)
 
 		if (nandc->cmd_chan)
 			dma_release_channel(nandc->cmd_chan);
+
+		if (nandc->props->qpic_v2) {
+			if (nandc->sts_chan)
+				dma_release_channel(nandc->sts_chan);
+		}
 	} else {
 		if (nandc->chan)
 			dma_release_channel(nandc->chan);
@@ -2815,6 +2821,14 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc)
 			goto unalloc;
 		}
 
+		if (nandc->props->qpic_v2) {
+			nandc->sts_chan = dma_request_slave_channel(nandc->dev, "sts");
+			if (!nandc->sts_chan) {
+				dev_err(nandc->dev, "failed to request sts channel\n");
+				return -ENODEV;
+			}
+		}
+
 		/*
 		 * Initially allocate BAM transaction to read ONFI param page.
 		 * After detecting all the devices, this BAM transaction will
-- 
2.7.4

Re: [PATCH 1/3] mtd: rawnand: qcom: Add support for status pipe

[mdalam]

On 2021-09-15 15:27, Md Sadre Alam wrote:
> From QPIC V2.0 onwards there is a separate pipe
> to read status of each code word, called "status" pipe.
> 
> "status" pipe will use to read CW status in case of
> enhanced read mode like page scope read, multi page read.
> 
> Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c
> b/drivers/mtd/nand/raw/qcom_nandc.c
> index 04e6f7b..42c6291 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -389,6 +389,7 @@ struct qcom_nand_controller {
>  			struct dma_chan *tx_chan;
>  			struct dma_chan *rx_chan;
>  			struct dma_chan *cmd_chan;
> +			struct dma_chan *sts_chan;
>  		};
> 
>  		/* will be used only by EBI2 for ADM DMA */
> @@ -2737,6 +2738,11 @@ static void qcom_nandc_unalloc(struct
> qcom_nand_controller *nandc)
> 
>  		if (nandc->cmd_chan)
>  			dma_release_channel(nandc->cmd_chan);
> +
> +		if (nandc->props->qpic_v2) {
> +			if (nandc->sts_chan)
> +				dma_release_channel(nandc->sts_chan);
> +		}
>  	} else {
>  		if (nandc->chan)
>  			dma_release_channel(nandc->chan);
> @@ -2815,6 +2821,14 @@ static int qcom_nandc_alloc(struct
> qcom_nand_controller *nandc)
>  			goto unalloc;
>  		}
> 
> +		if (nandc->props->qpic_v2) {
> +			nandc->sts_chan = dma_request_slave_channel(nandc->dev, "sts");
> +			if (!nandc->sts_chan) {
> +				dev_err(nandc->dev, "failed to request sts channel\n");
> +				return -ENODEV;
> +			}
> +		}
> +
>  		/*
>  		 * Initially allocate BAM transaction to read ONFI param page.
>  		 * After detecting all the devices, this BAM transaction will

Ping! Please provide me some updates on this patch.

Re: [PATCH 1/3] mtd: rawnand: qcom: Add support for status pipe

[Miquel Raynal]

Hello,

mdalam@codeaurora.org wrote on Tue, 28 Sep 2021 17:47:27 +0530:

> On 2021-09-15 15:27, Md Sadre Alam wrote:
> > From QPIC V2.0 onwards there is a separate pipe
> > to read status of each code word, called "status" pipe.
> > 
> > "status" pipe will use to read CW status in case of

What is a CW status?

> > enhanced read mode like page scope read, multi page read.

What is a page scope read?

> > 
> > Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> > ---
> >  drivers/mtd/nand/raw/qcom_nandc.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c
> > b/drivers/mtd/nand/raw/qcom_nandc.c
> > index 04e6f7b..42c6291 100644
> > --- a/drivers/mtd/nand/raw/qcom_nandc.c
> > +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> > @@ -389,6 +389,7 @@ struct qcom_nand_controller {
> >  			struct dma_chan *tx_chan;
> >  			struct dma_chan *rx_chan;
> >  			struct dma_chan *cmd_chan;
> > +			struct dma_chan *sts_chan;
> >  		};
> > 
> >  		/* will be used only by EBI2 for ADM DMA */
> > @@ -2737,6 +2738,11 @@ static void qcom_nandc_unalloc(struct
> > qcom_nand_controller *nandc)
> > 
> >  		if (nandc->cmd_chan)
> >  			dma_release_channel(nandc->cmd_chan);
> > +
> > +		if (nandc->props->qpic_v2) {
> > +			if (nandc->sts_chan)
> > +				dma_release_channel(nandc->sts_chan);
> > +		}
> >  	} else {
> >  		if (nandc->chan)
> >  			dma_release_channel(nandc->chan);
> > @@ -2815,6 +2821,14 @@ static int qcom_nandc_alloc(struct
> > qcom_nand_controller *nandc)
> >  			goto unalloc;
> >  		}
> > 
> > +		if (nandc->props->qpic_v2) {
> > +			nandc->sts_chan = dma_request_slave_channel(nandc->dev, "sts");
> > +			if (!nandc->sts_chan) {
> > +				dev_err(nandc->dev, "failed to request sts channel\n");
> > +				return -ENODEV;
> > +			}
> > +		}
> > +
> >  		/*
> >  		 * Initially allocate BAM transaction to read ONFI param page.
> >  		 * After detecting all the devices, this BAM transaction will  
> 
> Ping! Please provide me some updates on this patch.

I don't think you need to ping me on all your patches. This is
irritating given the time that I allocated to all your contributions so
far.

This being said, I had no particular comment regarding the
implementation of the series but giving it a second look I still don't
fully understand the goal of this "additional pipe" so, as my comments
above say, please elaborate a little bit.

Thanks,
Miquèl

Re: [PATCH 1/3] mtd: rawnand: qcom: Add support for status pipe

[Manivannan Sadhasivam]

On Wed, Sep 15, 2021 at 03:27:29PM +0530, Md Sadre Alam wrote:
> From QPIC V2.0 onwards there is a separate pipe
> to read status of each code word, called "status" pipe.
> 
> "status" pipe will use to read CW status in case of
> enhanced read mode like page scope read, multi page read.
> 

The pipe you are referring to is a DMA pipe (channel). So it should be mentioned
here.

> Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 04e6f7b..42c6291 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -389,6 +389,7 @@ struct qcom_nand_controller {
>  			struct dma_chan *tx_chan;
>  			struct dma_chan *rx_chan;
>  			struct dma_chan *cmd_chan;
> +			struct dma_chan *sts_chan;
>  		};
>  
>  		/* will be used only by EBI2 for ADM DMA */
> @@ -2737,6 +2738,11 @@ static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc)
>  
>  		if (nandc->cmd_chan)
>  			dma_release_channel(nandc->cmd_chan);
> +
> +		if (nandc->props->qpic_v2) {
> +			if (nandc->sts_chan)
> +				dma_release_channel(nandc->sts_chan);
> +		}
>  	} else {
>  		if (nandc->chan)
>  			dma_release_channel(nandc->chan);
> @@ -2815,6 +2821,14 @@ static int qcom_nandc_alloc(struct qcom_nand_controller *nandc)
>  			goto unalloc;
>  		}
>  
> +		if (nandc->props->qpic_v2) {
> +			nandc->sts_chan = dma_request_slave_channel(nandc->dev, "sts");
> +			if (!nandc->sts_chan) {
> +				dev_err(nandc->dev, "failed to request sts channel\n");
> +				return -ENODEV;
> +			}

If you are forcing the need of status pipe, then you should also update the
devicetree of relevant SoCs using the QPIC v2 controller. Else, they will fail
to probe.

Thanks,
Mani

> +		}
> +
>  		/*
>  		 * Initially allocate BAM transaction to read ONFI param page.
>  		 * After detecting all the devices, this BAM transaction will
> -- 
> 2.7.4
> 

[PATCH 2/3] mtd: rawnand: qcom: Add sg list to handle status pipe request

[Md Sadre Alam]

From QPIC V2.0 onwards there is separate pipe to read status
for each code word while reading in enhanced mode. page scope
read and multi page read.

This sgl list will be use to handle the request via status pipe
during page scope and multi page read.

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 42c6291..07448c4 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -213,6 +213,7 @@ nandc_set_reg(chip, reg,			\
 #define QPIC_PER_CW_CMD_ELEMENTS	32
 #define QPIC_PER_CW_CMD_SGL		32
 #define QPIC_PER_CW_DATA_SGL		8
+#define	QPIC_PER_CW_STS_SGL		8
 
 #define QPIC_NAND_COMPLETION_TIMEOUT	msecs_to_jiffies(2000)
 
@@ -258,6 +259,7 @@ struct bam_transaction {
 	struct bam_cmd_element *bam_ce;
 	struct scatterlist *cmd_sgl;
 	struct scatterlist *data_sgl;
+	struct scatterlist *sts_sgl;
 	u32 bam_ce_pos;
 	u32 bam_ce_start;
 	u32 cmd_sgl_pos;
@@ -266,6 +268,8 @@ struct bam_transaction {
 	u32 tx_sgl_start;
 	u32 rx_sgl_pos;
 	u32 rx_sgl_start;
+	u32 sts_sgl_pos;
+	u32 sts_sgl_start;
 	bool wait_second_completion;
 	struct completion txn_done;
 	struct dma_async_tx_descriptor *last_data_desc;
@@ -508,6 +512,8 @@ alloc_bam_transaction(struct qcom_nand_controller *nandc)
 		((sizeof(*bam_txn->bam_ce) * QPIC_PER_CW_CMD_ELEMENTS) +
 		(sizeof(*bam_txn->cmd_sgl) * QPIC_PER_CW_CMD_SGL) +
 		(sizeof(*bam_txn->data_sgl) * QPIC_PER_CW_DATA_SGL));
+	if (nandc->props->qpic_v2)
+		bam_txn_size += (sizeof(*bam_txn->sts_sgl) * QPIC_PER_CW_STS_SGL);
 
 	bam_txn_buf = devm_kzalloc(nandc->dev, bam_txn_size, GFP_KERNEL);
 	if (!bam_txn_buf)
@@ -526,6 +532,12 @@ alloc_bam_transaction(struct qcom_nand_controller *nandc)
 
 	bam_txn->data_sgl = bam_txn_buf;
 
+	if (nandc->props->qpic_v2) {
+		bam_txn_buf +=
+			sizeof(*bam_txn->sts_sgl) * QPIC_PER_CW_STS_SGL * num_cw;
+		bam_txn->sts_sgl = bam_txn_buf;
+	}
+
 	init_completion(&bam_txn->txn_done);
 
 	return bam_txn;
@@ -554,6 +566,12 @@ static void clear_bam_transaction(struct qcom_nand_controller *nandc)
 		      QPIC_PER_CW_CMD_SGL);
 	sg_init_table(bam_txn->data_sgl, nandc->max_cwperpage *
 		      QPIC_PER_CW_DATA_SGL);
+	if (nandc->props->qpic_v2) {
+		bam_txn->sts_sgl_pos = 0;
+		bam_txn->sts_sgl_start = 0;
+		sg_init_table(bam_txn->sts_sgl, nandc->max_cwperpage *
+				QPIC_PER_CW_STS_SGL);
+	}
 
 	reinit_completion(&bam_txn->txn_done);
 }
@@ -808,6 +826,12 @@ static int prepare_bam_async_desc(struct qcom_nand_controller *nandc,
 		bam_txn->tx_sgl_start = bam_txn->tx_sgl_pos;
 		dir_eng = DMA_MEM_TO_DEV;
 		desc->dir = DMA_TO_DEVICE;
+	} else if (nandc->props->qpic_v2 && chan == nandc->sts_chan) {
+		sgl = &bam_txn->sts_sgl[bam_txn->sts_sgl_start];
+		sgl_cnt = bam_txn->sts_sgl_pos - bam_txn->sts_sgl_start;
+		bam_txn->sts_sgl_start = bam_txn->sts_sgl_pos;
+		dir_eng = DMA_DEV_TO_MEM;
+		desc->dir = DMA_FROM_DEVICE;
 	} else {
 		sgl = &bam_txn->data_sgl[bam_txn->rx_sgl_start];
 		sgl_cnt = bam_txn->rx_sgl_pos - bam_txn->rx_sgl_start;
@@ -1394,6 +1418,14 @@ static int submit_descs(struct qcom_nand_controller *nandc)
 			if (r)
 				return r;
 		}
+
+		if (nandc->props->qpic_v2) {
+			if (bam_txn->sts_sgl_pos > bam_txn->sts_sgl_start) {
+				r = prepare_bam_async_desc(nandc, nandc->sts_chan, 0);
+				if (r)
+					return r;
+			}
+		}
 	}
 
 	list_for_each_entry(desc, &nandc->desc_list, node)
@@ -1411,6 +1443,8 @@ static int submit_descs(struct qcom_nand_controller *nandc)
 		dma_async_issue_pending(nandc->tx_chan);
 		dma_async_issue_pending(nandc->rx_chan);
 		dma_async_issue_pending(nandc->cmd_chan);
+		if (nandc->props->qpic_v2)
+			dma_async_issue_pending(nandc->sts_chan);
 
 		if (!wait_for_completion_timeout(&bam_txn->txn_done,
 						 QPIC_NAND_COMPLETION_TIMEOUT))
-- 
2.7.4

Re: [PATCH 2/3] mtd: rawnand: qcom: Add sg list to handle status pipe request

[mdalam]

On 2021-09-15 15:27, Md Sadre Alam wrote:
> From QPIC V2.0 onwards there is separate pipe to read status
> for each code word while reading in enhanced mode. page scope
> read and multi page read.
> 
> This sgl list will be use to handle the request via status pipe
> during page scope and multi page read.
> 
> Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 34 
> ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c
> b/drivers/mtd/nand/raw/qcom_nandc.c
> index 42c6291..07448c4 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -213,6 +213,7 @@ nandc_set_reg(chip, reg,			\
>  #define QPIC_PER_CW_CMD_ELEMENTS	32
>  #define QPIC_PER_CW_CMD_SGL		32
>  #define QPIC_PER_CW_DATA_SGL		8
> +#define	QPIC_PER_CW_STS_SGL		8
> 
>  #define QPIC_NAND_COMPLETION_TIMEOUT	msecs_to_jiffies(2000)
> 
> @@ -258,6 +259,7 @@ struct bam_transaction {
>  	struct bam_cmd_element *bam_ce;
>  	struct scatterlist *cmd_sgl;
>  	struct scatterlist *data_sgl;
> +	struct scatterlist *sts_sgl;
>  	u32 bam_ce_pos;
>  	u32 bam_ce_start;
>  	u32 cmd_sgl_pos;
> @@ -266,6 +268,8 @@ struct bam_transaction {
>  	u32 tx_sgl_start;
>  	u32 rx_sgl_pos;
>  	u32 rx_sgl_start;
> +	u32 sts_sgl_pos;
> +	u32 sts_sgl_start;
>  	bool wait_second_completion;
>  	struct completion txn_done;
>  	struct dma_async_tx_descriptor *last_data_desc;
> @@ -508,6 +512,8 @@ alloc_bam_transaction(struct qcom_nand_controller 
> *nandc)
>  		((sizeof(*bam_txn->bam_ce) * QPIC_PER_CW_CMD_ELEMENTS) +
>  		(sizeof(*bam_txn->cmd_sgl) * QPIC_PER_CW_CMD_SGL) +
>  		(sizeof(*bam_txn->data_sgl) * QPIC_PER_CW_DATA_SGL));
> +	if (nandc->props->qpic_v2)
> +		bam_txn_size += (sizeof(*bam_txn->sts_sgl) * QPIC_PER_CW_STS_SGL);
> 
>  	bam_txn_buf = devm_kzalloc(nandc->dev, bam_txn_size, GFP_KERNEL);
>  	if (!bam_txn_buf)
> @@ -526,6 +532,12 @@ alloc_bam_transaction(struct qcom_nand_controller 
> *nandc)
> 
>  	bam_txn->data_sgl = bam_txn_buf;
> 
> +	if (nandc->props->qpic_v2) {
> +		bam_txn_buf +=
> +			sizeof(*bam_txn->sts_sgl) * QPIC_PER_CW_STS_SGL * num_cw;
> +		bam_txn->sts_sgl = bam_txn_buf;
> +	}
> +
>  	init_completion(&bam_txn->txn_done);
> 
>  	return bam_txn;
> @@ -554,6 +566,12 @@ static void clear_bam_transaction(struct
> qcom_nand_controller *nandc)
>  		      QPIC_PER_CW_CMD_SGL);
>  	sg_init_table(bam_txn->data_sgl, nandc->max_cwperpage *
>  		      QPIC_PER_CW_DATA_SGL);
> +	if (nandc->props->qpic_v2) {
> +		bam_txn->sts_sgl_pos = 0;
> +		bam_txn->sts_sgl_start = 0;
> +		sg_init_table(bam_txn->sts_sgl, nandc->max_cwperpage *
> +				QPIC_PER_CW_STS_SGL);
> +	}
> 
>  	reinit_completion(&bam_txn->txn_done);
>  }
> @@ -808,6 +826,12 @@ static int prepare_bam_async_desc(struct
> qcom_nand_controller *nandc,
>  		bam_txn->tx_sgl_start = bam_txn->tx_sgl_pos;
>  		dir_eng = DMA_MEM_TO_DEV;
>  		desc->dir = DMA_TO_DEVICE;
> +	} else if (nandc->props->qpic_v2 && chan == nandc->sts_chan) {
> +		sgl = &bam_txn->sts_sgl[bam_txn->sts_sgl_start];
> +		sgl_cnt = bam_txn->sts_sgl_pos - bam_txn->sts_sgl_start;
> +		bam_txn->sts_sgl_start = bam_txn->sts_sgl_pos;
> +		dir_eng = DMA_DEV_TO_MEM;
> +		desc->dir = DMA_FROM_DEVICE;
>  	} else {
>  		sgl = &bam_txn->data_sgl[bam_txn->rx_sgl_start];
>  		sgl_cnt = bam_txn->rx_sgl_pos - bam_txn->rx_sgl_start;
> @@ -1394,6 +1418,14 @@ static int submit_descs(struct
> qcom_nand_controller *nandc)
>  			if (r)
>  				return r;
>  		}
> +
> +		if (nandc->props->qpic_v2) {
> +			if (bam_txn->sts_sgl_pos > bam_txn->sts_sgl_start) {
> +				r = prepare_bam_async_desc(nandc, nandc->sts_chan, 0);
> +				if (r)
> +					return r;
> +			}
> +		}
>  	}
> 
>  	list_for_each_entry(desc, &nandc->desc_list, node)
> @@ -1411,6 +1443,8 @@ static int submit_descs(struct
> qcom_nand_controller *nandc)
>  		dma_async_issue_pending(nandc->tx_chan);
>  		dma_async_issue_pending(nandc->rx_chan);
>  		dma_async_issue_pending(nandc->cmd_chan);
> +		if (nandc->props->qpic_v2)
> +			dma_async_issue_pending(nandc->sts_chan);
> 
>  		if (!wait_for_completion_timeout(&bam_txn->txn_done,
>  						 QPIC_NAND_COMPLETION_TIMEOUT))


Ping! Please provide me some updates on this patch.

Re: [PATCH 2/3] mtd: rawnand: qcom: Add sg list to handle status pipe request

[Miquel Raynal]

Hello,

mdalam@codeaurora.org wrote on Tue, 28 Sep 2021 17:50:06 +0530:

> On 2021-09-15 15:27, Md Sadre Alam wrote:
> > From QPIC V2.0 onwards there is separate pipe to read status
> > for each code word while reading in enhanced mode.

What is enhanced mode?

> page scope read and multi page read.

This is not a correct sentence.

> > This sgl list will be use to handle the request via status pipe

         ^^^              used                          the

Please use plain english in the commit description.

> > during page scope and multi page read.

What is page scope?

> > Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>

Thanks,
Miquèl

Re: [PATCH 2/3] mtd: rawnand: qcom: Add sg list to handle status pipe request

[Manivannan Sadhasivam]

On Wed, Sep 15, 2021 at 03:27:30PM +0530, Md Sadre Alam wrote:
> From QPIC V2.0 onwards there is separate pipe to read status
> for each code word while reading in enhanced mode. page scope
> read and multi page read.
> 
> This sgl list will be use to handle the request via status pipe
> during page scope and multi page read.
> 
> Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 42c6291..07448c4 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -213,6 +213,7 @@ nandc_set_reg(chip, reg,			\
>  #define QPIC_PER_CW_CMD_ELEMENTS	32
>  #define QPIC_PER_CW_CMD_SGL		32
>  #define QPIC_PER_CW_DATA_SGL		8
> +#define	QPIC_PER_CW_STS_SGL		8

Use space after #define

>  
>  #define QPIC_NAND_COMPLETION_TIMEOUT	msecs_to_jiffies(2000)
>  
> @@ -258,6 +259,7 @@ struct bam_transaction {
>  	struct bam_cmd_element *bam_ce;
>  	struct scatterlist *cmd_sgl;
>  	struct scatterlist *data_sgl;
> +	struct scatterlist *sts_sgl;
>  	u32 bam_ce_pos;
>  	u32 bam_ce_start;
>  	u32 cmd_sgl_pos;
> @@ -266,6 +268,8 @@ struct bam_transaction {
>  	u32 tx_sgl_start;
>  	u32 rx_sgl_pos;
>  	u32 rx_sgl_start;
> +	u32 sts_sgl_pos;
> +	u32 sts_sgl_start;
>  	bool wait_second_completion;
>  	struct completion txn_done;
>  	struct dma_async_tx_descriptor *last_data_desc;
> @@ -508,6 +512,8 @@ alloc_bam_transaction(struct qcom_nand_controller *nandc)
>  		((sizeof(*bam_txn->bam_ce) * QPIC_PER_CW_CMD_ELEMENTS) +
>  		(sizeof(*bam_txn->cmd_sgl) * QPIC_PER_CW_CMD_SGL) +
>  		(sizeof(*bam_txn->data_sgl) * QPIC_PER_CW_DATA_SGL));
> +	if (nandc->props->qpic_v2)
> +		bam_txn_size += (sizeof(*bam_txn->sts_sgl) * QPIC_PER_CW_STS_SGL);
>  
>  	bam_txn_buf = devm_kzalloc(nandc->dev, bam_txn_size, GFP_KERNEL);
>  	if (!bam_txn_buf)
> @@ -526,6 +532,12 @@ alloc_bam_transaction(struct qcom_nand_controller *nandc)
>  
>  	bam_txn->data_sgl = bam_txn_buf;
>  
> +	if (nandc->props->qpic_v2) {
> +		bam_txn_buf +=
> +			sizeof(*bam_txn->sts_sgl) * QPIC_PER_CW_STS_SGL * num_cw;
> +		bam_txn->sts_sgl = bam_txn_buf;
> +	}
> +
>  	init_completion(&bam_txn->txn_done);
>  
>  	return bam_txn;
> @@ -554,6 +566,12 @@ static void clear_bam_transaction(struct qcom_nand_controller *nandc)
>  		      QPIC_PER_CW_CMD_SGL);
>  	sg_init_table(bam_txn->data_sgl, nandc->max_cwperpage *
>  		      QPIC_PER_CW_DATA_SGL);
> +	if (nandc->props->qpic_v2) {
> +		bam_txn->sts_sgl_pos = 0;
> +		bam_txn->sts_sgl_start = 0;
> +		sg_init_table(bam_txn->sts_sgl, nandc->max_cwperpage *
> +				QPIC_PER_CW_STS_SGL);
> +	}
>  
>  	reinit_completion(&bam_txn->txn_done);
>  }
> @@ -808,6 +826,12 @@ static int prepare_bam_async_desc(struct qcom_nand_controller *nandc,
>  		bam_txn->tx_sgl_start = bam_txn->tx_sgl_pos;
>  		dir_eng = DMA_MEM_TO_DEV;
>  		desc->dir = DMA_TO_DEVICE;
> +	} else if (nandc->props->qpic_v2 && chan == nandc->sts_chan) {

These two are mutually inclusive, so why can't you simply check for the
existence of "nanc->sts_chan"?

> +		sgl = &bam_txn->sts_sgl[bam_txn->sts_sgl_start];
> +		sgl_cnt = bam_txn->sts_sgl_pos - bam_txn->sts_sgl_start;
> +		bam_txn->sts_sgl_start = bam_txn->sts_sgl_pos;
> +		dir_eng = DMA_DEV_TO_MEM;
> +		desc->dir = DMA_FROM_DEVICE;
>  	} else {
>  		sgl = &bam_txn->data_sgl[bam_txn->rx_sgl_start];
>  		sgl_cnt = bam_txn->rx_sgl_pos - bam_txn->rx_sgl_start;
> @@ -1394,6 +1418,14 @@ static int submit_descs(struct qcom_nand_controller *nandc)
>  			if (r)
>  				return r;
>  		}
> +
> +		if (nandc->props->qpic_v2) {

I feel like you should just check for "nandc->sts_chan" instead of qpic_v2. This
will make the driver work with future revisions of the IP as well.

> +			if (bam_txn->sts_sgl_pos > bam_txn->sts_sgl_start) {
> +				r = prepare_bam_async_desc(nandc, nandc->sts_chan, 0);
> +				if (r)
> +					return r;
> +			}
> +		}
>  	}
>  
>  	list_for_each_entry(desc, &nandc->desc_list, node)
> @@ -1411,6 +1443,8 @@ static int submit_descs(struct qcom_nand_controller *nandc)
>  		dma_async_issue_pending(nandc->tx_chan);
>  		dma_async_issue_pending(nandc->rx_chan);
>  		dma_async_issue_pending(nandc->cmd_chan);
> +		if (nandc->props->qpic_v2)
> +			dma_async_issue_pending(nandc->sts_chan);

Same here.

Thanks,
Mani

>  
>  		if (!wait_for_completion_timeout(&bam_txn->txn_done,
>  						 QPIC_NAND_COMPLETION_TIMEOUT))
> -- 
> 2.7.4
> 

[PATCH 3/3] mtd: rawnand: qcom: Add support for page scope read

[Md Sadre Alam]

QPIC V2.0 onwards QPIC controller support enhanced read mode
like page scope read and multi page read.

In QPIC V1, SW is needed to write EXEC_CMD register for each
Code word and collect any Status related to that CW before
issueing EXEC_CMD for next CW.

Page scope command is truly a page mode command where SW is
required to issue EXEC_CMD only once for a page. Controller
HW takes care of Codeword specific details and automatically
returns status associated with each CW to BAM pipe, dedicated
for status deposition.

With this command, SW now can issue one read command for a page
and upon receiving completion interrupt, can process status,
that have already been deposited in memory through status BAM pipe.

Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
---
 drivers/mtd/nand/raw/qcom_nandc.c | 77 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 07448c4..257dec7e 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -157,6 +157,10 @@
 #define	OP_FETCH_ID			0xb
 #define	OP_RESET_DEVICE			0xd
 
+/* Auto status val and mask */
+#define	AUTO_STS_VAL			0x000B000B
+#define PAGE_SCOPE_READ			BIT(23)
+
 /* Default Value for NAND_DEV_CMD_VLD */
 #define NAND_DEV_CMD_VLD_VAL		(READ_START_VLD | WRITE_START_VLD | \
 					 ERASE_START_VLD | SEQ_READ_START_VLD)
@@ -336,6 +340,8 @@ struct nandc_regs {
 
 	__le32 erased_cw_detect_cfg_clr;
 	__le32 erased_cw_detect_cfg_set;
+
+	__le32 auto_sts_en;
 };
 
 /*
@@ -421,6 +427,9 @@ struct qcom_nand_controller {
 
 	u32 cmd1, vld;
 	const struct qcom_nandc_props *props;
+
+	__le32 *status_buf;
+	int sts_buf_size;
 };
 
 /*
@@ -487,6 +496,7 @@ struct qcom_nandc_props {
 	bool is_bam;
 	bool is_qpic;
 	bool qpic_v2;
+	bool page_scope;
 	u32 dev_cmd_reg_start;
 };
 
@@ -656,6 +666,8 @@ static __le32 *offset_to_nandc_reg(struct nandc_regs *regs, int offset)
 		return &regs->cfg1;
 	case NAND_DEV0_ECC_CFG:
 		return &regs->ecc_bch_cfg;
+	case NAND_AUTO_STATUS_EN:
+		return &regs->auto_sts_en;
 	case NAND_READ_STATUS:
 		return &regs->clrreadstatus;
 	case NAND_DEV_CMD1:
@@ -756,10 +768,13 @@ static void update_rw_regs(struct qcom_nand_host *host, int num_cw, bool read, i
 	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
 
 	if (read) {
-		if (host->use_ecc)
+		if (host->use_ecc) {
 			cmd = OP_PAGE_READ_WITH_ECC | PAGE_ACC | LAST_PAGE;
-		else
+			if (nandc->props->qpic_v2 && nandc->props->page_scope)
+				cmd |= PAGE_SCOPE_READ;
+		} else {
 			cmd = OP_PAGE_READ | PAGE_ACC | LAST_PAGE;
+		}
 	} else {
 		cmd = OP_PROGRAM_PAGE | PAGE_ACC | LAST_PAGE;
 	}
@@ -788,6 +803,9 @@ static void update_rw_regs(struct qcom_nand_host *host, int num_cw, bool read, i
 	nandc_set_reg(chip, NAND_READ_STATUS, host->clrreadstatus);
 	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
 
+	if (nandc->props->qpic_v2 && nandc->props->page_scope)
+		nandc_set_reg(chip, NAND_AUTO_STATUS_EN, AUTO_STS_VAL);
+
 	if (read)
 		nandc_set_read_loc(chip, cw, 0, 0, host->use_ecc ?
 				   host->cw_data : host->cw_size, 1);
@@ -1040,6 +1058,26 @@ static int prep_adm_dma_desc(struct qcom_nand_controller *nandc, bool read,
 }
 
 /*
+ * read_status_data_dma: prepares a DMA descriptor to transfer status from the
+ *			 controller's status registers to buffer 'vaddr'
+ * @reg_off:             offset within the controller's data buffer
+ * @vaddr:               virtual address of the buffer we want to write to
+ * @size:                DMA transaction size in bytes
+ * @flags:               flags to control DMA descriptor preparation
+ */
+static int read_status_data_dma(struct qcom_nand_controller *nandc, int reg_off,
+				const u8 *vaddr, int size, unsigned int flags)
+{
+	struct bam_transaction *bam_txn = nandc->bam_txn;
+
+	sg_set_buf(&bam_txn->sts_sgl[bam_txn->sts_sgl_pos],
+		   vaddr, size);
+	bam_txn->sts_sgl_pos++;
+
+	return 0;
+}
+
+/*
  * read_reg_dma:	prepares a descriptor to read a given number of
  *			contiguous registers to the reg_read_buf pointer
  *
@@ -1186,13 +1224,20 @@ config_nand_cw_read(struct nand_chip *chip, bool use_ecc, int cw)
 		write_reg_dma(nandc, reg, 4, NAND_BAM_NEXT_SGL);
 
 	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
-	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
 
 	if (use_ecc) {
-		read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
-		read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
-			     NAND_BAM_NEXT_SGL);
+		if (nandc->props->qpic_v2 && nandc->props->page_scope) {
+			if (qcom_nandc_is_last_cw(ecc, cw))
+				write_reg_dma(nandc, NAND_EXEC_CMD, 1,
+					      NAND_BAM_NEXT_SGL);
+		} else {
+			write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
+			read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
+			read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
+				     NAND_BAM_NEXT_SGL);
+		}
 	} else {
+		write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
 		read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
 	}
 }
@@ -1959,6 +2004,8 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
 	u8 *data_buf_start = data_buf, *oob_buf_start = oob_buf;
 	int i, ret;
+	__le32 *status_buf_start = nandc->status_buf;
+	__le32 *status_buf_cw = nandc->status_buf;
 
 	config_nand_page_read(chip);
 
@@ -1994,6 +2041,12 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
 			read_data_dma(nandc, FLASH_BUF_ACC, data_buf,
 				      data_size, 0);
 
+		if (nandc->props->qpic_v2 && nandc->props->page_scope) {
+			read_status_data_dma(nandc, FLASH_BUF_ACC, (void *)status_buf_cw,
+					     (nandc->sts_buf_size >> 2), 0);
+			status_buf_cw += (nandc->sts_buf_size >> 4);
+		}
+
 		/*
 		 * when ecc is enabled, the controller doesn't read the real
 		 * or dummy bad block markers in each chunk. To maintain a
@@ -2025,6 +2078,9 @@ static int read_page_ecc(struct qcom_nand_host *host, u8 *data_buf,
 		return ret;
 	}
 
+	if (nandc->props->qpic_v2 && nandc->props->page_scope)
+		memmove(nandc->reg_read_buf, status_buf_start, nandc->sts_buf_size);
+
 	return parse_read_errors(host, data_buf_start, oob_buf_start, page);
 }
 
@@ -3005,6 +3061,14 @@ static int qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
 		}
 	}
 
+	if (nandc->props->qpic_v2 && nandc->props->page_scope) {
+		nandc->sts_buf_size = mtd->writesize == SZ_2K ? 48 : 96;
+		nandc->status_buf = devm_kzalloc(nandc->dev, nandc->sts_buf_size,
+						 GFP_KERNEL);
+		if (!nandc->status_buf)
+			return -ENOMEM;
+	}
+
 	ret = mtd_device_parse_register(mtd, probes, NULL, NULL, 0);
 	if (ret)
 		nand_cleanup(chip);
@@ -3197,6 +3261,7 @@ static const struct qcom_nandc_props sdx55_nandc_props = {
 	.is_bam = true,
 	.is_qpic = true,
 	.qpic_v2 = true,
+	.page_scope = true,
 	.dev_cmd_reg_start = 0x7000,
 };
 
-- 
2.7.4

Re: [PATCH 3/3] mtd: rawnand: qcom: Add support for page scope read

[mdalam]

On 2021-09-15 15:27, Md Sadre Alam wrote:
> QPIC V2.0 onwards QPIC controller support enhanced read mode
> like page scope read and multi page read.
> 
> In QPIC V1, SW is needed to write EXEC_CMD register for each
> Code word and collect any Status related to that CW before
> issueing EXEC_CMD for next CW.
> 
> Page scope command is truly a page mode command where SW is
> required to issue EXEC_CMD only once for a page. Controller
> HW takes care of Codeword specific details and automatically
> returns status associated with each CW to BAM pipe, dedicated
> for status deposition.
> 
> With this command, SW now can issue one read command for a page
> and upon receiving completion interrupt, can process status,
> that have already been deposited in memory through status BAM pipe.
> 
> Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 77 
> ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 71 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c
> b/drivers/mtd/nand/raw/qcom_nandc.c
> index 07448c4..257dec7e 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -157,6 +157,10 @@
>  #define	OP_FETCH_ID			0xb
>  #define	OP_RESET_DEVICE			0xd
> 
> +/* Auto status val and mask */
> +#define	AUTO_STS_VAL			0x000B000B
> +#define PAGE_SCOPE_READ			BIT(23)
> +
>  /* Default Value for NAND_DEV_CMD_VLD */
>  #define NAND_DEV_CMD_VLD_VAL		(READ_START_VLD | WRITE_START_VLD | \
>  					 ERASE_START_VLD | SEQ_READ_START_VLD)
> @@ -336,6 +340,8 @@ struct nandc_regs {
> 
>  	__le32 erased_cw_detect_cfg_clr;
>  	__le32 erased_cw_detect_cfg_set;
> +
> +	__le32 auto_sts_en;
>  };
> 
>  /*
> @@ -421,6 +427,9 @@ struct qcom_nand_controller {
> 
>  	u32 cmd1, vld;
>  	const struct qcom_nandc_props *props;
> +
> +	__le32 *status_buf;
> +	int sts_buf_size;
>  };
> 
>  /*
> @@ -487,6 +496,7 @@ struct qcom_nandc_props {
>  	bool is_bam;
>  	bool is_qpic;
>  	bool qpic_v2;
> +	bool page_scope;
>  	u32 dev_cmd_reg_start;
>  };
> 
> @@ -656,6 +666,8 @@ static __le32 *offset_to_nandc_reg(struct
> nandc_regs *regs, int offset)
>  		return &regs->cfg1;
>  	case NAND_DEV0_ECC_CFG:
>  		return &regs->ecc_bch_cfg;
> +	case NAND_AUTO_STATUS_EN:
> +		return &regs->auto_sts_en;
>  	case NAND_READ_STATUS:
>  		return &regs->clrreadstatus;
>  	case NAND_DEV_CMD1:
> @@ -756,10 +768,13 @@ static void update_rw_regs(struct qcom_nand_host
> *host, int num_cw, bool read, i
>  	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
> 
>  	if (read) {
> -		if (host->use_ecc)
> +		if (host->use_ecc) {
>  			cmd = OP_PAGE_READ_WITH_ECC | PAGE_ACC | LAST_PAGE;
> -		else
> +			if (nandc->props->qpic_v2 && nandc->props->page_scope)
> +				cmd |= PAGE_SCOPE_READ;
> +		} else {
>  			cmd = OP_PAGE_READ | PAGE_ACC | LAST_PAGE;
> +		}
>  	} else {
>  		cmd = OP_PROGRAM_PAGE | PAGE_ACC | LAST_PAGE;
>  	}
> @@ -788,6 +803,9 @@ static void update_rw_regs(struct qcom_nand_host
> *host, int num_cw, bool read, i
>  	nandc_set_reg(chip, NAND_READ_STATUS, host->clrreadstatus);
>  	nandc_set_reg(chip, NAND_EXEC_CMD, 1);
> 
> +	if (nandc->props->qpic_v2 && nandc->props->page_scope)
> +		nandc_set_reg(chip, NAND_AUTO_STATUS_EN, AUTO_STS_VAL);
> +
>  	if (read)
>  		nandc_set_read_loc(chip, cw, 0, 0, host->use_ecc ?
>  				   host->cw_data : host->cw_size, 1);
> @@ -1040,6 +1058,26 @@ static int prep_adm_dma_desc(struct
> qcom_nand_controller *nandc, bool read,
>  }
> 
>  /*
> + * read_status_data_dma: prepares a DMA descriptor to transfer status 
> from the
> + *			 controller's status registers to buffer 'vaddr'
> + * @reg_off:             offset within the controller's data buffer
> + * @vaddr:               virtual address of the buffer we want to 
> write to
> + * @size:                DMA transaction size in bytes
> + * @flags:               flags to control DMA descriptor preparation
> + */
> +static int read_status_data_dma(struct qcom_nand_controller *nandc,
> int reg_off,
> +				const u8 *vaddr, int size, unsigned int flags)
> +{
> +	struct bam_transaction *bam_txn = nandc->bam_txn;
> +
> +	sg_set_buf(&bam_txn->sts_sgl[bam_txn->sts_sgl_pos],
> +		   vaddr, size);
> +	bam_txn->sts_sgl_pos++;
> +
> +	return 0;
> +}
> +
> +/*
>   * read_reg_dma:	prepares a descriptor to read a given number of
>   *			contiguous registers to the reg_read_buf pointer
>   *
> @@ -1186,13 +1224,20 @@ config_nand_cw_read(struct nand_chip *chip,
> bool use_ecc, int cw)
>  		write_reg_dma(nandc, reg, 4, NAND_BAM_NEXT_SGL);
> 
>  	write_reg_dma(nandc, NAND_FLASH_CMD, 1, NAND_BAM_NEXT_SGL);
> -	write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
> 
>  	if (use_ecc) {
> -		read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
> -		read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
> -			     NAND_BAM_NEXT_SGL);
> +		if (nandc->props->qpic_v2 && nandc->props->page_scope) {
> +			if (qcom_nandc_is_last_cw(ecc, cw))
> +				write_reg_dma(nandc, NAND_EXEC_CMD, 1,
> +					      NAND_BAM_NEXT_SGL);
> +		} else {
> +			write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
> +			read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
> +			read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
> +				     NAND_BAM_NEXT_SGL);
> +		}
>  	} else {
> +		write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
>  		read_reg_dma(nandc, NAND_FLASH_STATUS, 1, NAND_BAM_NEXT_SGL);
>  	}
>  }
> @@ -1959,6 +2004,8 @@ static int read_page_ecc(struct qcom_nand_host
> *host, u8 *data_buf,
>  	struct nand_ecc_ctrl *ecc = &chip->ecc;
>  	u8 *data_buf_start = data_buf, *oob_buf_start = oob_buf;
>  	int i, ret;
> +	__le32 *status_buf_start = nandc->status_buf;
> +	__le32 *status_buf_cw = nandc->status_buf;
> 
>  	config_nand_page_read(chip);
> 
> @@ -1994,6 +2041,12 @@ static int read_page_ecc(struct qcom_nand_host
> *host, u8 *data_buf,
>  			read_data_dma(nandc, FLASH_BUF_ACC, data_buf,
>  				      data_size, 0);
> 
> +		if (nandc->props->qpic_v2 && nandc->props->page_scope) {
> +			read_status_data_dma(nandc, FLASH_BUF_ACC, (void *)status_buf_cw,
> +					     (nandc->sts_buf_size >> 2), 0);
> +			status_buf_cw += (nandc->sts_buf_size >> 4);
> +		}
> +
>  		/*
>  		 * when ecc is enabled, the controller doesn't read the real
>  		 * or dummy bad block markers in each chunk. To maintain a
> @@ -2025,6 +2078,9 @@ static int read_page_ecc(struct qcom_nand_host
> *host, u8 *data_buf,
>  		return ret;
>  	}
> 
> +	if (nandc->props->qpic_v2 && nandc->props->page_scope)
> +		memmove(nandc->reg_read_buf, status_buf_start, nandc->sts_buf_size);
> +
>  	return parse_read_errors(host, data_buf_start, oob_buf_start, page);
>  }
> 
> @@ -3005,6 +3061,14 @@ static int
> qcom_nand_host_init_and_register(struct qcom_nand_controller *nandc,
>  		}
>  	}
> 
> +	if (nandc->props->qpic_v2 && nandc->props->page_scope) {
> +		nandc->sts_buf_size = mtd->writesize == SZ_2K ? 48 : 96;
> +		nandc->status_buf = devm_kzalloc(nandc->dev, nandc->sts_buf_size,
> +						 GFP_KERNEL);
> +		if (!nandc->status_buf)
> +			return -ENOMEM;
> +	}
> +
>  	ret = mtd_device_parse_register(mtd, probes, NULL, NULL, 0);
>  	if (ret)
>  		nand_cleanup(chip);
> @@ -3197,6 +3261,7 @@ static const struct qcom_nandc_props 
> sdx55_nandc_props = {
>  	.is_bam = true,
>  	.is_qpic = true,
>  	.qpic_v2 = true,
> +	.page_scope = true,
>  	.dev_cmd_reg_start = 0x7000,
>  };

Ping! Please provide me some updates on this patch.

Re: [PATCH 3/3] mtd: rawnand: qcom: Add support for page scope read

[Miquel Raynal]

Hello,

mdalam@codeaurora.org wrote on Tue, 28 Sep 2021 17:51:00 +0530:

> On 2021-09-15 15:27, Md Sadre Alam wrote:
> > QPIC V2.0 onwards QPIC controller support enhanced read mode
> > like page scope read and multi page read.
> > 
> > In QPIC V1, SW is needed to write EXEC_CMD register for each

                the driver needs to (same below)

> > Code word and collect any Status related to that CW before
> > issueing EXEC_CMD for next CW.
> > 
> > Page scope command is truly a page mode command where SW is
> > required to issue EXEC_CMD only once for a page. Controller
> > HW takes care of Codeword specific details and automatically
> > returns status associated with each CW to BAM pipe, dedicated
> > for status deposition.
> > 
> > With this command, SW now can issue one read command for a page
> > and upon receiving completion interrupt, can process status,
> > that have already been deposited in memory through status BAM pipe.
> > 
> > Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> > ---
> >  drivers/mtd/nand/raw/qcom_nandc.c | 77 > ++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 71 insertions(+), 6 deletions(-)
> > 

Thanks,
Miquèl

Re: [PATCH 3/3] mtd: rawnand: qcom: Add support for page scope read

[Manivannan Sadhasivam]

On Wed, Sep 15, 2021 at 03:27:31PM +0530, Md Sadre Alam wrote:
> QPIC V2.0 onwards QPIC controller support enhanced read mode
> like page scope read and multi page read.
> 

Define page scope read.

> In QPIC V1, SW is needed to write EXEC_CMD register for each
> Code word and collect any Status related to that CW before
> issueing EXEC_CMD for next CW.
> 
> Page scope command is truly a page mode command where SW is
> required to issue EXEC_CMD only once for a page. Controller
> HW takes care of Codeword specific details and automatically
> returns status associated with each CW to BAM pipe, dedicated
> for status deposition.
> 
> With this command, SW now can issue one read command for a page
> and upon receiving completion interrupt, can process status,
> that have already been deposited in memory through status BAM pipe.
> 
> Signed-off-by: Md Sadre Alam <mdalam@codeaurora.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 77 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 71 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 07448c4..257dec7e 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -157,6 +157,10 @@
>  #define	OP_FETCH_ID			0xb
>  #define	OP_RESET_DEVICE			0xd
>  
> +/* Auto status val and mask */
> +#define	AUTO_STS_VAL			0x000B000B

Use non-cap hex.

> +#define PAGE_SCOPE_READ			BIT(23)
> +
>  /* Default Value for NAND_DEV_CMD_VLD */
>  #define NAND_DEV_CMD_VLD_VAL		(READ_START_VLD | WRITE_START_VLD | \
>  					 ERASE_START_VLD | SEQ_READ_START_VLD)
> @@ -336,6 +340,8 @@ struct nandc_regs {
>  
>  	__le32 erased_cw_detect_cfg_clr;
>  	__le32 erased_cw_detect_cfg_set;
> +
> +	__le32 auto_sts_en;
>  };
>  
>  /*
> @@ -421,6 +427,9 @@ struct qcom_nand_controller {
>  
>  	u32 cmd1, vld;
>  	const struct qcom_nandc_props *props;
> +
> +	__le32 *status_buf;
> +	int sts_buf_size;

Add kdoc for these two members.

>  };
>  
>  /*
> @@ -487,6 +496,7 @@ struct qcom_nandc_props {
>  	bool is_bam;
>  	bool is_qpic;
>  	bool qpic_v2;
> +	bool page_scope;
>  	u32 dev_cmd_reg_start;
>  };
>  
> @@ -656,6 +666,8 @@ static __le32 *offset_to_nandc_reg(struct nandc_regs *regs, int offset)
>  		return &regs->cfg1;
>  	case NAND_DEV0_ECC_CFG:
>  		return &regs->ecc_bch_cfg;
> +	case NAND_AUTO_STATUS_EN:
> +		return &regs->auto_sts_en;
>  	case NAND_READ_STATUS:
>  		return &regs->clrreadstatus;
>  	case NAND_DEV_CMD1:
> @@ -756,10 +768,13 @@ static void update_rw_regs(struct qcom_nand_host *host, int num_cw, bool read, i
>  	struct qcom_nand_controller *nandc = get_qcom_nand_controller(chip);
>  
>  	if (read) {
> -		if (host->use_ecc)
> +		if (host->use_ecc) {
>  			cmd = OP_PAGE_READ_WITH_ECC | PAGE_ACC | LAST_PAGE;
> -		else
> +			if (nandc->props->qpic_v2 && nandc->props->page_scope)

Again, why you are checking for both conditions? Using "page_scope" is
sufficient enough.

> +				cmd |= PAGE_SCOPE_READ;
> +		} else {
>  			cmd = OP_PAGE_READ | PAGE_ACC | LAST_PAGE;
> +		}
>  	} else {
>  		cmd = OP_PROGRAM_PAGE | PAGE_ACC | LAST_PAGE;
>  	}

[...]

>  	if (use_ecc) {
> -		read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
> -		read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
> -			     NAND_BAM_NEXT_SGL);
> +		if (nandc->props->qpic_v2 && nandc->props->page_scope) {
> +			if (qcom_nandc_is_last_cw(ecc, cw))
> +				write_reg_dma(nandc, NAND_EXEC_CMD, 1,
> +					      NAND_BAM_NEXT_SGL);
> +		} else {
> +			write_reg_dma(nandc, NAND_EXEC_CMD, 1, NAND_BAM_NEXT_SGL);
> +			read_reg_dma(nandc, NAND_FLASH_STATUS, 2, 0);
> +			read_reg_dma(nandc, NAND_ERASED_CW_DETECT_STATUS, 1,
> +				     NAND_BAM_NEXT_SGL);
> +		}

You need to add a comment for this.

Thanks,
Mani

Re: [PATCH 0/3] Add support for page scope read

[Manivannan Sadhasivam]

Hi,

On Wed, Sep 15, 2021 at 03:27:28PM +0530, Md Sadre Alam wrote:
> These series of patches to support page scope read.
> 
> In QPIC v1, SW is needed to write EXEC_CMD register
> for each Code word and collect any Status related to
> that CW before issueing EXEC_CMD for next CW.
> 
> Page scope command, currently supported only for read
> commands in bam mode, is truly a page mode command where
> SW is required to issue EXEC_CMD only once for a page.
> Controller HW takes care of Codeword specific details
> and automatically returns status associated with each
> CW to BAM pipe, dedicated for status deposition.
> 

This description doesn't clearly convey the intention of the patches. Please
take time to write the cover letter describing what the patches do and how
they solve a problem.

If possible, do mention the platform on which the patches were tested.

And please avoid using Qualcomm specific acronymns such as CW as it only makes
sense to limited amount of people who has access to technical documents.

Thanks,
Mani

> Md Sadre Alam (3):
>   mtd: rawnand: qcom: Add support for status pipe
>   mtd: rawnand: qcom: Add sg list to handle status pipe request
>   mtd: rawnand: qcom: Add support for page scope read
> 
>  drivers/mtd/nand/raw/qcom_nandc.c | 125 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 119 insertions(+), 6 deletions(-)
> 
> -- 
> 2.7.4
>