[PATCH 1/4] mtd: nand: Fix NAND_* options to use unique values.

[PATCH 1/4] mtd: nand: Fix NAND_* options to use unique values.

[Hans de Goede]

From: Michal Suchanek <hramrach@gmail.com>

NAND_BUSWIDTH_AUTO (64b37b2a6) and NAND_USE_BOUNCE_BUFFER (66507c7bc)
are the same value. Change the later introduced NAND_USE_BOUNCE_BUFFER
to a different value.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 include/linux/mtd/nand.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index e2d3219..67caec2 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -178,17 +178,17 @@ typedef enum {
 /* Chip may not exist, so silence any errors in scan */
 #define NAND_SCAN_SILENT_NODEV	0x00040000
 /*
- * This option could be defined by controller drivers to protect against
- * kmap'ed, vmalloc'ed highmem buffers being passed from upper layers
- */
-#define NAND_USE_BOUNCE_BUFFER	0x00080000
-/*
  * Autodetect nand buswidth with readid/onfi.
  * This suppose the driver will configure the hardware in 8 bits mode
  * when calling nand_scan_ident, and update its configuration
  * before calling nand_scan_tail.
  */
-#define NAND_BUSWIDTH_AUTO      0x00080000
+#define NAND_BUSWIDTH_AUTO	0x00080000
+/*
+ * This option could be defined by controller drivers to protect against
+ * kmap'ed, vmalloc'ed highmem buffers being passed from upper layers
+ */
+#define NAND_USE_BOUNCE_BUFFER	0x00100000
 
 /* Options set by nand scan */
 /* Nand scan has allocated controller struct */
-- 
2.4.3

[PATCH 2/4] mtd: nand: nand_decode_ext_id(): Fill in ecc strength and size for Samsung

[Hans de Goede]

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: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/nand_base.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 462984a..f3fefbf 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4059,6 +4059,41 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 		mtd->erasesize = (128 * 1024) <<
 			(((extid >> 1) & 0x04) | (extid & 0x03));
 		*busw = 0;
+		/* Calc ecc strength and size from 5th id byte*/
+		switch ((id_data[4] >> 4) & 0x07) {
+		case 0:
+			chip->ecc_strength_ds = 1;
+			chip->ecc_step_ds = 512;
+			break;
+		case 1:
+			chip->ecc_strength_ds = 2;
+			chip->ecc_step_ds = 512;
+			break;
+		case 2:
+			chip->ecc_strength_ds = 4;
+			chip->ecc_step_ds = 512;
+			break;
+		case 3:
+			chip->ecc_strength_ds = 8;
+			chip->ecc_step_ds = 512;
+			break;
+		case 4:
+			chip->ecc_strength_ds = 16;
+			chip->ecc_step_ds = 512;
+			break;
+		case 5:
+			chip->ecc_strength_ds = 24;
+			chip->ecc_step_ds = 1024;
+			break;
+		case 6:
+			chip->ecc_strength_ds = 40;
+			chip->ecc_step_ds = 1024;
+			break;
+		case 7:
+			chip->ecc_strength_ds = 60;
+			chip->ecc_step_ds = 1024;
+			break;
+		}
 	} else if (id_len == 6 && id_data[0] == NAND_MFR_HYNIX &&
 			!nand_is_slc(chip)) {
 		unsigned int tmp;
-- 
2.4.3

Re: [PATCH 2/4] mtd: nand: nand_decode_ext_id(): Fill in ecc strength and size for Samsung

[Boris Brezillon]

Hi,

On Wed, 29 Jul 2015 19:53:52 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> 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.

Okay, that's the one I'm not sure about. If that's really the case,
and Samsung is actually using the same layout for all its chips, we
should be good, but as I already stated in the other thread, this was
not the case for Hynix chips.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [linux-sunxi] Re: [PATCH 2/4] mtd: nand: nand_decode_ext_id(): Fill in ecc strength and size for Samsung

[Hans de Goede]

Hi,

On 30-07-15 09:29, Boris Brezillon wrote:
> Hi,
>
> On Wed, 29 Jul 2015 19:53:52 +0200
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> 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.
>
> Okay, that's the one I'm not sure about. If that's really the case,
> and Samsung is actually using the same layout for all its chips, we
> should be good, but as I already stated in the other thread, this was
> not the case for Hynix chips.

The list of datasheets I checked is longer then the one used to write
the other samsung code which is setting equally important properties.

Regards,

Hans

[PATCH 3/4] mtd: nand: nand_get_flash_type: Print detected ECC strength and size

[Hans de Goede]

Print the detected ECC strength and size from nand_get_flash_type().

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/nand_base.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index f3fefbf..e2e2690 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4433,9 +4433,10 @@ ident_done:
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name,
 				type->name);
 
