Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org AF7D7C433CA
Subject: Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set
 already
To:     Miquel Raynal <miquel.raynal@bootlin.com>
Cc:     richard@nod.at, vigneshr@ti.com, peter.ujfalusi@ti.com,
        linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
References: <1591701056-3944-1-git-send-email-sivaprak@codeaurora.org>
 <1591701056-3944-3-git-send-email-sivaprak@codeaurora.org>
 <20200609160352.60cbad80@xps13>
From:   Sivaprakash Murugesan <sivaprak@codeaurora.org>
Message-ID: <2abac8fb-28ac-5137-70cc-47cbd20613b7@codeaurora.org>
Date:   Thu, 11 Jun 2020 09:57:59 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.1
MIME-Version: 1.0
In-Reply-To: <20200609160352.60cbad80@xps13>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Miquel,

Thanks for the review.

On 6/9/2020 7:33 PM, Miquel Raynal wrote:
> Hi Sivaprakash,
>
> Sivaprakash Murugesan <sivaprak@codeaurora.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@codeaurora.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