[RFC PATCH v3 0/3] support 512B ECC step size for Meson NAND

[RFC PATCH v3 0/3] support 512B ECC step size for Meson NAND

[Arseniy Krasnov]

Hello,

this patchset adds support for 512B ECC step size for Meson NAND. Current
implementation only supports 1024B. There are three patches:

1) Update for device tree bindings to replace 'const' type of field
   'nand-ecc-step-size' with 'enum' which contains 512 and 1024.

2) Update for device tree bindings to add dependency between properties
   'nand-ecc-strength' and 'nand-ecc-step-size'.

3) Update for Meson driver - new enum value for 512B ECC and reworked
   ECC capabilities structure to support both 512B and 1024B ECC. By
   default this driver uses 1024B ECC, 512B could be enabled in device
   tree.

Changelog:
v1 -> v2:
 * Add default value of 1024 to the bindings patch (0001).
 * Remove "Acked-by: Rob Herring <robh@kernel.org>" from the bindings
   patch (0001) due to added default value.
 * Remove invalid calculation of OOB bytes, available for ECC engine
   from patch 0002. This logic is incorrect from the origins, so I don't
   touch it in this patchset. It will be fixed by another patch, as in
   fact, it doesn't affect this patchset.

v2 -> v3:
 * Add new patch which adds dependency between 'nand-ecc-strength' and
   'nand-ecc-step-size' (0002).
 * Return "Acked-by: Rob Herring <robh@kernel.org>" to 0001.
 * Remove "default" value from 0001, due to dependency patch.
 * Remove example value from 0001, due to dependency patch.

Links:
v1:
https://lore.kernel.org/linux-mtd/20230628092937.538683-1-AVKrasnov@sberdevices.ru/
v2:
https://lore.kernel.org/linux-mtd/20230705065434.297040-1-AVKrasnov@sberdevices.ru/

Arseniy Krasnov (3):
  dt-bindings: nand: meson: support for 512B ECC step size
  dt-bindings: nand: meson: make ECC properties dependent
  mtd: rawnand: meson: support for 512B ECC step size

 .../bindings/mtd/amlogic,meson-nand.yaml      |  6 ++-
 drivers/mtd/nand/raw/meson_nand.c             | 45 ++++++++++++++-----
 2 files changed, 39 insertions(+), 12 deletions(-)

-- 
2.35.0

[RFC PATCH v3 1/3] dt-bindings: nand: meson: support for 512B ECC step size

[Arseniy Krasnov]

Meson NAND supports both 512B and 1024B ECC step size, so replace
'const' for only 1024B step size with enum for both sizes.

Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
index 28fb9a7dd70f..e81757c9e0ed 100644
--- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
@@ -49,7 +49,7 @@ patternProperties:
         const: hw
 
       nand-ecc-step-size:
-        const: 1024
+        enum: [512, 1024]
 
       nand-ecc-strength:
         enum: [8, 16, 24, 30, 40, 50, 60]
-- 
2.35.0

Re: [RFC PATCH v3 1/3] dt-bindings: nand: meson: support for 512B ECC step size

[Miquel Raynal]

On Tue, 2023-07-11 at 12:21:27 UTC, Arseniy Krasnov wrote:
> Meson NAND supports both 512B and 1024B ECC step size, so replace
> 'const' for only 1024B step size with enum for both sizes.
> 
> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> Acked-by: Rob Herring <robh@kernel.org>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.

Miquel

[RFC PATCH v3 2/3] dt-bindings: nand: meson: make ECC properties dependent

[Arseniy Krasnov]

ECC properties 'nand-ecc-strength' and 'nand-ecc-step-size' depends on
each other, so they must be both either set or not set. In first case
ECC core will try to use these values if possible (by checking ECC caps
provided by driver), in second case ECC core will select most optimal
values for both properties.

Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
---
 Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
index e81757c9e0ed..4f6e56542a7e 100644
--- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
@@ -58,6 +58,10 @@ patternProperties:
             meson-gxl-nfc 8, 16, 24, 30, 40, 50, 60
             meson-axg-nfc 8
 
