Re: [PATCH v3 33/33] mtd: rawnand: allocate dynamically ONFI parameters during detection

2018-07-19 Thread Boris Brezillon
On Fri, 20 Jul 2018 01:00:26 +0200
Miquel Raynal  wrote:

> Now that it is possible to do dynamic allocations during the
> identification phase, convert the onfi_params structure (which is only
> needed with ONFI compliant chips) into a pointer that will be allocated
> only if needed.
> 
> Signed-off-by: Miquel Raynal 
> ---
>  drivers/mtd/nand/raw/nand_base.c| 61 
> ++---
>  drivers/mtd/nand/raw/nand_micron.c  |  6 ++--
>  drivers/mtd/nand/raw/nand_timings.c | 12 
>  include/linux/mtd/rawnand.h |  6 ++--
>  4 files changed, 48 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c 
> b/drivers/mtd/nand/raw/nand_base.c
> index dfee2556a8a8..4dc248769abb 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -5151,6 +5151,8 @@ static int nand_flash_detect_onfi(struct nand_chip 
> *chip)
>  {
>   struct mtd_info *mtd = nand_to_mtd(chip);
>   struct nand_onfi_params *p;
> + struct onfi_params *onfi;
> + int onfi_version = 0;
>   char *model;
>   char id[4];
>   int i, ret, val;
> @@ -5207,21 +5209,19 @@ static int nand_flash_detect_onfi(struct nand_chip 
> *chip)
>   /* Check version */
>   val = le16_to_cpu(p->revision);
>   if (val & ONFI_VERSION_2_3)
> - chip->parameters.onfi.version = 23;
> + onfi_version = 23;
>   else if (val & ONFI_VERSION_2_2)
> - chip->parameters.onfi.version = 22;
> + onfi_version = 22;
>   else if (val & ONFI_VERSION_2_1)
> - chip->parameters.onfi.version = 21;
> + onfi_version = 21;
>   else if (val & ONFI_VERSION_2_0)
> - chip->parameters.onfi.version = 20;
> + onfi_version = 20;
>   else if (val & ONFI_VERSION_1_0)
> - chip->parameters.onfi.version = 10;
> + onfi_version = 10;
>  
> - if (!chip->parameters.onfi.version) {
> + if (!onfi_version) {
>   pr_info("unsupported ONFI version: %d\n", val);
>   goto free_onfi_param_page;
> - } else {
> - ret = 1;
>   }
>  
>   sanitize_string(p->manufacturer, sizeof(p->manufacturer));
> @@ -5262,7 +5262,7 @@ static int nand_flash_detect_onfi(struct nand_chip 
> *chip)
>   if (p->ecc_bits != 0xff) {
>   chip->ecc_strength_ds = p->ecc_bits;
>   chip->ecc_step_ds = 512;
> - } else if (chip->parameters.onfi.version >= 21 &&
> + } else if (onfi_version >= 21 &&
>   (le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
>  
>   /*
> @@ -5289,19 +5289,32 @@ static int nand_flash_detect_onfi(struct nand_chip 
> *chip)
>   bitmap_set(chip->parameters.set_feature_list,
>  ONFI_FEATURE_ADDR_TIMING_MODE, 1);
>   }
> - chip->parameters.onfi.tPROG = le16_to_cpu(p->t_prog);
> - chip->parameters.onfi.tBERS = le16_to_cpu(p->t_bers);
> - chip->parameters.onfi.tR = le16_to_cpu(p->t_r);
> - chip->parameters.onfi.tCCS = le16_to_cpu(p->t_ccs);
> - chip->parameters.onfi.async_timing_mode =
> - le16_to_cpu(p->async_timing_mode);
> - chip->parameters.onfi.vendor_revision =
> - le16_to_cpu(p->vendor_revision);
> - memcpy(chip->parameters.onfi.vendor, p->vendor,
> -sizeof(p->vendor));
>  
> + onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
> + if (!onfi) {
> + ret = -ENOMEM;
> + goto free_model;
> + }
> + onfi->version = onfi_version;
> + onfi->tPROG = le16_to_cpu(p->t_prog);
> + onfi->tBERS = le16_to_cpu(p->t_bers);
> + onfi->tR = le16_to_cpu(p->t_r);
> + onfi->tCCS = le16_to_cpu(p->t_ccs);
> + onfi->async_timing_mode = le16_to_cpu(p->async_timing_mode);
> + onfi->vendor_revision = le16_to_cpu(p->vendor_revision);
> + memcpy(onfi->vendor, p->vendor, sizeof(p->vendor));
> + chip->parameters.onfi = onfi;
> +
> + /* Identification done, free the full ONFI parameter page and exit */
> + kfree(p);
> +
> + return 1;
> +
> +free_model:
> + kfree(model);
>  free_onfi_param_page:
>   kfree(p);
> +
>   return ret;
>  }
>  
> @@ -5562,9 +5575,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
>   chip->ecc_step_ds = NAND_ECC_STEP(type);
>   chip->onfi_timing_mode_default =
>   type->onfi_timing_mode_default;
> -
> - strncpy(chip->parameters.model, type->name,
> - sizeof(chip->parameters.model) - 1);
> + chip->parameters.model = type->name;
>  
>   return true;
>   }
> @@ -5703,7 +5714,6 @@ static int nand_detect(struct nand_chip *chip, struct 
> nand_flash_dev *type)
>   }
>   }
>  
> - chip->parameters.onfi.version = 0;
>   if (!type->name || !type->pagesize) {
>   /* Check if the chip is ONFI compliant */
>   

[PATCH v3 33/33] mtd: rawnand: allocate dynamically ONFI parameters during detection

2018-07-19 Thread Miquel Raynal
Now that it is possible to do dynamic allocations during the
identification phase, convert the onfi_params structure (which is only
needed with ONFI compliant chips) into a pointer that will be allocated
only if needed.

Signed-off-by: Miquel Raynal 
---
 drivers/mtd/nand/raw/nand_base.c| 61 ++---
 drivers/mtd/nand/raw/nand_micron.c  |  6 ++--
 drivers/mtd/nand/raw/nand_timings.c | 12 
 include/linux/mtd/rawnand.h |  6 ++--
 4 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index dfee2556a8a8..4dc248769abb 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5151,6 +5151,8 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
 {
struct mtd_info *mtd = nand_to_mtd(chip);
struct nand_onfi_params *p;
+   struct onfi_params *onfi;
+   int onfi_version = 0;
char *model;
char id[4];
int i, ret, val;
@@ -5207,21 +5209,19 @@ static int nand_flash_detect_onfi(struct nand_chip 
*chip)
/* Check version */
val = le16_to_cpu(p->revision);
if (val & ONFI_VERSION_2_3)
-   chip->parameters.onfi.version = 23;
+   onfi_version = 23;
else if (val & ONFI_VERSION_2_2)
-   chip->parameters.onfi.version = 22;
+   onfi_version = 22;
else if (val & ONFI_VERSION_2_1)
-   chip->parameters.onfi.version = 21;
+   onfi_version = 21;
else if (val & ONFI_VERSION_2_0)
-   chip->parameters.onfi.version = 20;
+   onfi_version = 20;
else if (val & ONFI_VERSION_1_0)
-   chip->parameters.onfi.version = 10;
+   onfi_version = 10;
 
-   if (!chip->parameters.onfi.version) {
+   if (!onfi_version) {
pr_info("unsupported ONFI version: %d\n", val);
goto free_onfi_param_page;
-   } else {
-   ret = 1;
}
 
sanitize_string(p->manufacturer, sizeof(p->manufacturer));
@@ -5262,7 +5262,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
if (p->ecc_bits != 0xff) {
chip->ecc_strength_ds = p->ecc_bits;
chip->ecc_step_ds = 512;
-   } else if (chip->parameters.onfi.version >= 21 &&
+   } else if (onfi_version >= 21 &&
(le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
 
/*
@@ -5289,19 +5289,32 @@ static int nand_flash_detect_onfi(struct nand_chip 
*chip)
bitmap_set(chip->parameters.set_feature_list,
   ONFI_FEATURE_ADDR_TIMING_MODE, 1);
}
-   chip->parameters.onfi.tPROG = le16_to_cpu(p->t_prog);
-   chip->parameters.onfi.tBERS = le16_to_cpu(p->t_bers);
-   chip->parameters.onfi.tR = le16_to_cpu(p->t_r);
-   chip->parameters.onfi.tCCS = le16_to_cpu(p->t_ccs);
-   chip->parameters.onfi.async_timing_mode =
-   le16_to_cpu(p->async_timing_mode);
-   chip->parameters.onfi.vendor_revision =
-   le16_to_cpu(p->vendor_revision);
-   memcpy(chip->parameters.onfi.vendor, p->vendor,
-  sizeof(p->vendor));
 
+   onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
+   if (!onfi) {
+   ret = -ENOMEM;
+   goto free_model;
+   }
+   onfi->version = onfi_version;
+   onfi->tPROG = le16_to_cpu(p->t_prog);
+   onfi->tBERS = le16_to_cpu(p->t_bers);
+   onfi->tR = le16_to_cpu(p->t_r);
+   onfi->tCCS = le16_to_cpu(p->t_ccs);
+   onfi->async_timing_mode = le16_to_cpu(p->async_timing_mode);
+   onfi->vendor_revision = le16_to_cpu(p->vendor_revision);
+   memcpy(onfi->vendor, p->vendor, sizeof(p->vendor));
+   chip->parameters.onfi = onfi;
+
+   /* Identification done, free the full ONFI parameter page and exit */
+   kfree(p);
+
+   return 1;
+
+free_model:
+   kfree(model);
 free_onfi_param_page:
kfree(p);
+
return ret;
 }
 
@@ -5562,9 +5575,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
chip->ecc_step_ds = NAND_ECC_STEP(type);
chip->onfi_timing_mode_default =
type->onfi_timing_mode_default;
-
-   strncpy(chip->parameters.model, type->name,
-   sizeof(chip->parameters.model) - 1);
+   chip->parameters.model = type->name;
 
return true;
}
@@ -5703,7 +5714,6 @@ static int nand_detect(struct nand_chip *chip, struct 
nand_flash_dev *type)
}
}
 
-   chip->parameters.onfi.version = 0;
if (!type->name || !type->pagesize) {
/* Check if the chip is ONFI compliant */
ret = nand_flash_detect_onfi(chip);
@@ -5723,8 +5733,7 @@ static int nand_detect(struct nand_chip *chip, struct 
nand_flash_dev *type)
if