[PATCH 2/2] mtd: nand: add Hynix's H27UCG8T2ATR-BC to nand_ids table

[Boris BREZILLON]

Add the full description of the Hynix H27UCG8T2ATR-BC NAND chip in the
nand_ids table so that we can later use the NAND ECC infos and ONFI timings
mode in controller drivers.

Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>
---
drivers/mtd/nand/nand_ids.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 3d7c89f..19d4168 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -46,6 +46,10 @@ struct nand_flash_dev nand_flash_ids[] = {
{"SDTNRGAMA 64G 3.3V 8-bit",
{ .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x50} },
SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },
+ {"H27UCG8T2ATR-BC 64G 3.3V 8-bit",
+ { .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
+ SZ_8K, SZ_8K, SZ_4M, 0, 6, 640, NAND_ECC_INFO(40, SZ_1K),
+ 4 },

LEGACY_ID_NAND("NAND 4MiB 5V 8-bit", 0x6B, 4, SZ_8K, SP_OPTIONS),
LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS),
--
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Boris BREZILLON]

Add an onfi_timing_mode_ds field to nand_chip and nand_flash_dev in order
to support NAND timings definition for non-ONFI NAND.

NAND that support better timings mode than the default one (timings mode 0)
have to define a new entry in the nand_ids table.

The timings mode should be deduced from timings description from the
datasheet and the ONFI specification
(www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf, chapter 4.15
"Timing Parameters").
You should choose the closest mode that fit the timings requirements of
your NAND chip.

Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>
---

drivers/mtd/nand/nand_base.c | 1 +
include/linux/mtd/nand.h | 7 +++++++
2 files changed, 8 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d8cdf06..c952c21 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3576,6 +3576,7 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
chip->options |= type->options;
chip->ecc_strength_ds = NAND_ECC_STRENGTH(type);
chip->ecc_step_ds = NAND_ECC_STEP(type);
+ chip->onfi_timing_mode_ds = type->onfi_timing_mode_ds;

