Re: [PATCH V3 1/2] mtd: rawnand: qcom: avoid write to unavailable register

[Miquel Raynal]

Hi Sivaprakash,

Sivaprakash Murugesan <sivaprak@xxxxxxxxxxxxxx> wrote on Fri, 12 Jun
2020 12:19:48 +0530:

> SFLASHC_BURST_CFG is only available on older ipq nand platforms, this
> register has been removed when the NAND controller is moved as part of qpic
> controller.
> 
> avoid register writes to this register on devices which are based on qpic

Avoid writing this register on ...

> NAND controllers.
> 
> Fixes: a0637834 (mtd: nand: qcom: support for IPQ4019 QPIC NANDcontroller)
> Fixes: dce84760 (mtd: nand: qcom: Support for IPQ8074 QPIC NAND controller)

I don't think having two Fixes tag is allowed. Take the older one
instead.

> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Sivaprakash Murugesan <sivaprak@xxxxxxxxxxxxxx>
> ---
> [V3]
>  * Addressed Miquel comments, added flag based on nand controller hw
>    to avoid the register writes to specific ipq platforms
>  drivers/mtd/nand/raw/qcom_nandc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index f1daf33..e0c55bb 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -459,11 +459,13 @@ struct qcom_nand_host {
>   * among different NAND controllers.
>   * @ecc_modes - ecc mode for NAND
>   * @is_bam - whether NAND controller is using BAM
> + * @is_qpic - whether NAND CTRL is part of qpic IP
>   * @dev_cmd_reg_start - NAND_DEV_CMD_* registers starting offset
>   */
>  struct qcom_nandc_props {
>  	u32 ecc_modes;
>  	bool is_bam;
> +	bool is_qpic;
>  	u32 dev_cmd_reg_start;
>  };
>  
> @@ -2774,7 +2776,8 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
>  	u32 nand_ctrl;
>  
>  	/* kill onenand */
> -	nandc_write(nandc, SFLASHC_BURST_CFG, 0);
> +	if (!nandc->props->is_qpic)
> +		nandc_write(nandc, SFLASHC_BURST_CFG, 0);
>  	nandc_write(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD_VLD),
>  		    NAND_DEV_CMD_VLD_VAL);
>  
> @@ -3029,18 +3032,21 @@ 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,
> +	.is_qpic = false,

This line is unneeded.

>  	.dev_cmd_reg_start = 0x0,
>  };
>  
>  static const struct qcom_nandc_props ipq4019_nandc_props = {
>  	.ecc_modes = (ECC_BCH_4BIT | ECC_BCH_8BIT),
>  	.is_bam = true,
> +	.is_qpic = true,
>  	.dev_cmd_reg_start = 0x0,
>  };
>  
>  static const struct qcom_nandc_props ipq8074_nandc_props = {
>  	.ecc_modes = (ECC_BCH_4BIT | ECC_BCH_8BIT),
>  	.is_bam = true,
> +	.is_qpic = true,
>  	.dev_cmd_reg_start = 0x7000,
>  };
>  

Much better patch IMHO, just a few nits and we'll be good.

Thanks,
MiquÃl