-	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
+	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d, ECC strength %d size %d\n",
 		(int)(chip->chipsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
-		mtd->erasesize >> 10, mtd->writesize, mtd->oobsize);
+		mtd->erasesize >> 10, mtd->writesize, mtd->oobsize,
+		chip->ecc_strength_ds, chip->ecc_step_ds);
 	return type;
 }
 
-- 
2.4.3

Re: [PATCH 3/4] mtd: nand: nand_get_flash_type: Print detected ECC strength and size

[Boris Brezillon]

Hi Hans,

On Wed, 29 Jul 2015 19:53:53 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> Print the detected ECC strength and size from nand_get_flash_type().

Indeed, that's an interesting information.

> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mtd/nand/nand_base.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index f3fefbf..e2e2690 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -4433,9 +4433,10 @@ ident_done:
>  		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name,
>  				type->name);
>  
> -	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
> +	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d, ECC strength %d size %d\n",

How about the follwing formalism ?

	pr_info("%d MiB, type: %s, erase size: %d KiB, page size: %d, OOB size: %d, ECC: %dbits/%dbytes\n",

or just

	pr_info("%d MiB, type: %s, erase size: %d KiB, page size: %d, OOB size: %d, ECC: %d/%d\n",

Best Regards,

Boris


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [linux-sunxi] Re: [PATCH 3/4] mtd: nand: nand_get_flash_type: Print detected ECC strength and size

[Hans de Goede]

Hi,

On 30-07-15 09:23, Boris Brezillon wrote:
> Hi Hans,
>
> On Wed, 29 Jul 2015 19:53:53 +0200
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> Print the detected ECC strength and size from nand_get_flash_type().
>
> Indeed, that's an interesting information.
>
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>   drivers/mtd/nand/nand_base.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index f3fefbf..e2e2690 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -4433,9 +4433,10 @@ ident_done:
>>   		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name,
>>   				type->name);
>>
>> -	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
>> +	pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d, ECC strength %d size %d\n",
>
> How about the follwing formalism ?
>
> 	pr_info("%d MiB, type: %s, erase size: %d KiB, page size: %d, OOB size: %d, ECC: %dbits/%dbytes\n",
>
> or just
>
> 	pr_info("%d MiB, type: %s, erase size: %d KiB, page size: %d, OOB size: %d, ECC: %d/%d\n",

Both works for me too, Brian can you let us know which version you
prefer and/or just adjust this when you merge it ?

Regards,

Hans

[PATCH 4/4] mtd: nand: print full chip ID

[Hans de Goede]

From: Michal Suchanek <hramrach@gmail.com>

Full chip ID is printed so user has data to paste from syslog in case
of chip misidentification.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mtd/nand/nand_base.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index e2e2690..13e9938 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -4243,7 +4243,7 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
 }
 
 static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
-		   struct nand_flash_dev *type, u8 *id_data, int *busw)
+		   struct nand_flash_dev *type, const u8 *id_data, int *busw)
 {
 	if (!strncmp(type->id, id_data, type->id_len)) {
 		mtd->writesize = type->pagesize;
@@ -4269,6 +4269,26 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /*
+ * Print full detail of chip ID read from chip.
+ */
+static void print_nand_chip_info(int maf_id, int dev_id, u8 id_data[8])
+{
+	u8 delim[8] = { [0 ... 7] = ',' };
+
+	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
+		maf_id, dev_id);
+
+	delim[7] = ' ';
+	delim[nand_id_len(id_data, 8) - 1] = ';';
+
+	pr_info("chip id data: 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c\n",
+		id_data[0], delim[0], id_data[1], delim[1],
+		id_data[2], delim[2], id_data[3], delim[3],
+		id_data[4], delim[4], id_data[5], delim[5],
+		id_data[6], delim[6], id_data[7], delim[7]);
+}
+
+/*
  * Get the flash and manufacturer id and lookup if the type is supported.
  */
 static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
@@ -4381,8 +4401,7 @@ ident_done:
 		 * Check, if buswidth is correct. Hardware drivers should set
 		 * chip correct!
 		 */
-		pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
-			*maf_id, *dev_id);
+		print_nand_chip_info(*maf_id, *dev_id, id_data);
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name, mtd->name);
 		pr_warn("bus width %d instead %d bit\n",
 			   (chip->options & NAND_BUSWIDTH_16) ? 16 : 8,
@@ -4420,8 +4439,7 @@ ident_done:
 			return ERR_PTR(err);
 	}
 
