linux-kernel - Re: [PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC base address to BAM

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: <abe02188-9cb8-aa4b-9723-372000e08110@quicinc.com>
Date: Thu, 20 Mar 2025 11:29:01 +0530
From: Md Sadre Alam <quic_mdalam@...cinc.com>
To: Gabor Juhos <j4g8y7@...il.com>, <manivannan.sadhasivam@...aro.org>,
        <miquel.raynal@...tlin.com>, <richard@....at>, <vigneshr@...com>,
        <broonie@...nel.org>, <bbrezillon@...nel.org>,
        <linux-mtd@...ts.infradead.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-spi@...r.kernel.org>
Subject: Re: [PATCH v3 1/4] mtd: rawnand: qcom: Pass 18 bit offset from QPIC
 base address to BAM



On 3/18/2025 8:22 PM, Gabor Juhos wrote:
> 2025. 03. 10. 13:09 keltezéssel, Md Sadre Alam írta:
>> Currently we are configuring lower 24 bits of address in descriptor
>> whereas QPIC design expects 18 bit register offset from QPIC base
>> address to be configured in cmd descriptors. This is leading to a
>> different address actually being used in HW, leading to wrong value
>> read.
>>
>> the actual issue is that the NANDc base address is different from the
>> QPIC base address. But the driver doesn't take it into account and just
>> used the QPIC base as the NANDc base. This used to work as the NANDc IP
>> only considers the lower 18 bits of the address passed by the driver to
>> derive the register offset. Since the base address of QPIC used to contain
>> all 0 for lower 18 bits (like 0x07980000), the driver ended up passing the
>> actual register offset in it and NANDc worked properly. But on newer SoCs
>> like SDX75, the QPIC base address doesn't contain all 0 for lower 18 bits
>> (like 0x01C98000). So NANDc sees wrong offset as per the current logic
>>
>> The address should be passed to BAM 0x30000 + offset. In older targets
>> the lower 18-bits are zero so that correct address being paased. But
>> in newer targets the lower 18-bits are non-zero in QPIC base so that
>> 0x300000 + offset giving the wrong value.
>>
>> SDX75 : QPIC_QPIC | 0x01C98000 (Lower 18 bits are non zero)
>> SDX55 : QPIC_QPIC | 0x1B00000 (Lower 18 bits are zero) Same for
>> older targets.
>>
>> Cc: stable@...r.kernel.org
>> Fixes: 8d6b6d7e135e ("mtd: nand: qcom: support for command descriptor formation")
>> Tested-by: Lakshmi Sowjanya D <quic_laksd@...cinc.com>
>> Signed-off-by: Md Sadre Alam <quic_mdalam@...cinc.com>
>> ---
> 
> <...>
> 
>>   /*
>> diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
>> index cd7172e6c1bb..6268f08b9d19 100644
>> --- a/include/linux/mtd/nand-qpic-common.h
>> +++ b/include/linux/mtd/nand-qpic-common.h
>> @@ -200,7 +200,7 @@
>>   #define dev_cmd_reg_addr(nandc, reg) ((nandc)->props->dev_cmd_reg_start + (reg))
>>   
>>   /* Returns the NAND register physical address */
>> -#define nandc_reg_phys(chip, offset) ((chip)->base_phys + (offset))
>> +#define nandc_reg_phys(chip, offset)  ((nandc)->props->nandc_offset + (offset))
> 
> The macro has no parameter named 'nandc', so this works only when there is an
> identifier with that name in the code where the macro is used.
> 
> Additionally, the macro will no longer return the physical address of a register
> after the change, so both the comment before the macro and the name of the macro
> will be misleading.
> 
> Since the macro is used only in the qcom_prep_bam_dma_desc_cmd() function to
> compute the 'addr' parameter for the bam_prep_ce{_le32}() functions, maybe it
> would be better to get rid of it completely, and do the computation directly in
> the function instead.
Ok, Will handle in next revision.
> 
> Regards,
> Gabor