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

[Andre Przywara <andre.przywara@arm.com>]

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?

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.

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;