+    dependencies:
+      nand-ecc-strength: ['nand-ecc-step-size']
+      nand-ecc-step-size: ['nand-ecc-strength']
+
 required:
   - compatible
   - reg
-- 
2.35.0

Re: [RFC PATCH v3 2/3] dt-bindings: nand: meson: make ECC properties dependent

[Rob Herring]

On Tue, Jul 11, 2023 at 03:21:28PM +0300, Arseniy Krasnov wrote:
> ECC properties 'nand-ecc-strength' and 'nand-ecc-step-size' depends on
> each other, so they must be both either set or not set. 

Is that true for all drivers or just meson?

> In first case
> ECC core will try to use these values if possible (by checking ECC caps
> provided by driver), in second case ECC core will select most optimal
> values for both properties.
> 
> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> ---
>  Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> index e81757c9e0ed..4f6e56542a7e 100644
> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
> @@ -58,6 +58,10 @@ patternProperties:
>              meson-gxl-nfc 8, 16, 24, 30, 40, 50, 60
>              meson-axg-nfc 8
>  
> +    dependencies:
> +      nand-ecc-strength: ['nand-ecc-step-size']
> +      nand-ecc-step-size: ['nand-ecc-strength']
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.35.0
> 

Re: [RFC PATCH v3 2/3] dt-bindings: nand: meson: make ECC properties dependent

[Arseniy Krasnov]

On 14.07.2023 18:55, Rob Herring wrote:
> On Tue, Jul 11, 2023 at 03:21:28PM +0300, Arseniy Krasnov wrote:
>> ECC properties 'nand-ecc-strength' and 'nand-ecc-step-size' depends on
>> each other, so they must be both either set or not set. 
> 
> Is that true for all drivers or just meson?

As Miquel Raynal suggested, this will be implemented only for Meson driver at this moment.
Here is link with reply:
https://lore.kernel.org/linux-mtd/20230705065434.297040-1-AVKrasnov@sberdevices.ru/T/#m88705a0039fb30c18a9bb2b03a1add5f7e212a3b

Thanks, Arseniy

> 
>> In first case
>> ECC core will try to use these values if possible (by checking ECC caps
>> provided by driver), in second case ECC core will select most optimal
>> values for both properties.
>>
>> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
>> ---
>>  Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> index e81757c9e0ed..4f6e56542a7e 100644
>> --- a/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml
>> @@ -58,6 +58,10 @@ patternProperties:
>>              meson-gxl-nfc 8, 16, 24, 30, 40, 50, 60
>>              meson-axg-nfc 8
>>  
>> +    dependencies:
>> +      nand-ecc-strength: ['nand-ecc-step-size']
>> +      nand-ecc-step-size: ['nand-ecc-strength']
>> +
>>  required:
>>    - compatible
>>    - reg
>> -- 
>> 2.35.0
>>

Re: [RFC PATCH v3 2/3] dt-bindings: nand: meson: make ECC properties dependent

[Rob Herring]

On Tue, 11 Jul 2023 15:21:28 +0300, Arseniy Krasnov wrote:
> ECC properties 'nand-ecc-strength' and 'nand-ecc-step-size' depends on
> each other, so they must be both either set or not set. In first case
> ECC core will try to use these values if possible (by checking ECC caps
> provided by driver), in second case ECC core will select most optimal
> values for both properties.
> 
> Signed-off-by: Arseniy Krasnov <AVKrasnov@sberdevices.ru>
> ---
>  Documentation/devicetree/bindings/mtd/amlogic,meson-nand.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

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

[Arseniy Krasnov]

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 | 45 +++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/nand/raw/meson_nand.c b/drivers/mtd/nand/raw/meson_nand.c
index 345212e8c691..369e81356240 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;
 		}
-- 
2.35.0

Re: [RFC PATCH v3 0/3] support 512B ECC step size for Meson NAND

[Miquel Raynal]

Hi Arseniy,

AVKrasnov@sberdevices.ru wrote on Tue, 11 Jul 2023 15:21:26 +0300:

