Re: [PATCH 11/24] mtd: rawnand: sunxi: cosmetic: add has_ecc_block_512 capability

[Richard GENOUD <richard.genoud@bootlin.com>]

Hi,
Le 18/10/2025 à 01:11, Andre Przywara a écrit :
> Hi,
> 
> On 16/10/2025 15:27, Richard Genoud wrote:
>> Introduce has_ecc_block_512 capability
>>
>> The H616 controller can't handle 512 bytes ECC block size.
>> Let it be a capability.
>>
>> No functional change.
>>
>> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
>> ---
>>   drivers/mtd/nand/raw/sunxi_nand.c | 14 +++++++++++---
>>   drivers/mtd/nand/raw/sunxi_nand.h |  2 ++
>>   2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/ 
>> sunxi_nand.c
>> index 869b3ddd971c..fddb1cada023 100644
>> --- a/drivers/mtd/nand/raw/sunxi_nand.c
>> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
>> @@ -660,11 +660,12 @@ static void sunxi_nfc_hw_ecc_enable(struct 
>> mtd_info *mtd)
>>       u32 ecc_ctl;
>>       ecc_ctl = readl(nfc->regs + NFC_REG_ECC_CTL);
>> -    ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE |
>> -             NFC_ECC_BLOCK_SIZE_MSK);
>> +    ecc_ctl &= ~(NFC_ECC_MODE_MSK | NFC_ECC_PIPELINE);
>> +    if (nfc->caps->has_ecc_block_512)
>> +        ecc_ctl &= ~NFC_ECC_BLOCK_SIZE_MSK;
> 
> So if I get this correctly, then NFC_ECC_BLOCK_SIZE_MSK and 
> NFC_ECC_BLOCK_512 are referring to the same single bit register field, 
> in bit 5? Can we lose the MSK definition then, and clear the field 
> unconditionally? Or is it that on the H6/H616 we must never touch bit 5, 
> because it refers to something else (NDFC_RANDOM_EN?)?
> In any case I think we don't need identical MSK and BLOCK_512 
> definitions, do we?
Yes, 2nd case:
On A23/A10, the bit 5 (NFC_ECC_BLOCK_512) in NDFC_ECC_CTL register 
(0x34) is controlling the size of one ECC data block (0: 1024bytes 1:512 
bytes).
On H6, this bit NFC_ECC_BLOCK_512 doesn't exists anymore, but instead, 
bit 5 of NDFC_ECC_CTL register (0x34) is NDFC_RANDOM_EN (which was 
itself on bit 9 for A10/A23)
So yes, we can definitely loose NFC_ECC_BLOCK_SIZE_MSK, and keep 
has_ecc_block_512 to differentiate A10 from H6.

> 
> By the way: Do you have any MMIO frame register description for the old 
> NAND device? I checked some of the older manuals, but the ones I looked 
> at were only showing timing diagrams, but no register map.
Yes, the NAND controller in A33 is the same (AFAIK) than on A23:
https://linux-sunxi.org/images/4/49/A33_Nand_Flash_Controller_Specification.pdf


Thanks!

> 
> Cheers,
> Andre
> 
>>       ecc_ctl |= NFC_ECC_EN | NFC_ECC_MODE(data->mode) | 
>> NFC_ECC_EXCEPTION;
>> -    if (nand->ecc.size == 512)
>> +    if (nand->ecc.size == 512 && nfc->caps->has_ecc_block_512)
>>           ecc_ctl |= NFC_ECC_BLOCK_512;
>>       writel(ecc_ctl, nfc->regs + NFC_REG_ECC_CTL);
>> @@ -1454,6 +1455,8 @@ static void sunxi_nand_ecc_cleanup(struct 
>> nand_ecc_ctrl *ecc)
>>   static int sunxi_nand_ecc_init(struct mtd_info *mtd, struct 
>> nand_ecc_ctrl *ecc)
>>   {
>>       struct nand_chip *nand = mtd_to_nand(mtd);
>> +    struct sunxi_nand_chip *sunxi_nand = to_sunxi_nand(nand);
>> +    struct sunxi_nfc *nfc = to_sunxi_nfc(sunxi_nand->nand.controller);
>>       int ret;
>>       if (!ecc->size) {
>> @@ -1464,6 +1467,10 @@ static int sunxi_nand_ecc_init(struct mtd_info 
>> *mtd, struct nand_ecc_ctrl *ecc)
>>       if (!ecc->size || !ecc->strength)
>>           return -EINVAL;
>> +    /* If 512B ECC is not supported, switch to 1024 */
>> +    if (ecc->size == 512 && !nfc->caps->has_ecc_block_512)
>> +        ecc->size = 1024;
>> +
>>       switch (ecc->mode) {
>>       case NAND_ECC_SOFT_BCH:
>>           break;
>> @@ -1715,6 +1722,7 @@ static int sunxi_nand_probe(struct udevice *dev)
>>   }
>>   static const struct sunxi_nfc_caps sunxi_nfc_a10_caps = {
>> +    .has_ecc_block_512 = true,
>>       .nstrengths = 9,
>>       .reg_ecc_err_cnt = NFC_REG_A10_ECC_ERR_CNT,
>>       .reg_user_data = NFC_REG_A10_USER_DATA,
>> diff --git a/drivers/mtd/nand/raw/sunxi_nand.h b/drivers/mtd/nand/raw/ 
>> sunxi_nand.h
>> index 35079d37bb1f..be294d7cea0a 100644
>> --- a/drivers/mtd/nand/raw/sunxi_nand.h
>> +++ b/drivers/mtd/nand/raw/sunxi_nand.h
>> @@ -169,6 +169,7 @@
>>    * NAND Controller capabilities structure: stores NAND controller 
>> capabilities
>>    * for distinction between compatible strings.
>>    *
>> + * @has_ecc_block_512:    If the ECC can handle 512B or only 1024B 
>> chuncks
>>    * @nstrengths:        Number of element of ECC strengths array
>>    * @reg_ecc_err_cnt:    ECC error counter register
>>    * @reg_user_data:    User data register
>> @@ -176,6 +177,7 @@
>>    * @pat_found_mask:    ECC_PAT_FOUND mask in NFC_REG_PAT_FOUND register
>>    */
>>   struct sunxi_nfc_caps {
>> +    bool has_ecc_block_512;
>>       unsigned int nstrengths;
>>       unsigned int reg_ecc_err_cnt;
>>       unsigned int reg_user_data;
> 


-- 
Richard Genoud, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com