-	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
-		*maf_id, *dev_id);
+	print_nand_chip_info(*maf_id, *dev_id, id_data);
 
 	if (chip->onfi_version)
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name,
-- 
2.4.3

Re: [linux-sunxi] [PATCH 4/4] mtd: nand: print full chip ID

[Julian Calaby]

Hi Hans, Michal,

On Thu, Jul 30, 2015 at 3:53 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> From: Michal Suchanek <hramrach@gmail.com>
>
> Full chip ID is printed so user has data to paste from syslog in case
> of chip misidentification.
>
> Signed-off-by: Michal Suchanek <hramrach@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mtd/nand/nand_base.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index e2e2690..13e9938 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -4243,7 +4243,7 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
>  }
>
>  static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
> -                  struct nand_flash_dev *type, u8 *id_data, int *busw)
> +                  struct nand_flash_dev *type, const u8 *id_data, int *busw)

Should this be in another patch?

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [linux-sunxi] [PATCH 4/4] mtd: nand: print full chip ID

[Michal Suchanek]

On 30 July 2015 at 01:37, Julian Calaby <julian.calaby@gmail.com> wrote:
> Hi Hans, Michal,
>
> On Thu, Jul 30, 2015 at 3:53 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> From: Michal Suchanek <hramrach@gmail.com>
>>
>> Full chip ID is printed so user has data to paste from syslog in case
>> of chip misidentification.
>>
>> Signed-off-by: Michal Suchanek <hramrach@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/mtd/nand/nand_base.c | 28 +++++++++++++++++++++++-----
>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index e2e2690..13e9938 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -4243,7 +4243,7 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
>>  }
>>
>>  static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
>> -                  struct nand_flash_dev *type, u8 *id_data, int *busw)
>> +                  struct nand_flash_dev *type, const u8 *id_data, int *busw)
>
> Should this be in another patch?
>

Yes, that's not needed here.

Thanks

Michal

Re: [PATCH 4/4] mtd: nand: print full chip ID

[Boris Brezillon]

Hans, Michal,

On Wed, 29 Jul 2015 19:53:54 +0200
Hans de Goede <hdegoede@redhat.com> wrote:

> From: Michal Suchanek <hramrach@gmail.com>
> 
> Full chip ID is printed so user has data to paste from syslog in case
> of chip misidentification.
> 
> Signed-off-by: Michal Suchanek <hramrach@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mtd/nand/nand_base.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index e2e2690..13e9938 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -4243,7 +4243,7 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
>  }
>  
>  static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
> -		   struct nand_flash_dev *type, u8 *id_data, int *busw)
> +		   struct nand_flash_dev *type, const u8 *id_data, int *busw)
>  {
>  	if (!strncmp(type->id, id_data, type->id_len)) {
>  		mtd->writesize = type->pagesize;
> @@ -4269,6 +4269,26 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
>  }
>  
>  /*
> + * Print full detail of chip ID read from chip.
> + */
> +static void print_nand_chip_info(int maf_id, int dev_id, u8 id_data[8])
> +{
> +	u8 delim[8] = { [0 ... 7] = ',' };
> +
> +	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
> +		maf_id, dev_id);
> +
> +	delim[7] = ' ';
> +	delim[nand_id_len(id_data, 8) - 1] = ';';
> +
> +	pr_info("chip id data: 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c\n",
> +		id_data[0], delim[0], id_data[1], delim[1],
> +		id_data[2], delim[2], id_data[3], delim[3],
> +		id_data[4], delim[4], id_data[5], delim[5],
> +		id_data[6], delim[6], id_data[7], delim[7]);

This looks like debug information to me, how about using pr_debug ?

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 4/4] mtd: nand: print full chip ID

