1. Patchew
  2. linux
  3. [RFC PATCH v1 0/2] support 512B ECC step size for Meson NAND
  4. View patch

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

Arseniy Krasnov posted 2 patches 2 years, 7 months ago
There is a newer version of this series
[RFC PATCH v1 2/2] mtd: rawnand: meson: support for 512B ECC step size
Posted by Arseniy Krasnov 2 years, 7 months ago 
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;
 	int raw_writesize;
 	int ret;
 
-- 
2.35.0
Re: [RFC PATCH v1 2/2] mtd: rawnand: meson: support for 512B ECC step size
Posted by Miquel Raynal 2 years, 7 months ago 
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
Re: [RFC PATCH v1 2/2] mtd: rawnand: meson: support for 512B ECC step size
Posted by Arseniy Krasnov 2 years, 7 months ago 

On 04.07.2023 11:36, Miquel Raynal wrote:
> 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?

Hello Miquel!

Yes, this code looks strange. 'nsectors' is used to calculate space in OOB
that could be used by ECC engine (this value will be passed as 'oobavail'
to 'nand_ecc_choose_conf()'). Idea of 512 is to consider "worst" case
for ECC, e.g. minimal number of bytes for ECC engine (and at the same time
maximum number of free bytes). For Meson, if ECC step size is 512, then we
have 4 x 2 free bytes in OOB (if step size if 1024 then we have 2 x 2 free
bytes in OOB).

I think this code could be reworked in the following way:

if ECC step size is already known here (from DTS), calculate 'nsectors' using
given value (div by 512 for example). Otherwise calculate 'nsectors' in the
current manner:

int nsectors = mtd->writesize / 1024;

Moreover 1024 is default ECC step size for this driver, so default behaviour
will be preserved.

Thanks, Arseniy

> 
>>  	int raw_writesize;
>>  	int ret;
>>  
> 
> 
> Thanks,
> Miquèl
Re: [RFC PATCH v1 2/2] mtd: rawnand: meson: support for 512B ECC step size
Posted by Miquel Raynal 2 years, 7 months ago 
Hi Arseniy,

avkrasnov@sberdevices.ru wrote on Tue, 4 Jul 2023 12:23:03 +0300:

> On 04.07.2023 11:36, Miquel Raynal wrote:
> > 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?  
> 
> Hello Miquel!
> 
> Yes, this code looks strange. 'nsectors' is used to calculate space in OOB
> that could be used by ECC engine (this value will be passed as 'oobavail'
> to 'nand_ecc_choose_conf()'). Idea of 512 is to consider "worst" case
> for ECC, e.g. minimal number of bytes for ECC engine (and at the same time
> maximum number of free bytes). For Meson, if ECC step size is 512, then we
> have 4 x 2 free bytes in OOB (if step size if 1024 then we have 2 x 2 free
> bytes in OOB).
> 
> I think this code could be reworked in the following way:
> 
> if ECC step size is already known here (from DTS), calculate 'nsectors' using
> given value (div by 512 for example). Otherwise calculate 'nsectors' in the
> current manner:

It will always be known when these function are run. There is no
guessing here.

> 
> int nsectors = mtd->writesize / 1024;
> 
> Moreover 1024 is default ECC step size for this driver, so default behaviour
> will be preserved.

Yes, otherwise you would break existing users.

> 
> Thanks, Arseniy
> 
> >   
> >>  	int raw_writesize;
> >>  	int ret;
> >>    
> > 
> > 
> > Thanks,
> > Miquèl  


Thanks,
Miquèl

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.