*busw = type->options & NAND_BUSWIDTH_16;

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 3083c53..435c005 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -587,6 +587,7 @@ struct nand_buffers {
* @ecc_step_ds: [INTERN] ECC step required by the @ecc_strength_ds,
* also from the datasheet. It is the recommended ECC step
* size, if known; if unknown, set to zero.
+ * @onfi_timing_mode_ds:[INTERN] ONFI timing mode deduced from datasheet.
* @numchips: [INTERN] number of physical chips
* @chipsize: [INTERN] the size of one chip for multichip arrays
* @pagemask: [INTERN] page number mask = number of (pages / chip) - 1
@@ -671,6 +672,7 @@ struct nand_chip {
uint8_t bits_per_cell;
uint16_t ecc_strength_ds;
uint16_t ecc_step_ds;
+ int onfi_timing_mode_ds;
int badblockpos;
int badblockbits;

@@ -772,6 +774,10 @@ struct nand_chip {
* @ecc_step_ds in nand_chip{}, also from the datasheet.
* For example, the "4bit ECC for each 512Byte" can be set with
* NAND_ECC_INFO(4, 512).
+ * @onfi_timing_mode_ds: the ONFI timing mode supported by this NAND chip. This
+ * should be deduced from timings described in the
+ * datasheet.
+ *
*/
struct nand_flash_dev {
char *name;
@@ -792,6 +798,7 @@ struct nand_flash_dev {
uint16_t strength_ds;
uint16_t step_ds;
} ecc;
+ int onfi_timing_mode_ds;
};

/**

[Boris BREZILLON]

Hi,

Following the series adding ONFI timings mode support, here is a series
adding support for timings mode retrieval on non-ONFI NANDs.

It just adds a new field to the nand_chip and nand_flash_dev struct so
that anyone can define its chip requirements in the nand_ids table.

The 2nd patch serves as an example and adds an entry for the Hynix
H27UCG8T2ATR-BC NAND chip used on the Cubietruck board.

Best Regards,

Boris

Boris BREZILLON (2):
mtd: nand: support ONFI timing mode retrieval for non-ONFI NANDs
mtd: nand: add Hynix's H27UCG8T2ATR-BC to nand_ids table

drivers/mtd/nand/nand_base.c | 1 +
drivers/mtd/nand/nand_ids.c | 4 ++++
include/linux/mtd/nand.h | 7 +++++++
3 files changed, 12 insertions(+)

[Boris BREZILLON]

On Mon, 28 Jul 2014 11:16:52 +0200
Boris BREZILLON <boris.b...@free-electrons.com> wrote:

> Add the full description of the Hynix H27UCG8T2ATR-BC NAND chip in the
> nand_ids table so that we can later use the NAND ECC infos and ONFI timings
> mode in controller drivers.
>
> Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>
> ---
> drivers/mtd/nand/nand_ids.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
> index 3d7c89f..19d4168 100644
> --- a/drivers/mtd/nand/nand_ids.c
> +++ b/drivers/mtd/nand/nand_ids.c
> @@ -46,6 +46,10 @@ struct nand_flash_dev nand_flash_ids[] = {
> {"SDTNRGAMA 64G 3.3V 8-bit",
> { .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x50} },
> SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },
> + {"H27UCG8T2ATR-BC 64G 3.3V 8-bit",
> + { .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
> + SZ_8K, SZ_8K, SZ_4M, 0, 6, 640, NAND_ECC_INFO(40, SZ_1K),

There is a big mistake here ------^

It should be SZ_2M not SZ_4M, I'll fix that.

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

[Brian Norris]

Since you were asking about this series, I have a comment:

Might want at least one space between '[INTERN]' and 'ONFI'?

> * @numchips: [INTERN] number of physical chips
> * @chipsize: [INTERN] the size of one chip for multichip arrays
> * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1
> @@ -671,6 +672,7 @@ struct nand_chip {
> uint8_t bits_per_cell;
> uint16_t ecc_strength_ds;
> uint16_t ecc_step_ds;
> + int onfi_timing_mode_ds;

I'm not sure if we'll just want a field specific to non-ONFI chips,
faking an ONFI timing mode; can we make this into a more generally
useful field, that represents something consistent across all NAND
types? I was thinking a "highest mode supported", but that actually
isn't great, since for true ONFI modes (except mode 0) require a SET
FEATURES command to change to a higher mode.

Maybe just something like onfi_timing_mode_default, which we could then
use as mode 0 for ONFI chips.

Ideally, we could centralize as much of this as possible, so that a NAND
driver only has to know the mechanics of "how do I translate an ONFI
mode to a timing configuration for my NAND controller," instead of any
details of how to choose from one of many supported ONFI modes. i.e., it
could implement something like the following hook:

int (*set_timing_mode)(struct nand_sdr_timings *timing);

But anyway, that's out of the scope of this series, and it may be harder
than I make it sound. I just wanted to throw that out there.

> int badblockpos;
> int badblockbits;
>
> @@ -772,6 +774,10 @@ struct nand_chip {
> * @ecc_step_ds in nand_chip{}, also from the datasheet.
> * For example, the "4bit ECC for each 512Byte" can be set with
> * NAND_ECC_INFO(4, 512).
> + * @onfi_timing_mode_ds: the ONFI timing mode supported by this NAND chip. This
> + * should be deduced from timings described in the
> + * datasheet.
> + *
> */
> struct nand_flash_dev {
> char *name;
> @@ -792,6 +798,7 @@ struct nand_flash_dev {
> uint16_t strength_ds;
> uint16_t step_ds;
> } ecc;
> + int onfi_timing_mode_ds;
> };
>
> /**

Brian

[Boris BREZILLON]

You mean between the colon and '[INTERN]', right ?

>
> > * @numchips: [INTERN] number of physical chips
> > * @chipsize: [INTERN] the size of one chip for multichip arrays
> > * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1
> > @@ -671,6 +672,7 @@ struct nand_chip {
> > uint8_t bits_per_cell;
> > uint16_t ecc_strength_ds;
> > uint16_t ecc_step_ds;
> > + int onfi_timing_mode_ds;
>
> I'm not sure if we'll just want a field specific to non-ONFI chips,
> faking an ONFI timing mode; can we make this into a more generally
> useful field, that represents something consistent across all NAND
> types? I was thinking a "highest mode supported", but that actually
> isn't great, since for true ONFI modes (except mode 0) require a SET
> FEATURES command to change to a higher mode.
>
> Maybe just something like onfi_timing_mode_default, which we could then
> use as mode 0 for ONFI chips.
>
> Ideally, we could centralize as much of this as possible, so that a NAND
> driver only has to know the mechanics of "how do I translate an ONFI
> mode to a timing configuration for my NAND controller,"

I guess you meant "how do I translate a NAND timing config given by
nand_sdr_timings to my specific NAND controller config", right ?

> instead of any
> details of how to choose from one of many supported ONFI modes. i.e., it
> could implement something like the following hook:
>
> int (*set_timing_mode)(struct nand_sdr_timings *timing);

I'm not opposed to this solution (I actually think this is how it
should be done :-), and, IIRC, Jason proposed to do the same kind of
things a while ago), but this means the conversion would be done by the
NAND FW and passed to the NAND driver, and this implies reworking the
drivers already directly using ONFI modes to configure their NAND
timings to make use nand_sdr_timings instead.

Moreover, I think we should define a union to handle several kind of
timings (ddr NAND are coming :-P):

enum nand_timings_type /* or nand_bus_type or something else ;-) */ {
SDR_NAND,
DDR_NAND,
}

struct nand_timings {
enum nand_timings_type type;
union {
struct nand_sdr_timings sdr;
struct nand_ddr_timings ddr;
} timings;
};

But this is another story ;-).

>
> But anyway, that's out of the scope of this series, and it may be harder
> than I make it sound. I just wanted to throw that out there.

If you don't mind I'd like to get something simple first: just renaming
onfi_timing_mode_ds is fine, but adding the whole conversion stuff to
the core implies much more changes. And I can work on a more
integrated solution as a second step.

Let me know what you think.

Best Regards,

Boris

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

[Brian Norris]

Yep! My mistake.

> >
> > > * @numchips: [INTERN] number of physical chips
> > > * @chipsize: [INTERN] the size of one chip for multichip arrays
> > > * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1
> > > @@ -671,6 +672,7 @@ struct nand_chip {
> > > uint8_t bits_per_cell;
> > > uint16_t ecc_strength_ds;
> > > uint16_t ecc_step_ds;
> > > + int onfi_timing_mode_ds;
> >
> > I'm not sure if we'll just want a field specific to non-ONFI chips,
> > faking an ONFI timing mode; can we make this into a more generally
> > useful field, that represents something consistent across all NAND
> > types? I was thinking a "highest mode supported", but that actually
> > isn't great, since for true ONFI modes (except mode 0) require a SET
> > FEATURES command to change to a higher mode.
> >
> > Maybe just something like onfi_timing_mode_default, which we could then
> > use as mode 0 for ONFI chips.
> >
> > Ideally, we could centralize as much of this as possible, so that a NAND
> > driver only has to know the mechanics of "how do I translate an ONFI
> > mode to a timing configuration for my NAND controller,"
>
> I guess you meant "how do I translate a NAND timing config given by
> nand_sdr_timings to my specific NAND controller config", right ?

Yes.

> > instead of any
> > details of how to choose from one of many supported ONFI modes. i.e., it
> > could implement something like the following hook:
> >
> > int (*set_timing_mode)(struct nand_sdr_timings *timing);
>
> I'm not opposed to this solution (I actually think this is how it
> should be done :-), and, IIRC, Jason proposed to do the same kind of
> things a while ago), but this means the conversion would be done by the
> NAND FW and passed to the NAND driver, and this implies reworking the
> drivers already directly using ONFI modes to configure their NAND
> timings to make use nand_sdr_timings instead.

I think we can safely ignore most of how drivers already configured
timings (I see at least denali.c with its own custom boot parameter),
and try to work out a solution that can be used by more than one driver
eventually. Conversion may or may not happen for some drivers, so we
should plan to have some sort of opt-in or opt-out ability.

> Moreover, I think we should define a union to handle several kind of
> timings (ddr NAND are coming :-P):
>
> enum nand_timings_type /* or nand_bus_type or something else ;-) */ {
> SDR_NAND,
> DDR_NAND,
> }
>
> struct nand_timings {
> enum nand_timings_type type;
> union {
> struct nand_sdr_timings sdr;
> struct nand_ddr_timings ddr;
> } timings;
> };

Perhaps. Although it's not immediately clear to me why we would need to
represent them in the same struct; I'm pretty sure we will have some
controllers that can handle one but not the other, so we might just have
two independent interfaces, and no need for the enum.

But anyway, I'm not 100% clear on how this would look, until one of us
provides a patch set, and we can work from there.

> But this is another story ;-).