[Michal Suchanek]

Hello,

On 30 July 2015 at 09:17, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> Hans, Michal,
>
> On Wed, 29 Jul 2015 19:53:54 +0200
> Hans de Goede <hdegoede@redhat.com> wrote:
>
>> From: Michal Suchanek <hramrach@gmail.com>
>>
>> Full chip ID is printed so user has data to paste from syslog in case
>> of chip misidentification.
>>
>> Signed-off-by: Michal Suchanek <hramrach@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/mtd/nand/nand_base.c | 28 +++++++++++++++++++++++-----
>>  1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index e2e2690..13e9938 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -4243,7 +4243,7 @@ static inline bool is_full_id_nand(struct nand_flash_dev *type)
>>  }
>>
>>  static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
>> -                struct nand_flash_dev *type, u8 *id_data, int *busw)
>> +                struct nand_flash_dev *type, const u8 *id_data, int *busw)
>>  {
>>       if (!strncmp(type->id, id_data, type->id_len)) {
>>               mtd->writesize = type->pagesize;
>> @@ -4269,6 +4269,26 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
>>  }
>>
>>  /*
>> + * Print full detail of chip ID read from chip.
>> + */
>> +static void print_nand_chip_info(int maf_id, int dev_id, u8 id_data[8])
>> +{
>> +     u8 delim[8] = { [0 ... 7] = ',' };
>> +
>> +     pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
>> +             maf_id, dev_id);
>> +
>> +     delim[7] = ' ';
>> +     delim[nand_id_len(id_data, 8) - 1] = ';';
>> +
>> +     pr_info("chip id data: 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c\n",
>> +             id_data[0], delim[0], id_data[1], delim[1],
>> +             id_data[2], delim[2], id_data[3], delim[3],
>> +             id_data[4], delim[4], id_data[5], delim[5],
>> +             id_data[6], delim[6], id_data[7], delim[7]);
>
> This looks like debug information to me, how about using pr_debug ?
>

This is informative message saying that a device was detected. The
only change is that full id is printed as part of the message. The
same message is printed when the device fails to identify so it should
probably be shown by default. Maybe joining it into one line to avoid
excessive spam would be better.

Thanks

Michal

[PATCH v2] mtd: nand: print full chip ID

[Michal Suchanek]

Full chip ID is printed so user has data to paste from syslog in case
of chip misidentification.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>

---
v2:

 - remove superfluous hunk
 - join printed lines
---
 drivers/mtd/nand/nand_base.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ceb68ca..7c10e06 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3631,6 +3631,22 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
 }
 
 /*
+ * Print full detail of chip ID read from chip.
+ */
+static void nand_print_chip_info(int maf_id, int dev_id, u8 id_data[8])
+{
+	u8 delim[8] = { [0 ... 7] = ',' };
+
+	/* Kernel inserts newline after every other printk so format in one go. */
+	delim[7] = ' ';
+	delim[nand_id_len(id_data, 8) - 1] = ';';
+	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x, ID data: 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c\n",
+		maf_id, dev_id,
+		id_data[0], delim[0], id_data[1], delim[1], id_data[2], delim[2], id_data[3], delim[3],
+		id_data[4], delim[4], id_data[5], delim[5], id_data[6], delim[6], id_data[7], delim[7]);
+}
+
+/*
  * Get the flash and manufacturer id and lookup if the type is supported.
  */
 static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
@@ -3743,8 +3759,7 @@ ident_done:
 		 * Check, if buswidth is correct. Hardware drivers should set
 		 * chip correct!
 		 */
-		pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
-			*maf_id, *dev_id);
+		nand_print_chip_info(*maf_id, *dev_id, id_data);
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name, mtd->name);
 		pr_warn("bus width %d instead %d bit\n",
 			   (chip->options & NAND_BUSWIDTH_16) ? 16 : 8,
@@ -3775,8 +3790,7 @@ ident_done:
 	if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
 		chip->cmdfunc = nand_command_lp;
 
-	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
-		*maf_id, *dev_id);
+	nand_print_chip_info(*maf_id, *dev_id, id_data);
 
 	if (chip->onfi_version)
 		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name,
