Re: [PATCH v3 01/20] mtd: nand: qcom: program NAND_DEV_CMD_VLD register

2017-08-10 Thread Abhishek Sahu

On 2017-08-10 15:12, Boris Brezillon wrote:

Le Sat,  5 Aug 2017 21:49:39 +0530,
Abhishek Sahu  a écrit :


The NAND page read fails without complete boot chain since
NAND_DEV_CMD_VLD value is not proper. The default power on reset
value for this register is

0xe - ERASE_START_VALID | WRITE_START_VALID | READ_STOP_VALID

The READ_START_VALID should be enabled for sending PAGE_READ
command. READ_STOP_VALID should be cleared since normal NAND
page read does not require READ_STOP command.

Fixes: c76b78d8ec05a ("mtd: nand: Qualcomm NAND controller driver")
Cc: sta...@vger.kernel.org
Reviewed-by: Archit Taneja 
Signed-off-by: Abhishek Sahu 
---
 drivers/mtd/nand/qcom_nandc.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/qcom_nandc.c 
b/drivers/mtd/nand/qcom_nandc.c

index 0289f09..7406019 100644
--- a/drivers/mtd/nand/qcom_nandc.c
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -110,6 +110,10 @@

 /* NAND_DEV_CMD_VLD bits */
 #defineREAD_START_VLD  0
+#defineREAD_STOP_VALID 1
+#defineWRITE_START_VALID   2
+#defineERASE_START_VALID   3
+#defineSEQ_READ_START_VLD  4


Why not

#define READ_START_VLD  BIT(0)
#define READ_STOP_VALID BIT(1)
#define WRITE_START_VALID   BIT(2)
#define ERASE_START_VALID   BIT(3)
#define SEQ_READ_START_VLD  BIT(4)



 Sure. we can change to use BIT macro.

 We are using READ_START_VLD only at one place so will replace in
 current code from

 /* configure CMD1 and VLD for ONFI param probing */
nandc_set_reg(nandc, NAND_DEV_CMD_VLD,
  (nandc->vld & ~(1 << READ_START_VLD))
  | 0 << READ_START_VLD);
 to

 nandc_set_reg(nandc, NAND_DEV_CMD_VLD, nandc->vld & ~READ_START_VLD)



 /* NAND_EBI2_ECC_BUF_CFG bits */
 #defineNUM_STEPS   0
@@ -148,6 +152,12 @@
 #defineFETCH_ID0xb
 #defineRESET_DEVICE0xd

+/* Default Value for NAND_DEV_CMD_VLD */
+#define NAND_DEV_CMD_VLD_VAL   (BIT(READ_START_VLD)| \
+BIT(WRITE_START_VALID) | \
+BIT(ERASE_START_VALID) | \
+BIT(SEQ_READ_START_VLD))
+


and then:

#define NAND_DEV_CMD_VLD_VAL(READ_START_VLD | \
 WRITE_START_VALID | \
 ERASE_START_VALID | \
 SEQ_READ_START_VLD)

BTW, can you use consistent suffixes? Sometime definitions are suffixed
with _VLD and others are suffixed with _VALID.



 Sure. I will change all the names to _VLD.


 /*
  * the NAND controller performs reads/writes with ECC in 516 byte 
chunks.

  * the driver calls the chunks 'step' or 'codeword' interchangeably
@@ -1995,13 +2005,14 @@ static int qcom_nandc_setup(struct 
qcom_nand_controller *nandc)

 {
/* kill onenand */
nandc_write(nandc, SFLASHC_BURST_CFG, 0);
+   nandc_write(nandc, NAND_DEV_CMD_VLD, NAND_DEV_CMD_VLD_VAL);

/* enable ADM DMA */
nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);

/* save the original values of these registers */
nandc->cmd1 = nandc_read(nandc, NAND_DEV_CMD1);
-   nandc->vld = nandc_read(nandc, NAND_DEV_CMD_VLD);
+   nandc->vld = NAND_DEV_CMD_VLD_VAL;

return 0;
 }


Re: [PATCH v3 01/20] mtd: nand: qcom: program NAND_DEV_CMD_VLD register

2017-08-10 Thread Boris Brezillon
Le Sat,  5 Aug 2017 21:49:39 +0530,
Abhishek Sahu  a écrit :

> The NAND page read fails without complete boot chain since
> NAND_DEV_CMD_VLD value is not proper. The default power on reset
> value for this register is
> 
> 0xe - ERASE_START_VALID | WRITE_START_VALID | READ_STOP_VALID
> 
> The READ_START_VALID should be enabled for sending PAGE_READ
> command. READ_STOP_VALID should be cleared since normal NAND
> page read does not require READ_STOP command.
> 
> Fixes: c76b78d8ec05a ("mtd: nand: Qualcomm NAND controller driver")
> Cc: sta...@vger.kernel.org
> Reviewed-by: Archit Taneja 
> Signed-off-by: Abhishek Sahu 
> ---
>  drivers/mtd/nand/qcom_nandc.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
> index 0289f09..7406019 100644
> --- a/drivers/mtd/nand/qcom_nandc.c
> +++ b/drivers/mtd/nand/qcom_nandc.c
> @@ -110,6 +110,10 @@
>  
>  /* NAND_DEV_CMD_VLD bits */
>  #define  READ_START_VLD  0
> +#define  READ_STOP_VALID 1
> +#define  WRITE_START_VALID   2
> +#define  ERASE_START_VALID   3
> +#define  SEQ_READ_START_VLD  4