Yes, I think so.

> >
> > But anyway, that's out of the scope of this series, and it may be harder
> > than I make it sound. I just wanted to throw that out there.
>
> If you don't mind I'd like to get something simple first: just renaming
> onfi_timing_mode_ds is fine, but adding the whole conversion stuff to
> the core implies much more changes. And I can work on a more
> integrated solution as a second step.

Sounds good. I really didn't expect much modification to this patch set,
but since you needed to fix the second patch, I though I'd give you my
nitpicks too.

One last random thought: if we ever integrate the SET_FEATURES commands
to set timing modes (async or synchronous ONFI modes) in the core code,
I think we'll need to ensure that we resend the SET_FEATURES command
after any NAND_CMD_RESET, as well as after any resume from suspend
(nand_resume()?).

Brian

[Boris BREZILLON]

Hi,

Following the series adding ONFI timing mode support, here is a series
adding support for timing mode retrieval on non-ONFI NANDs.

It just adds a new field to the nand_chip and nand_flash_dev struct so
that anyone can define its chip requirements in the nand_ids table.

The 2nd patch serves as an example and adds an entry for the Hynix
H27UCG8T2ATR-BC NAND chip used on the Cubietruck board.

Best Regards,

Boris

Changes since v1:
- fix H27UCG8T2ATR-BC definition
- rename onfi_timing_mode_ds field into onfi_timing_mode_default

