[4/7] mtd: nand: samsung: Retrieve ECC requirements from extended

Message ID 20220727093748.1415135-5-michael@amarulasolutions.com
State New
Headers show
Series
  • NAND new improvements
Related show

Commit Message

Michael Trimarchi July 27, 2022, 9:37 a.m. UTC
Upstream linux commit 8fc82d456e40a0.

On some nand controllers with hw-ecc the controller code wants to know
the ecc strength and size and having these as 0, 0 is not accepted.

Specifying these in devicetree is possible but undesirable as the nand
may be different in different production runs of the same board, so it
is better to get this info from the nand id where possible.

This commit adds code to read the ecc strength and size from the nand
for Samsung extended-id nands. This code is based on the info for the 5th
id byte in the datasheets for the following Samsung nands: K9GAG08U0E,
K9GAG08U0F, K9GAG08X0D, K9GBG08U0A, K9GBG08U0B. These all use these bits
in the exact same way.

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/mtd/nand/raw/nand_samsung.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Dario Binacchi July 27, 2022, 12:21 p.m. UTC | #1
Hi Michael,

On Wed, Jul 27, 2022 at 11:37 AM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Upstream linux commit 8fc82d456e40a0.
>
> On some nand controllers with hw-ecc the controller code wants to know
> the ecc strength and size and having these as 0, 0 is not accepted.
>
> Specifying these in devicetree is possible but undesirable as the nand
> may be different in different production runs of the same board, so it
> is better to get this info from the nand id where possible.
>
> This commit adds code to read the ecc strength and size from the nand
> for Samsung extended-id nands. This code is based on the info for the 5th
> id byte in the datasheets for the following Samsung nands: K9GAG08U0E,
> K9GAG08U0F, K9GAG08X0D, K9GBG08U0A, K9GBG08U0B. These all use these bits
> in the exact same way.
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/nand_samsung.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c
> index 0ab8062193..36ef48e5ec 100644
> --- a/drivers/mtd/nand/raw/nand_samsung.c
> +++ b/drivers/mtd/nand/raw/nand_samsung.c
> @@ -64,6 +64,26 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
>                 extid >>= 2;
>                 mtd->erasesize = (128 * 1024) <<
>                                  (((extid >> 1) & 0x04) | (extid & 0x03));
> +
> +               /* Extract ECC requirements from 5th id byte*/
> +               extid = (chip->id.data[4] >> 4) & 0x07;
> +               if (extid < 5) {
> +                       chip->ecc_step_ds = 512;
> +                       chip->ecc_strength_ds = 1 << extid;
> +               } else {
> +                       chip->ecc_step_ds = 1024;
> +                       switch (extid) {
> +                       case 5:
> +                               chip->ecc_strength_ds = 24;
> +                               break;
> +                       case 6:
> +                               chip->ecc_strength_ds = 40;
> +                               break;
> +                       case 7:
> +                               chip->ecc_strength_ds = 60;
> +                               break;
> +                       }
> +               }
>         } else {
>                 nand_decode_ext_id(chip);
>         }
> --
> 2.34.1
>

looks good to me.