> Hello,
> 
> this patchset adds support for 512B ECC step size for Meson NAND. Current
> implementation only supports 1024B. There are three patches:
> 
> 1) Update for device tree bindings to replace 'const' type of field
>    'nand-ecc-step-size' with 'enum' which contains 512 and 1024.
> 
> 2) Update for device tree bindings to add dependency between properties
>    'nand-ecc-strength' and 'nand-ecc-step-size'.
> 
> 3) Update for Meson driver - new enum value for 512B ECC and reworked
>    ECC capabilities structure to support both 512B and 1024B ECC. By
>    default this driver uses 1024B ECC, 512B could be enabled in device
>    tree.

This series does not apply correctly on nand/next, would you mind
rebasing (nand/next on linux-mtd) and sending it again?

BTW the "rfc" prefix is only needed for the "first" submission, when
you try something "new", otherwise it is no longer required.

Thanks,
Miquèl

Re: [RFC PATCH v3 0/3] support 512B ECC step size for Meson NAND

[Arseniy Krasnov]

Hello Miquel!

On 15.07.2023 19:15, Miquel Raynal wrote:
> Hi Arseniy,
> 
> AVKrasnov@sberdevices.ru wrote on Tue, 11 Jul 2023 15:21:26 +0300:
> 
>> Hello,
>>
>> this patchset adds support for 512B ECC step size for Meson NAND. Current
>> implementation only supports 1024B. There are three patches:
>>
>> 1) Update for device tree bindings to replace 'const' type of field
>>    'nand-ecc-step-size' with 'enum' which contains 512 and 1024.
>>
>> 2) Update for device tree bindings to add dependency between properties
>>    'nand-ecc-strength' and 'nand-ecc-step-size'.
>>
>> 3) Update for Meson driver - new enum value for 512B ECC and reworked
>>    ECC capabilities structure to support both 512B and 1024B ECC. By
>>    default this driver uses 1024B ECC, 512B could be enabled in device
>>    tree.
> 
> This series does not apply correctly on nand/next, would you mind
> rebasing (nand/next on linux-mtd) and sending it again?

Sure, as I see 0001 was applied to nand/next, so I can resend only 0002 and 0003,
as 0002 is the first patch which fails to apply?

> 
> BTW the "rfc" prefix is only needed for the "first" submission, when
> you try something "new", otherwise it is no longer required.

Ok, got it

> 
> Thanks,
> Miquèl

Thanks, Arseniy

Re: [RFC PATCH v3 0/3] support 512B ECC step size for Meson NAND

[Miquel Raynal]

Hi Arseniy,

avkrasnov@sberdevices.ru wrote on Sat, 15 Jul 2023 20:48:34 +0300:

> Hello Miquel!
> 
> On 15.07.2023 19:15, Miquel Raynal wrote:
> > Hi Arseniy,
> > 
> > AVKrasnov@sberdevices.ru wrote on Tue, 11 Jul 2023 15:21:26 +0300:
> >   
> >> Hello,
> >>
> >> this patchset adds support for 512B ECC step size for Meson NAND. Current
> >> implementation only supports 1024B. There are three patches:
> >>
> >> 1) Update for device tree bindings to replace 'const' type of field
> >>    'nand-ecc-step-size' with 'enum' which contains 512 and 1024.
> >>
> >> 2) Update for device tree bindings to add dependency between properties
> >>    'nand-ecc-strength' and 'nand-ecc-step-size'.
> >>
> >> 3) Update for Meson driver - new enum value for 512B ECC and reworked
> >>    ECC capabilities structure to support both 512B and 1024B ECC. By
> >>    default this driver uses 1024B ECC, 512B could be enabled in device
> >>    tree.  
> > 
> > This series does not apply correctly on nand/next, would you mind
> > rebasing (nand/next on linux-mtd) and sending it again?  
> 
> Sure, as I see 0001 was applied to nand/next, so I can resend only 0002 and 0003,
> as 0002 is the first patch which fails to apply?

Yes indeed.

> 
> > 
> > BTW the "rfc" prefix is only needed for the "first" submission, when
> > you try something "new", otherwise it is no longer required.  
> 
> Ok, got it
> 
> > 
> > Thanks,
> > Miquèl  
> 
> Thanks, Arseniy


Thanks,
Miquèl