Why not

#define READ_START_VLD  BIT(0)
#define READ_STOP_VALID BIT(1)
#define WRITE_START_VALID   BIT(2)
#define ERASE_START_VALID   BIT(3)
#define SEQ_READ_START_VLD  BIT(4)

>  
>  /* NAND_EBI2_ECC_BUF_CFG bits */
>  #define  NUM_STEPS   0
> @@ -148,6 +152,12 @@
>  #define  FETCH_ID0xb
>  #define  RESET_DEVICE0xd
>  
> +/* Default Value for NAND_DEV_CMD_VLD */
> +#define NAND_DEV_CMD_VLD_VAL (BIT(READ_START_VLD)| \
> +  BIT(WRITE_START_VALID) | \
> +  BIT(ERASE_START_VALID) | \
> +  BIT(SEQ_READ_START_VLD))
> +

and then:

#define NAND_DEV_CMD_VLD_VAL(READ_START_VLD | \
 WRITE_START_VALID | \
 ERASE_START_VALID | \
 SEQ_READ_START_VLD)

BTW, can you use consistent suffixes? Sometime definitions are suffixed
with _VLD and others are suffixed with _VALID.

>  /*
>   * the NAND controller performs reads/writes with ECC in 516 byte chunks.
>   * the driver calls the chunks 'step' or 'codeword' interchangeably
> @@ -1995,13 +2005,14 @@ static int qcom_nandc_setup(struct 
> qcom_nand_controller *nandc)
>  {
>   /* kill onenand */
>   nandc_write(nandc, SFLASHC_BURST_CFG, 0);
> + nandc_write(nandc, NAND_DEV_CMD_VLD, NAND_DEV_CMD_VLD_VAL);
>  
>   /* enable ADM DMA */
>   nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
>  
>   /* save the original values of these registers */
>   nandc->cmd1 = nandc_read(nandc, NAND_DEV_CMD1);
> - nandc->vld = nandc_read(nandc, NAND_DEV_CMD_VLD);
> + nandc->vld = NAND_DEV_CMD_VLD_VAL;
>  
>   return 0;
>  }



[PATCH v3 01/20] mtd: nand: qcom: program NAND_DEV_CMD_VLD register

2017-08-05 Thread Abhishek Sahu
The NAND page read fails without complete boot chain since
NAND_DEV_CMD_VLD value is not proper. The default power on reset
value for this register is

0xe - ERASE_START_VALID | WRITE_START_VALID | READ_STOP_VALID

The READ_START_VALID should be enabled for sending PAGE_READ
command. READ_STOP_VALID should be cleared since normal NAND
page read does not require READ_STOP command.

Fixes: c76b78d8ec05a ("mtd: nand: Qualcomm NAND controller driver")
Cc: sta...@vger.kernel.org
Reviewed-by: Archit Taneja 
Signed-off-by: Abhishek Sahu 
---
 drivers/mtd/nand/qcom_nandc.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c
index 0289f09..7406019 100644
--- a/drivers/mtd/nand/qcom_nandc.c
+++ b/drivers/mtd/nand/qcom_nandc.c
@@ -110,6 +110,10 @@
 
 /* NAND_DEV_CMD_VLD bits */
 #defineREAD_START_VLD  0
+#defineREAD_STOP_VALID 1
+#defineWRITE_START_VALID   2
+#defineERASE_START_VALID   3
+#defineSEQ_READ_START_VLD  4
 
 /* NAND_EBI2_ECC_BUF_CFG bits */
 #defineNUM_STEPS   0
@@ -148,6 +152,12 @@
 #defineFETCH_ID0xb
 #defineRESET_DEVICE0xd
 
+/* Default Value for NAND_DEV_CMD_VLD */
+#define NAND_DEV_CMD_VLD_VAL   (BIT(READ_START_VLD)| \
+BIT(WRITE_START_VALID) | \
+BIT(ERASE_START_VALID) | \
+BIT(SEQ_READ_START_VLD))
+
 /*
  * the NAND controller performs reads/writes with ECC in 516 byte chunks.
  * the driver calls the chunks 'step' or 'codeword' interchangeably
@@ -1995,13 +2005,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller 
*nandc)
 {
/* kill onenand */
nandc_write(nandc, SFLASHC_BURST_CFG, 0);
+   nandc_write(nandc, NAND_DEV_CMD_VLD, NAND_DEV_CMD_VLD_VAL);
 
/* enable ADM DMA */
nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
 
/* save the original values of these registers */
nandc->cmd1 = nandc_read(nandc, NAND_DEV_CMD1);
-   nandc->vld = nandc_read(nandc, NAND_DEV_CMD_VLD);
+   nandc->vld = NAND_DEV_CMD_VLD_VAL;
 
return 0;
 }
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of 
Code Aurora Forum, hosted by The Linux Foundation