linux-kernel - Re: [PATCH] mtd: raw: qcom_nand: Fix register write error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200608143739.368f2b53@xps13>
Date:   Mon, 8 Jun 2020 14:37:39 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Sivaprakash Murugesan <sivaprak@...eaurora.org>
Cc:     richard@....at, vigneshr@...com, peter.ujfalusi@...com,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: raw: qcom_nand: Fix register write error

Hi Sivaprakash,

Sivaprakash Murugesan <sivaprak@...eaurora.org> wrote on Mon,  8 Jun
2020 16:17:34 +0530:

Subject prefix should be mtd: rawnand: qcom:

And I don't think "Fix register write error" is relevant in any of the
two following cases.

> 1. SFLASHC_BURST_CFG register is not available on all ipq nand platforms,
>    it is available only on ipq8064 devices and the nand controller works
>    without configuring these registers in this platform, so register
>    write to this can be removed.
> 
> 2. Once BAM mode is enabled register writes to NAND_CTRL should be
>    performed through BAM command descriptors. The NAND BAM mode will
>    be enabled by bootloaders. Check if BAM mode is already enabled and
>    enable it only if not enabled already.
> 

It looks like there are two completely different changes that you are
doing here, please split.

Also, please explain why #2 is needed, it is not very clear.

Thanks,
Miquèl

> Signed-off-by: Sivaprakash Murugesan <sivaprak@...eaurora.org>
> ---
>  drivers/mtd/nand/raw/qcom_nandc.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 5b11c70..7bfd93a 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -36,7 +36,6 @@
>  #define	NAND_DEV_CMD1			0xa4
>  #define	NAND_DEV_CMD2			0xa8
>  #define	NAND_DEV_CMD_VLD		0xac
> -#define	SFLASHC_BURST_CFG		0xe0
>  #define	NAND_ERASED_CW_DETECT_CFG	0xe8
>  #define	NAND_ERASED_CW_DETECT_STATUS	0xec
>  #define	NAND_EBI2_ECC_BUF_CFG		0xf0
> @@ -2774,14 +2773,20 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
>  	u32 nand_ctrl;
>  
>  	/* kill onenand */
> -	nandc_write(nandc, SFLASHC_BURST_CFG, 0);
>  	nandc_write(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD_VLD),
>  		    NAND_DEV_CMD_VLD_VAL);
>  
>  	/* enable ADM or BAM DMA */
>  	if (nandc->props->is_bam) {
>  		nand_ctrl = nandc_read(nandc, NAND_CTRL);
> -		nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> +		/* Once BAM_MODE_EN bit is set, writes to the NAND_CTRL
> +		 * should be done through BAM command descriptors.
> +		 * in most cases bootloader enables the bam mode we
> +		 * need to set the BAM mode only if it is not set by
> +		 * bootloader
> +		 */
> +		if (!(nand_ctrl & BAM_MODE_EN))
> +			nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
>  	} else {
>  		nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
>  	}