Re: [PATCH] mtd: rawnand: sunxi: reject invalid ECC step sizes

From: James Hilliard

Date: Fri Sep 04 2026 - 13:37:42 EST


On Fri, Sep 4, 2026 at 9:25 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
>
> On 11/08/2026 at 00:06:48 -06, James Hilliard <james.hilliard1@xxxxxxxxx> wrote:
>
> > ECC maximization forces a 1024-byte ECC step and divides the
> > available OOB bytes by the number of steps. A NAND with a smaller
> > page therefore produces zero steps and a division by zero.
>
> No, steps cannot be < 1. If they are, it's a bug that must be fixed.

The zero step count is created by the Sunxi maximization path itself.
sunxi_nand_hw_ecc_ctrl_init() unconditionally sets ecc->size to 1024,
including for a 512-byte-page NAND, before calculating nsectors.

The controller attach callback runs before nand_scan_tail(), so this
division happens before the generic ECC handling can reject the
configuration or fall back to software ECC.

For this case, would you expect the Sunxi maximization path to retain a
512-byte ECC step on 512-byte-page NANDs, rather than selecting 1024 and
then validating the resulting step count?

> > An explicitly configured ECC step which is larger than, or does not
> > divide, the page also produces an unusable step count before the NAND
> > core can diagnose the configuration.
> >
> > Validate the step size at both points where the driver derives the
> > number of sectors and reject invalid geometries.
> >
> > Fixes: 4796d8655915 ("mtd: nand: sunxi: Support ECC maximization")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> > ---
> > drivers/mtd/nand/raw/sunxi_nand.c | 17 +++++++++++++++--
> > 1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> > index 45ccbce91551..ad314c0e30b2 100644
> > --- a/drivers/mtd/nand/raw/sunxi_nand.c
> > +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> > @@ -2032,6 +2032,15 @@ static void sunxi_nand_detach_chip(struct nand_chip *nand)
> > sunxi_nand->user_data_bytes = NULL;
> > }
> >
> > +static int sunxi_nfc_ecc_steps(struct mtd_info *mtd, unsigned int step_size)
> > +{
> > + if (!step_size || mtd->writesize < step_size ||
> > + mtd->writesize % step_size)
>
> I don't get it, mtd->writesize < step_size is conceptually
> impossible. If that happens, it must be fixed earlier than that.

An explicitly configured step reaches the controller attach callback
after the NAND geometry has been detected but before nand_scan_tail().
The Sunxi binding permits step sizes of 512 and 1024 without constraining
them against the detected page size.

Could you clarify where you expect this invariant to be established:
in the raw NAND core before ->attach_chip(), or in the Sunxi driver when
it selects or consumes the ECC configuration?

> > + return -EINVAL;
> > +
> > + return mtd->writesize / step_size;
> > +}
> > +
> > static int sunxi_nfc_maximize_user_data(struct nand_chip *nand, uint32_t oobsize,
> > int ecc_bytes, int nsectors)
> > {
>
> Thanks,
> Miquèl