Dario
Tommaso Merciai July 28, 2022, 8 a.m. UTC | #2
On Wed, Jul 27, 2022 at 11:37:45AM +0200, Michael Trimarchi wrote:
> Upstream linux commit 8fc82d456e40a0.
> 
> On some nand controllers with hw-ecc the controller code wants to know
> the ecc strength and size and having these as 0, 0 is not accepted.
> 
> Specifying these in devicetree is possible but undesirable as the nand
> may be different in different production runs of the same board, so it
> is better to get this info from the nand id where possible.
> 
> This commit adds code to read the ecc strength and size from the nand
> for Samsung extended-id nands. This code is based on the info for the 5th
> id byte in the datasheets for the following Samsung nands: K9GAG08U0E,
> K9GAG08U0F, K9GAG08X0D, K9GBG08U0A, K9GBG08U0B. These all use these bits
> in the exact same way.
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/nand_samsung.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c
> index 0ab8062193..36ef48e5ec 100644
> --- a/drivers/mtd/nand/raw/nand_samsung.c
> +++ b/drivers/mtd/nand/raw/nand_samsung.c
> @@ -64,6 +64,26 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
>  		extid >>= 2;
>  		mtd->erasesize = (128 * 1024) <<
>  				 (((extid >> 1) & 0x04) | (extid & 0x03));
> +
> +		/* Extract ECC requirements from 5th id byte*/
> +		extid = (chip->id.data[4] >> 4) & 0x07;
> +		if (extid < 5) {

Just for curiosity, 5 is for?
Maybe is better to use a define and avoid magic number?

> +			chip->ecc_step_ds = 512;
> +			chip->ecc_strength_ds = 1 << extid;
> +		} else {
> +			chip->ecc_step_ds = 1024;
> +			switch (extid) {
> +			case 5:
> +				chip->ecc_strength_ds = 24;
> +				break;
> +			case 6:
> +				chip->ecc_strength_ds = 40;
> +				break;
> +			case 7:
> +				chip->ecc_strength_ds = 60;
> +				break;
> +			}
> +		}
>  	} else {
>  		nand_decode_ext_id(chip);
>  	}
> -- 
> 2.34.1
>
Tommaso Merciai July 28, 2022, 8:05 a.m. UTC | #3
On Wed, Jul 27, 2022 at 11:37:45AM +0200, Michael Trimarchi wrote:
> Upstream linux commit 8fc82d456e40a0.
> 
> On some nand controllers with hw-ecc the controller code wants to know
> the ecc strength and size and having these as 0, 0 is not accepted.
> 
> Specifying these in devicetree is possible but undesirable as the nand
> may be different in different production runs of the same board, so it
> is better to get this info from the nand id where possible.
> 
> This commit adds code to read the ecc strength and size from the nand
> for Samsung extended-id nands. This code is based on the info for the 5th
> id byte in the datasheets for the following Samsung nands: K9GAG08U0E,
> K9GAG08U0F, K9GAG08X0D, K9GBG08U0A, K9GBG08U0B. These all use these bits
> in the exact same way.
> 
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/nand_samsung.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c
> index 0ab8062193..36ef48e5ec 100644
> --- a/drivers/mtd/nand/raw/nand_samsung.c
> +++ b/drivers/mtd/nand/raw/nand_samsung.c
> @@ -64,6 +64,26 @@ static void samsung_nand_decode_id(struct nand_chip *chip)
>  		extid >>= 2;
>  		mtd->erasesize = (128 * 1024) <<
>  				 (((extid >> 1) & 0x04) | (extid & 0x03));
> +
> +		/* Extract ECC requirements from 5th id byte*/
> +		extid = (chip->id.data[4] >> 4) & 0x07;
> +		if (extid < 5) {
> +			chip->ecc_step_ds = 512;
> +			chip->ecc_strength_ds = 1 << extid;
> +		} else {
> +			chip->ecc_step_ds = 1024;
> +			switch (extid) {
> +			case 5:
> +				chip->ecc_strength_ds = 24;
> +				break;
> +			case 6:
> +				chip->ecc_strength_ds = 40;
> +				break;
> +			case 7:
> +				chip->ecc_strength_ds = 60;
> +				break;
> +			}
> +		}
>  	} else {
>  		nand_decode_ext_id(chip);
>  	}
> -- 
> 2.34.1
> 

My bad delete my last msg. I see the comment is clear :)
look's good

Patch

diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c
index 0ab8062193..36ef48e5ec 100644
--- a/drivers/mtd/nand/raw/nand_samsung.c
+++ b/drivers/mtd/nand/raw/nand_samsung.c
@@ -64,6 +64,26 @@  static void samsung_nand_decode_id(struct nand_chip *chip)
 		extid >>= 2;
 		mtd->erasesize = (128 * 1024) <<
 				 (((extid >> 1) & 0x04) | (extid & 0x03));
+
+		/* Extract ECC requirements from 5th id byte*/
+		extid = (chip->id.data[4] >> 4) & 0x07;
+		if (extid < 5) {
+			chip->ecc_step_ds = 512;
+			chip->ecc_strength_ds = 1 << extid;
+		} else {
+			chip->ecc_step_ds = 1024;
+			switch (extid) {
+			case 5:
+				chip->ecc_strength_ds = 24;
+				break;
+			case 6:
+				chip->ecc_strength_ds = 40;
+				break;
+			case 7:
+				chip->ecc_strength_ds = 60;
+				break;
+			}
+		}
 	} else {
 		nand_decode_ext_id(chip);
 	}