Boris BREZILLON (2):
mtd: nand: support ONFI timing mode retrieval for non-ONFI NANDs
mtd: nand: add Hynix's H27UCG8T2ATR-BC to nand_ids table

drivers/mtd/nand/nand_base.c | 2 ++
drivers/mtd/nand/nand_ids.c | 4 ++++
include/linux/mtd/nand.h | 11 +++++++++++
3 files changed, 17 insertions(+)

--
1.9.1

[Boris BREZILLON]

Add the full description of the Hynix H27UCG8T2ATR-BC NAND chip in the
nand_ids table so that we can later use the NAND ECC infos and ONFI timings
mode in controller drivers.

Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>
---

drivers/mtd/nand/nand_ids.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c

index 3d7c89f..fbde8910 100644

--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -46,6 +46,10 @@ struct nand_flash_dev nand_flash_ids[] = {
{"SDTNRGAMA 64G 3.3V 8-bit",
{ .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x50} },
SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },
+ {"H27UCG8T2ATR-BC 64G 3.3V 8-bit",
+ { .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },

+ SZ_8K, SZ_8K, SZ_2M, 0, 6, 640, NAND_ECC_INFO(40, SZ_1K),

+ 4 },

LEGACY_ID_NAND("NAND 4MiB 5V 8-bit", 0x6B, 4, SZ_8K, SP_OPTIONS),
LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS),

[Boris BREZILLON]

Add an onfi_timing_mode_default field to nand_chip and nand_flash_dev in

order to support NAND timings definition for non-ONFI NAND.

NAND that support better timings mode than the default one have to define
a new entry in the nand_ids table.

The default timing mode should be deduced from timings description from

the datasheet and the ONFI specification
(www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf, chapter 4.15
"Timing Parameters").
You should choose the closest mode that fit the timings requirements of
your NAND chip.

Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>
---

drivers/mtd/nand/nand_base.c | 2 ++
include/linux/mtd/nand.h | 11 +++++++++++
2 files changed, 13 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ae6e7c4..c37fa2a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3594,6 +3594,8 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,

chip->options |= type->options;
chip->ecc_strength_ds = NAND_ECC_STRENGTH(type);
chip->ecc_step_ds = NAND_ECC_STEP(type);

+ chip->onfi_timing_mode_default =
+ type->onfi_timing_mode_default;

*busw = type->options & NAND_BUSWIDTH_16;

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h

index b7c1199..e795fbf 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -587,6 +587,11 @@ struct nand_buffers {

* @ecc_step_ds: [INTERN] ECC step required by the @ecc_strength_ds,
* also from the datasheet. It is the recommended ECC step
* size, if known; if unknown, set to zero.

+ * @onfi_timing_mode_default: [INTERN] default ONFI timing mode. This field is
+ * either deduced from the datasheet if the NAND
+ * chip is not ONFI compliant or set to 0 if it is
+ * (an ONFI chip is always configured in mode 0
+ * after a NAND reset)

* @numchips: [INTERN] number of physical chips
* @chipsize: [INTERN] the size of one chip for multichip arrays
* @pagemask: [INTERN] page number mask = number of (pages / chip) - 1

@@ -671,6 +676,7 @@ struct nand_chip {

uint8_t bits_per_cell;
uint16_t ecc_strength_ds;
uint16_t ecc_step_ds;

+ int onfi_timing_mode_default;
int badblockpos;
int badblockbits;

@@ -773,6 +779,10 @@ struct nand_chip {

* @ecc_step_ds in nand_chip{}, also from the datasheet.
* For example, the "4bit ECC for each 512Byte" can be set with
* NAND_ECC_INFO(4, 512).

+ * @onfi_timing_mode_default: the default ONFI timing mode entered after a NAND
+ * reset. Should be deduced from timings described
+ * in the datasheet.

+ *
*/
struct nand_flash_dev {
char *name;

@@ -793,6 +803,7 @@ struct nand_flash_dev {

uint16_t strength_ds;
uint16_t step_ds;
} ecc;

+ int onfi_timing_mode_default;
};

/**

[Brian Norris]

On Mon, Sep 22, 2014 at 04:25:10PM +0200, Boris BREZILLON wrote:
> Add an onfi_timing_mode_default field to nand_chip and nand_flash_dev in
> order to support NAND timings definition for non-ONFI NAND.
>
> NAND that support better timings mode than the default one have to define
> a new entry in the nand_ids table.
>
> The default timing mode should be deduced from timings description from
> the datasheet and the ONFI specification
> (www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf, chapter 4.15
> "Timing Parameters").
> You should choose the closest mode that fit the timings requirements of
> your NAND chip.
>
> Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>

You have some (new?) checkpatch warnings:

WARNING: please, no space before tabs
#49: FILE: include/linux/mtd/nand.h:591:
+ * ^I^I^I either deduced from the datasheet if the NAND$

WARNING: please, no space before tabs
#50: FILE: include/linux/mtd/nand.h:592:
+ * ^I^I^I chip is not ONFI compliant or set to 0 if it is$

WARNING: please, no space before tabs
#51: FILE: include/linux/mtd/nand.h:593:
+ * ^I^I^I (an ONFI chip is always configured in mode 0$

WARNING: please, no space before tabs
#52: FILE: include/linux/mtd/nand.h:594:
+ * ^I^I^I after a NAND reset)$

total: 0 errors, 4 warnings, 43 lines checked

Your patch has style problems, please review.

If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.

Brian

[Boris BREZILLON]

Add the full description of the Hynix H27UCG8T2ATR-BC NAND chip in the
nand_ids table so that we can later use the NAND ECC infos and ONFI timings
mode in controller drivers.

Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>
---

drivers/mtd/nand/nand_ids.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 3d7c89f..fbde8910 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -46,6 +46,10 @@ struct nand_flash_dev nand_flash_ids[] = {
{"SDTNRGAMA 64G 3.3V 8-bit",
{ .id = {0x45, 0xde, 0x94, 0x93, 0x76, 0x50} },
SZ_16K, SZ_8K, SZ_4M, 0, 6, 1280, NAND_ECC_INFO(40, SZ_1K) },
+ {"H27UCG8T2ATR-BC 64G 3.3V 8-bit",
+ { .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
+ SZ_8K, SZ_8K, SZ_2M, 0, 6, 640, NAND_ECC_INFO(40, SZ_1K),
+ 4 },

LEGACY_ID_NAND("NAND 4MiB 5V 8-bit", 0x6B, 4, SZ_8K, SP_OPTIONS),
LEGACY_ID_NAND("NAND 4MiB 3,3V 8-bit", 0xE3, 4, SZ_8K, SP_OPTIONS),

[Boris BREZILLON]

On Mon, 22 Sep 2014 10:58:34 -0700
Brian Norris <computer...@gmail.com> wrote:

> On Mon, Sep 22, 2014 at 04:25:10PM +0200, Boris BREZILLON wrote:
> > Add an onfi_timing_mode_default field to nand_chip and nand_flash_dev in
> > order to support NAND timings definition for non-ONFI NAND.
> >
> > NAND that support better timings mode than the default one have to define
> > a new entry in the nand_ids table.
> >
> > The default timing mode should be deduced from timings description from
> > the datasheet and the ONFI specification
> > (www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf, chapter 4.15
> > "Timing Parameters").
> > You should choose the closest mode that fit the timings requirements of
> > your NAND chip.
> >
> > Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>
>
> You have some (new?) checkpatch warnings:
>

Sorry for that. I just sent a new version fixing those warnings.

Best Regards,

Boris

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

[Boris BREZILLON]

Add an onfi_timing_mode_default field to nand_chip and nand_flash_dev in
order to support NAND timings definition for non-ONFI NAND.

NAND that support better timings mode than the default one have to define
a new entry in the nand_ids table.

The default timing mode should be deduced from timings description from
the datasheet and the ONFI specification
(www.onfi.org/~/media/ONFI/specs/onfi_3_1_spec.pdf, chapter 4.15
"Timing Parameters").
You should choose the closest mode that fit the timings requirements of
your NAND chip.

Signed-off-by: Boris BREZILLON <boris.b...@free-electrons.com>

---
drivers/mtd/nand/nand_base.c | 2 ++
include/linux/mtd/nand.h | 11 +++++++++++
2 files changed, 13 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ae6e7c4..c37fa2a 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3594,6 +3594,8 @@ static bool find_full_id_nand(struct mtd_info *mtd, struct nand_chip *chip,
chip->options |= type->options;
chip->ecc_strength_ds = NAND_ECC_STRENGTH(type);
chip->ecc_step_ds = NAND_ECC_STEP(type);
+ chip->onfi_timing_mode_default =
+ type->onfi_timing_mode_default;

*busw = type->options & NAND_BUSWIDTH_16;

diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h

index b7c1199..b0b74cc 100644

[Boris BREZILLON]

Hi,

Following the series adding ONFI timing mode support, here is a series
adding support for timing mode retrieval on non-ONFI NANDs.

It just adds a new field to the nand_chip and nand_flash_dev struct so
that anyone can define its chip requirements in the nand_ids table.

The 2nd patch serves as an example and adds an entry for the Hynix
H27UCG8T2ATR-BC NAND chip used on the Cubietruck board.

Best Regards,

Boris

Changes since v2:
- fix checkpatch warnings

Changes since v1:
- fix H27UCG8T2ATR-BC definition
- rename onfi_timing_mode_ds field into onfi_timing_mode_default

Boris BREZILLON (2):
mtd: nand: support ONFI timing mode retrieval for non-ONFI NANDs
mtd: nand: add Hynix's H27UCG8T2ATR-BC to nand_ids table

drivers/mtd/nand/nand_base.c | 2 ++
drivers/mtd/nand/nand_ids.c | 4 ++++
include/linux/mtd/nand.h | 11 +++++++++++
3 files changed, 17 insertions(+)

[Brian Norris]

> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index b7c1199..b0b74cc 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -587,6 +587,11 @@ struct nand_buffers {
> * @ecc_step_ds: [INTERN] ECC step required by the @ecc_strength_ds,
> * also from the datasheet. It is the recommended ECC step
> * size, if known; if unknown, set to zero.
> + * @onfi_timing_mode_default: [INTERN] default ONFI timing mode. This field is
> + * either deduced from the datasheet if the NAND
> + * chip is not ONFI compliant or set to 0 if it is
> + * (an ONFI chip is always configured in mode 0
> + * after a NAND reset)

This is probably OK only if every NAND chip is at least as fast as ONFI
mode 0. For older / legacy flash, I'm not sure if that's 100% true.
Maybe we'll need an UNKNOWN value, for those whose timing information is
not known?

Anyway, I think this is OK for now. Pushed the series to l2-mtd.git.
Thanks!

> * @numchips: [INTERN] number of physical chips
> * @chipsize: [INTERN] the size of one chip for multichip arrays
> * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1

Brian