-- 
2.1.4

Re: [PATCH v2] mtd: nand: print full chip ID

[Hans de Goede]

Hi,

On 30-07-15 13:52, Michal Suchanek wrote:
> Full chip ID is printed so user has data to paste from syslog in case
> of chip misidentification.
>
> Signed-off-by: Michal Suchanek <hramrach@gmail.com>
>
> ---
> v2:
>
>   - remove superfluous hunk
>   - join printed lines
> ---
>   drivers/mtd/nand/nand_base.c | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index ceb68ca..7c10e06 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3631,6 +3631,22 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
>   }
>
>   /*
> + * Print full detail of chip ID read from chip.
> + */
> +static void nand_print_chip_info(int maf_id, int dev_id, u8 id_data[8])
> +{
> +	u8 delim[8] = { [0 ... 7] = ',' };
> +
> +	/* Kernel inserts newline after every other printk so format in one go. */
> +	delim[7] = ' ';
> +	delim[nand_id_len(id_data, 8) - 1] = ';';
> +	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x, ID data: 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c 0x%02x%c\n",
> +		maf_id, dev_id,
> +		id_data[0], delim[0], id_data[1], delim[1], id_data[2], delim[2], id_data[3], delim[3],
> +		id_data[4], delim[4], id_data[5], delim[5], id_data[6], delim[6], id_data[7], delim[7]);

These lines are longer then 80 chars, that is allowed only for lines ending with "...",

Which is why the version of this patch I submitted upstream had:

+		id_data[0], delim[0], id_data[1], delim[1],
+		id_data[2], delim[2], id_data[3], delim[3],
+		id_data[4], delim[4], id_data[5], delim[5],
+		id_data[6], delim[6], id_data[7], delim[7]);

Other then that this version looks good to me, and I agree that having this
info as non pr_debug is quite useful so that if (variants of) boards show up
with a nand with an unknown nand id, but one which the auto-detect code
does handle, we will still know and we can add an entry to the id table
for the nand to enable extra features (higher speed, special read retry
logic). This is quite likely to happen with e.g. the popular mk802 hdmi
tv sticks which have a single dts file for all of them, but come with
different nand chips based on the exact production run.

Regards,

Hans


> +}
> +
> +/*
>    * Get the flash and manufacturer id and lookup if the type is supported.
>    */
>   static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
> @@ -3743,8 +3759,7 @@ ident_done:
>   		 * Check, if buswidth is correct. Hardware drivers should set
>   		 * chip correct!
>   		 */
> -		pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
> -			*maf_id, *dev_id);
> +		nand_print_chip_info(*maf_id, *dev_id, id_data);
>   		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name, mtd->name);
>   		pr_warn("bus width %d instead %d bit\n",
>   			   (chip->options & NAND_BUSWIDTH_16) ? 16 : 8,
> @@ -3775,8 +3790,7 @@ ident_done:
>   	if (mtd->writesize > 512 && chip->cmdfunc == nand_command)
>   		chip->cmdfunc = nand_command_lp;
>
> -	pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
> -		*maf_id, *dev_id);
> +	nand_print_chip_info(*maf_id, *dev_id, id_data);
>
>   	if (chip->onfi_version)
>   		pr_info("%s %s\n", nand_manuf_ids[maf_idx].name,
>

Re: [PATCH 1/4] mtd: nand: Fix NAND_* options to use unique values.

[Brian Norris]

On Wed, Jul 29, 2015 at 07:53:51PM +0200, Hans de Goede wrote:
> From: Michal Suchanek <hramrach@gmail.com>
> 
> NAND_BUSWIDTH_AUTO (64b37b2a6) and NAND_USE_BOUNCE_BUFFER (66507c7bc)
> are the same value. Change the later introduced NAND_USE_BOUNCE_BUFFER
> to a different value.
> 
> Signed-off-by: Michal Suchanek <hramrach@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Thanks for the fix! But Scott already caught this one. The fix is in
4.2-rc already as commit 5f867db63473 ("mtd: nand: Fix
NAND_USE_BOUNCE_BUFFER flag conflict").

(Obligatory: please base your MTD patches on l2-mtd.git, or
linux-next.git if necessary [1].)

Regards,
Brian

[1] http://linux-mtd.infradead.org/source.html