Re: [RFC PATCH v1 2/2] mtd: rawnand: meson: support for 512B ECC step size

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
Cc: Liang Yang <liang.yang@amlogic.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Jerome Brunet <jbrunet@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	<oxffffaa@gmail.com>, <kernel@sberdevices.ru>,
	<linux-mtd@lists.infradead.org>, <devicetree@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-amlogic@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH v1 2/2] mtd: rawnand: meson: support for 512B ECC step size
Date: Tue, 4 Jul 2023 10:36:17 +0200	[thread overview]
Message-ID: <20230704103617.4affae8a@xps-13> (raw)
In-Reply-To: <20230628092937.538683-3-AVKrasnov@sberdevices.ru>

Hi Arseniy,

AVKrasnov@sberdevices.ru wrote on Wed, 28 Jun 2023 12:29:36 +0300:

> Meson NAND supports both 512B and 1024B ECC step size.
> 
> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> ---
>  drivers/mtd/nand/raw/meson_nand.c | 47 +++++++++++++++++++++++--------
>  1 file changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
> index 345212e8c691..6cc4f63b86c8 100644
> --- a/drivers/mtd/nand/raw/meson_nand.c
> +++ b/drivers/mtd/nand/raw/meson_nand.c
> @@ -135,6 +135,7 @@ struct meson_nfc_nand_chip {
>  struct meson_nand_ecc {
>  	u32 bch;
>  	u32 strength;
> +	u32 size;
>  };
>  
>  struct meson_nfc_data {
> @@ -190,7 +191,8 @@ struct meson_nfc {
>  };
>  
>  enum {
> -	NFC_ECC_BCH8_1K		= 2,
> +	NFC_ECC_BCH8_512	= 1,
> +	NFC_ECC_BCH8_1K,
>  	NFC_ECC_BCH24_1K,
>  	NFC_ECC_BCH30_1K,
>  	NFC_ECC_BCH40_1K,
> @@ -198,15 +200,16 @@ enum {
>  	NFC_ECC_BCH60_1K,
>  };
>  
> -#define MESON_ECC_DATA(b, s)	{ .bch = (b),	.strength = (s)}
> +#define MESON_ECC_DATA(b, s, sz)	{ .bch = (b), .strength = (s), .size = (sz) }
>  
>  static struct meson_nand_ecc meson_ecc[] = {
> -	MESON_ECC_DATA(NFC_ECC_BCH8_1K, 8),
> -	MESON_ECC_DATA(NFC_ECC_BCH24_1K, 24),
> -	MESON_ECC_DATA(NFC_ECC_BCH30_1K, 30),
> -	MESON_ECC_DATA(NFC_ECC_BCH40_1K, 40),
> -	MESON_ECC_DATA(NFC_ECC_BCH50_1K, 50),
> -	MESON_ECC_DATA(NFC_ECC_BCH60_1K, 60),
> +	MESON_ECC_DATA(NFC_ECC_BCH8_512, 8,  512),
> +	MESON_ECC_DATA(NFC_ECC_BCH8_1K,  8,  1024),
> +	MESON_ECC_DATA(NFC_ECC_BCH24_1K, 24, 1024),
> +	MESON_ECC_DATA(NFC_ECC_BCH30_1K, 30, 1024),
> +	MESON_ECC_DATA(NFC_ECC_BCH40_1K, 40, 1024),
> +	MESON_ECC_DATA(NFC_ECC_BCH50_1K, 50, 1024),
> +	MESON_ECC_DATA(NFC_ECC_BCH60_1K, 60, 1024),
>  };
>  
>  static int meson_nand_calc_ecc_bytes(int step_size, int strength)
> @@ -224,8 +227,27 @@ static int meson_nand_calc_ecc_bytes(int step_size, int strength)
>  
>  NAND_ECC_CAPS_SINGLE(meson_gxl_ecc_caps,
>  		     meson_nand_calc_ecc_bytes, 1024, 8, 24, 30, 40, 50, 60);
> -NAND_ECC_CAPS_SINGLE(meson_axg_ecc_caps,
> -		     meson_nand_calc_ecc_bytes, 1024, 8);
> +
> +static const int axg_stepinfo_strengths[] = { 8 };
> +static const struct nand_ecc_step_info axg_stepinfo_1024 = {
> +	.stepsize = 1024,
> +	.strengths = axg_stepinfo_strengths,
> +	.nstrengths = ARRAY_SIZE(axg_stepinfo_strengths)
> +};
> +
> +static const struct nand_ecc_step_info axg_stepinfo_512 = {
> +	.stepsize = 512,
> +	.strengths = axg_stepinfo_strengths,
> +	.nstrengths = ARRAY_SIZE(axg_stepinfo_strengths)
> +};
> +
> +static const struct nand_ecc_step_info axg_stepinfo[] = { axg_stepinfo_1024, axg_stepinfo_512 };
> +
> +static const struct nand_ecc_caps meson_axg_ecc_caps = {
> +	.stepinfos = axg_stepinfo,
> +	.nstepinfos = ARRAY_SIZE(axg_stepinfo),
> +	.calc_ecc_bytes = meson_nand_calc_ecc_bytes,
> +};
>  
>  static struct meson_nfc_nand_chip *to_meson_nand(struct nand_chip *nand)
>  {
> @@ -1259,7 +1281,8 @@ static int meson_nand_bch_mode(struct nand_chip *nand)
>  		return -EINVAL;
>  
>  	for (i = 0; i < ARRAY_SIZE(meson_ecc); i++) {
> -		if (meson_ecc[i].strength == nand->ecc.strength) {
> +		if (meson_ecc[i].strength == nand->ecc.strength &&
> +		    meson_ecc[i].size == nand->ecc.size) {
>  			meson_chip->bch_mode = meson_ecc[i].bch;
>  			return 0;
>  		}
> @@ -1278,7 +1301,7 @@ static int meson_nand_attach_chip(struct nand_chip *nand)
>  	struct meson_nfc *nfc = nand_get_controller_data(nand);
>  	struct meson_nfc_nand_chip *meson_chip = to_meson_nand(nand);
>  	struct mtd_info *mtd = nand_to_mtd(nand);
> -	int nsectors = mtd->writesize / 1024;
> +	int nsectors = mtd->writesize / 512;

This cannot be unconditional, right?

>  	int raw_writesize;
>  	int ret;
>  

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel