linux-kernel - Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2abac8fb-28ac-5137-70cc-47cbd20613b7@codeaurora.org>
Date:   Thu, 11 Jun 2020 09:57:59 +0530
From:   Sivaprakash Murugesan <sivaprak@...eaurora.org>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     richard@....at, vigneshr@...com, peter.ujfalusi@...com,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set
 already

Hi Miquel,

Thanks for the review.

On 6/9/2020 7:33 PM, Miquel Raynal wrote:
> Hi Sivaprakash,
>
> Sivaprakash Murugesan <sivaprak@...eaurora.org> wrote on Tue,  9 Jun
> 2020 16:40:56 +0530:
>
>> BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
>> NAND_CTRL is an operational register and in BAM mode operational
>> registers are read only.
>>
>> So, before writing into NAND_CTRL register check if BAM mode is already
>> enabled by bootloader, and set BAM mode only if it is not set already.
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@...eaurora.org>
>> ---
>>   drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index e0afa2c..7740059 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
>>   	/* 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);
>> +		/* NAND_CTRL is an operational registers, and CPU
>> +		 * access to operational registers are read only
>> +		 * in BAM mode. So update the NAND_CTRL register
>> +		 * only if it is not in BAM mode. In most cases BAM
>> +		 * mode will be enabled in 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);
>>   	}
> Does this currently produces an issue at runtime?
>
> If yes, you should have a Fixes/CC: stable pair of tags.
>
> Also, what is BAM mode? Please tell us in the commit log.

Currently this is not causing any issue on run time.

The writes to this register is silently ignored.

However, this could be an issue in future Hardware designs.

BAM is the DMA engine on QCOM IPQ platforms, sure will explain this

mode in next patchset.

>
> Thanks,
> Miquèl