Re: [PATCH v4 13/20] mtd: nand: qcom: support for different DEV_CMD register offsets

[Abhishek Sahu <absahu@xxxxxxxxxxxxxx>]

On 2017-08-16 11:22, Archit Taneja wrote:
> On 08/11/2017 05:09 PM, Abhishek Sahu wrote:
> > The FLASH_DEV_CMD registers starting offset is not same in
> > different QPIC NAND controller versions. This patch adds
> > the starting offset in NAND controller properties and uses
> > the same for calculating the actual offset of these registers.
> >
> > Signed-off-by: Abhishek Sahu <absahu@xxxxxxxxxxxxxx>
> > ---
> >   drivers/mtd/nand/qcom_nandc.c | 22 ++++++++++++++++------
> >   1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/qcom_nandc.c 
> > b/drivers/mtd/nand/qcom_nandc.c
> > index 85fbe00..c0c140b 100644
> > --- a/drivers/mtd/nand/qcom_nandc.c
> > +++ b/drivers/mtd/nand/qcom_nandc.c
> > @@ -193,6 +193,9 @@
> >   	      ((size) << READ_LOCATION_SIZE) |			\
> >   	      ((is_last) << READ_LOCATION_LAST))
> >   +/* Returns the actual register address for NAND_FLASH_DEV_* */
>
> There aren't any registers starting with NAND_FLASH_DEV_* in the 
> registers
> defined above, it might get confusing for someone who doesn't have 
> access
> to the HW docs. Could you explicitly mention in this comment all the 
> register
> names that are required to go through this translation, it should make 
> things
> more readable. With that:

 Thanks Archit. Following are the registers

 #define NAND_DEV_CMD0                   0xa0
 #define NAND_DEV_CMD1                   0xa4
 #define NAND_DEV_CMD2                   0xa8
 #define NAND_DEV_CMD_VLD                0xac

 I will update the comment.

> Reviewed-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
>
> Thanks,
> Archit
>
>
> > +#define nandc_dev_addr(nandc, reg) ((nandc)->props->flash_dev_offset 
> > + (reg))
> > +
> >   #define QPIC_PER_CW_CMD_SGL		32
> >   #define QPIC_PER_CW_DATA_SGL		8
> >   @@ -426,10 +429,12 @@ struct qcom_nand_host {
> >    * among different NAND controllers.
> >    * @ecc_modes - ecc mode for NAND
> >    * @is_bam - whether NAND controller is using BAM
> > + * @flash_dev_offset - NAND_FLASH_DEV_* registers start offset
> >    */
> >   struct qcom_nandc_props {
> >   	u32 ecc_modes;
> >   	bool is_bam;
> > +	u32 flash_dev_offset;
> >   };
> >     /* Frees the BAM transaction memory */
> > @@ -844,6 +849,9 @@ static int read_reg_dma(struct 
> > qcom_nand_controller *nandc, int first,
> >   	if (first == NAND_READ_ID || first == NAND_FLASH_STATUS)
> >   		flow_control = true;
> >   +	if (first == NAND_DEV_CMD_VLD || first == NAND_DEV_CMD1)
> > +		first = nandc_dev_addr(nandc, first);
> > +
> >   	size = num_regs * sizeof(u32);
> >   	vaddr = nandc->reg_read_buf + nandc->reg_read_pos;
> >   	nandc->reg_read_pos += num_regs;
> > @@ -881,11 +889,11 @@ static int write_reg_dma(struct 
> > qcom_nand_controller *nandc, int first,
> >   	if (first == NAND_EXEC_CMD)
> >   		flags |= NAND_BAM_NWD;
> >   -	if (first == NAND_DEV_CMD1_RESTORE)
> > -		first = NAND_DEV_CMD1;
> > +	if (first == NAND_DEV_CMD1_RESTORE || first == NAND_DEV_CMD1)
> > +		first = nandc_dev_addr(nandc, NAND_DEV_CMD1);
> >   -	if (first == NAND_DEV_CMD_VLD_RESTORE)
> > -		first = NAND_DEV_CMD_VLD;
> > +	if (first == NAND_DEV_CMD_VLD_RESTORE || first == NAND_DEV_CMD_VLD)
> > +		first = nandc_dev_addr(nandc, NAND_DEV_CMD_VLD);
> >     	size = num_regs * sizeof(u32);
> >   @@ -2492,7 +2500,8 @@ 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);
> > +	nandc_write(nandc, nandc_dev_addr(nandc, NAND_DEV_CMD_VLD),
> > +		    NAND_DEV_CMD_VLD_VAL);
> >     	/* enable ADM or BAM DMA */
> >   	if (nandc->props->is_bam) {
> > @@ -2503,7 +2512,7 @@ static int qcom_nandc_setup(struct 
> > qcom_nand_controller *nandc)
> >   	}
> >     	/* save the original values of these registers */
> > -	nandc->cmd1 = nandc_read(nandc, NAND_DEV_CMD1);
> > +	nandc->cmd1 = nandc_read(nandc, nandc_dev_addr(nandc, 
> > NAND_DEV_CMD1));
> >   	nandc->vld = NAND_DEV_CMD_VLD_VAL;
> >     	return 0;
> > @@ -2752,6 +2761,7 @@ static int qcom_nandc_remove(struct 
> > platform_device *pdev)
> >   static const struct qcom_nandc_props ipq806x_nandc_props = {
> >   	.ecc_modes = (ECC_RS_4BIT | ECC_BCH_8BIT),
> >   	.is_bam = false,
> > +	.flash_dev_offset = 0x0,
> >   };
> >     /*
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html