* [PATCH v4 00/19] Allow vendor drivers to propose their own timings
@ 2020-05-25 17:42 Miquel Raynal
2020-05-25 17:42 ` [PATCH v4 01/19] mtd: rawnand: Use unsigned types for nand_chip unsigned values Miquel Raynal
` (18 more replies)
0 siblings, 19 replies; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
As raised by Rickard, certain chips like Toshiba/Kioxia
TH58NVG2S3HBAI4 are not ONFI compliant and because of that, work at a
very slow pace. This chip in particular supports running at a pace
"close" to ONFI mode 4.
This series provides a way to manufacturer drivers to propose a data
interface to the core with a very simple interface (see patch 19/19).
Cheers,
Miquèl
Changes since v3:
* Took Rickard patch based on my previous proposals over Github and
tweaked a few more things:
- Added a "generic" helper to fallback on slower ONFI modes when the
proposed interface is not supported by the controller.
- Fixed more kdoc.
- Simplified the vendor driver side by providing additional helpers.
* Tweaked a little bit Rickard patch to fit the latest changes.
Miquel Raynal (18):
mtd: rawnand: Use unsigned types for nand_chip unsigned values
mtd: rawnand: Only use u8 instead of uint8_t in nand_chip structure
mtd: rawnand: Create a nand_chip operations structure
mtd: rawnand: Rename the manufacturer structure
mtd: rawnand: Declare the nand_manufacturer structure out of nand_chip
mtd: rawnand: Reorganize the nand_chip structure
mtd: rawnand: Compare the actual timing values
mtd: rawnand: Rename onfi_timing_mode_default
mtd: rawnand: Use the data interface mode entry when relevant
mtd: rawnand: Rename nand_has_setup_data_interface()
mtd: rawnand: Fix nand_setup_data_interface() description
mtd: rawnand: Rename nand_init_data_interface()
mtd: rawnand: timings: Update onfi_fill_data_interface() kernel doc
mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data
interface
mtd: rawnand: timings: Add a helper to find the closest ONFI mode
mtd: rawnand: Introduce nand_choose_best_sdr_iface()
mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
mtd: rawnand: Add the ->choose_data_interface() hook
Rickard x Andersson (1):
mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4
drivers/mtd/nand/raw/internals.h | 17 +-
drivers/mtd/nand/raw/nand_base.c | 181 ++++++++++++++--------
drivers/mtd/nand/raw/nand_hynix.c | 2 +-
drivers/mtd/nand/raw/nand_ids.c | 19 ++-
drivers/mtd/nand/raw/nand_legacy.c | 2 +-
drivers/mtd/nand/raw/nand_macronix.c | 10 +-
drivers/mtd/nand/raw/nand_micron.c | 2 +-
drivers/mtd/nand/raw/nand_timings.c | 62 +++++++-
drivers/mtd/nand/raw/nand_toshiba.c | 40 ++++-
include/linux/mtd/rawnand.h | 224 +++++++++++++--------------
10 files changed, 354 insertions(+), 205 deletions(-)
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 01/19] mtd: rawnand: Use unsigned types for nand_chip unsigned values
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 18:33 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 02/19] mtd: rawnand: Only use u8 instead of uint8_t in nand_chip structure Miquel Raynal
` (17 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
page_shift, phys_erase_shift, bbt_erase_shift, chip_shift, pagemask,
subpagesize and badblockbits are all positive values, so declare
them as unsigned.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
include/linux/mtd/rawnand.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 2804c13e5662..8a1e0192f78e 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1110,11 +1110,11 @@ struct nand_chip {
unsigned int options;
unsigned int bbt_options;
- int page_shift;
- int phys_erase_shift;
- int bbt_erase_shift;
- int chip_shift;
- int pagemask;
+ unsigned int page_shift;
+ unsigned int phys_erase_shift;
+ unsigned int bbt_erase_shift;
+ unsigned int chip_shift;
+ unsigned int pagemask;
u8 *data_buf;
struct {
@@ -1122,10 +1122,10 @@ struct nand_chip {
int page;
} pagecache;
- int subpagesize;
+ unsigned int subpagesize;
int onfi_timing_mode_default;
unsigned int badblockpos;
- int badblockbits;
+ unsigned int badblockbits;
struct nand_id id;
struct nand_parameters parameters;
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 02/19] mtd: rawnand: Only use u8 instead of uint8_t in nand_chip structure
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
2020-05-25 17:42 ` [PATCH v4 01/19] mtd: rawnand: Use unsigned types for nand_chip unsigned values Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 18:36 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 03/19] mtd: rawnand: Create a nand_chip operations structure Miquel Raynal
` (16 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
Mechanical change to avoid using old types.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
include/linux/mtd/rawnand.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 8a1e0192f78e..7d62e0e719ac 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1141,13 +1141,13 @@ struct nand_chip {
int (*suspend)(struct nand_chip *chip);
void (*resume)(struct nand_chip *chip);
- uint8_t *oob_poi;
+ u8 *oob_poi;
struct nand_controller *controller;
struct nand_ecc_ctrl ecc;
unsigned long buf_align;
- uint8_t *bbt;
+ u8 *bbt;
struct nand_bbt_descr *bbt_td;
struct nand_bbt_descr *bbt_md;
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 03/19] mtd: rawnand: Create a nand_chip operations structure
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
2020-05-25 17:42 ` [PATCH v4 01/19] mtd: rawnand: Use unsigned types for nand_chip unsigned values Miquel Raynal
2020-05-25 17:42 ` [PATCH v4 02/19] mtd: rawnand: Only use u8 instead of uint8_t in nand_chip structure Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 18:37 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 04/19] mtd: rawnand: Rename the manufacturer structure Miquel Raynal
` (15 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
And move nand_chip hooks there.
While moving entries from one structure to the other, adapt the
documentation style.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_base.c | 20 ++++++++---------
drivers/mtd/nand/raw/nand_hynix.c | 2 +-
drivers/mtd/nand/raw/nand_macronix.c | 10 ++++-----
drivers/mtd/nand/raw/nand_micron.c | 2 +-
include/linux/mtd/rawnand.h | 32 ++++++++++++++++------------
5 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 6a6a0a36b3fd..b86f641f6d74 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3285,10 +3285,10 @@ static int nand_setup_read_retry(struct nand_chip *chip, int retry_mode)
if (retry_mode >= chip->read_retries)
return -EINVAL;
- if (!chip->setup_read_retry)
+ if (!chip->ops.setup_read_retry)
return -EOPNOTSUPP;
- return chip->setup_read_retry(chip, retry_mode);
+ return chip->ops.setup_read_retry(chip, retry_mode);
}
static void nand_wait_readrdy(struct nand_chip *chip)
@@ -4532,8 +4532,8 @@ static int nand_suspend(struct mtd_info *mtd)
int ret = 0;
mutex_lock(&chip->lock);
- if (chip->suspend)
- ret = chip->suspend(chip);
+ if (chip->ops.suspend)
+ ret = chip->ops.suspend(chip);
if (!ret)
chip->suspended = 1;
mutex_unlock(&chip->lock);
@@ -4551,8 +4551,8 @@ static void nand_resume(struct mtd_info *mtd)
mutex_lock(&chip->lock);
if (chip->suspended) {
- if (chip->resume)
- chip->resume(chip);
+ if (chip->ops.resume)
+ chip->ops.resume(chip);
chip->suspended = 0;
} else {
pr_err("%s called for a chip which is not in suspended state\n",
@@ -4581,10 +4581,10 @@ static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
struct nand_chip *chip = mtd_to_nand(mtd);
- if (!chip->lock_area)
+ if (!chip->ops.lock_area)
return -ENOTSUPP;
- return chip->lock_area(chip, ofs, len);
+ return chip->ops.lock_area(chip, ofs, len);
}
/**
@@ -4597,10 +4597,10 @@ static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
struct nand_chip *chip = mtd_to_nand(mtd);
- if (!chip->unlock_area)
+ if (!chip->ops.unlock_area)
return -ENOTSUPP;
- return chip->unlock_area(chip, ofs, len);
+ return chip->ops.unlock_area(chip, ofs, len);
}
/* Set default functions */
diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
index 7caedaa5b9e5..7d1be53f27f3 100644
--- a/drivers/mtd/nand/raw/nand_hynix.c
+++ b/drivers/mtd/nand/raw/nand_hynix.c
@@ -337,7 +337,7 @@ static int hynix_mlc_1xnm_rr_init(struct nand_chip *chip,
rr->nregs = nregs;
rr->regs = hynix_1xnm_mlc_read_retry_regs;
hynix->read_retry = rr;
- chip->setup_read_retry = hynix_nand_setup_read_retry;
+ chip->ops.setup_read_retry = hynix_nand_setup_read_retry;
chip->read_retries = nmodes;
out:
diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
index 09c254c97b5c..1472f925f386 100644
--- a/drivers/mtd/nand/raw/nand_macronix.c
+++ b/drivers/mtd/nand/raw/nand_macronix.c
@@ -130,7 +130,7 @@ static void macronix_nand_onfi_init(struct nand_chip *chip)
return;
chip->read_retries = MACRONIX_NUM_READ_RETRY_MODES;
- chip->setup_read_retry = macronix_nand_setup_read_retry;
+ chip->ops.setup_read_retry = macronix_nand_setup_read_retry;
if (p->supports_set_get_features) {
bitmap_set(p->set_feature_list,
@@ -242,8 +242,8 @@ static void macronix_nand_block_protection_support(struct nand_chip *chip)
bitmap_set(chip->parameters.set_feature_list,
ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
- chip->lock_area = mxic_nand_lock;
- chip->unlock_area = mxic_nand_unlock;
+ chip->ops.lock_area = mxic_nand_lock;
+ chip->ops.unlock_area = mxic_nand_unlock;
}
static int nand_power_down_op(struct nand_chip *chip)
@@ -312,8 +312,8 @@ static void macronix_nand_deep_power_down_support(struct nand_chip *chip)
if (i < 0)
return;
- chip->suspend = mxic_nand_suspend;
- chip->resume = mxic_nand_resume;
+ chip->ops.suspend = mxic_nand_suspend;
+ chip->ops.resume = mxic_nand_resume;
}
static int macronix_nand_init(struct nand_chip *chip)
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 3589b4fce0d4..4385092a9325 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -84,7 +84,7 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
struct nand_onfi_vendor_micron *micron = (void *)p->onfi->vendor;
chip->read_retries = micron->read_retry_options;
- chip->setup_read_retry = micron_nand_setup_read_retry;
+ chip->ops.setup_read_retry = micron_nand_setup_read_retry;
}
if (p->supports_set_get_features) {
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 7d62e0e719ac..b33cd68852c4 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1027,16 +1027,31 @@ struct nand_legacy {
struct nand_controller dummy_controller;
};
+/**
+ * struct nand_chip_ops - NAND chip operations
+ * @suspend: Suspend operation
+ * @resume: Resume operation
+ * @lock_area: Lock operation
+ * @unlock_area: Unlock operation
+ * @setup_read_retry: Set the read-retry mode (mostly needed for MLC NANDs)
+ */
+struct nand_chip_ops {
+ int (*suspend)(struct nand_chip *chip);
+ void (*resume)(struct nand_chip *chip);
+ int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
+ int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
+ int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
+};
+
/**
* struct nand_chip - NAND Private Flash Chip Data
* @base: Inherit from the generic NAND device
+ * @ops: NAND chip operations
* @legacy: All legacy fields/hooks. If you develop a new driver,
* don't even try to use any of these fields/hooks, and if
* you're modifying an existing driver that is using those
* fields/hooks, you should consider reworking the driver
* avoid using them.
- * @setup_read_retry: [FLASHSPECIFIC] flash (vendor) specific function for
- * setting the read-retry mode. Mostly needed for MLC NAND.
* @ecc: [BOARDSPECIFIC] ECC control structure
* @buf_align: minimum buffer alignment required by a platform
* @oob_poi: "poison value buffer," used for laying out OOB data
@@ -1081,8 +1096,6 @@ struct nand_legacy {
* @lock: lock protecting the suspended field. Also used to
* serialize accesses to the NAND device.
* @suspended: set to 1 when the device is suspended, 0 when it's not.
- * @suspend: [REPLACEABLE] specific NAND device suspend operation
- * @resume: [REPLACEABLE] specific NAND device resume operation
* @bbt: [INTERN] bad block table pointer
* @bbt_td: [REPLACEABLE] bad block table descriptor for flash
* lookup.
@@ -1096,17 +1109,13 @@ struct nand_legacy {
* @manufacturer: [INTERN] Contains manufacturer information
* @manufacturer.desc: [INTERN] Contains manufacturer's description
* @manufacturer.priv: [INTERN] Contains manufacturer private information
- * @lock_area: [REPLACEABLE] specific NAND chip lock operation
- * @unlock_area: [REPLACEABLE] specific NAND chip unlock operation
*/
struct nand_chip {
struct nand_device base;
-
+ struct nand_chip_ops ops;
struct nand_legacy legacy;
- int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
-
unsigned int options;
unsigned int bbt_options;
@@ -1138,8 +1147,6 @@ struct nand_chip {
struct mutex lock;
unsigned int suspended : 1;
- int (*suspend)(struct nand_chip *chip);
- void (*resume)(struct nand_chip *chip);
u8 *oob_poi;
struct nand_controller *controller;
@@ -1159,9 +1166,6 @@ struct nand_chip {
const struct nand_manufacturer *desc;
void *priv;
} manufacturer;
-
- int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
- int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
};
extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 04/19] mtd: rawnand: Rename the manufacturer structure
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (2 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 03/19] mtd: rawnand: Create a nand_chip operations structure Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 18:38 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 05/19] mtd: rawnand: Declare the nand_manufacturer structure out of nand_chip Miquel Raynal
` (14 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
It is currently called nand_manufacturer but could actually be called
nand_manufacturer_desc, like its instances, so that the former name is
left unused for now.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/internals.h | 6 +++---
drivers/mtd/nand/raw/nand_base.c | 14 +++++++-------
drivers/mtd/nand/raw/nand_ids.c | 16 ++++++++--------
include/linux/mtd/rawnand.h | 2 +-
4 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index 03866b0aadea..a518acfd9b3f 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -53,12 +53,12 @@ struct nand_manufacturer_ops {
};
/**
- * struct nand_manufacturer - NAND Flash Manufacturer structure
+ * struct nand_manufacturer_desc - NAND Flash Manufacturer descriptor
* @name: Manufacturer name
* @id: manufacturer ID code of device.
* @ops: manufacturer operations
*/
-struct nand_manufacturer {
+struct nand_manufacturer_desc {
int id;
char *name;
const struct nand_manufacturer_ops *ops;
@@ -79,7 +79,7 @@ extern const struct nand_manufacturer_ops toshiba_nand_manuf_ops;
extern const struct mtd_pairing_scheme dist3_pairing_scheme;
/* Core functions */
-const struct nand_manufacturer *nand_get_manufacturer(u8 id);
+const struct nand_manufacturer_desc *nand_get_manufacturer_desc(u8 id);
int nand_bbm_get_next_page(struct nand_chip *chip, int page);
int nand_markbad_bbm(struct nand_chip *chip, loff_t ofs);
int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index b86f641f6d74..14f1359a60b8 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4880,9 +4880,9 @@ static void nand_manufacturer_cleanup(struct nand_chip *chip)
}
static const char *
-nand_manufacturer_name(const struct nand_manufacturer *manufacturer)
+nand_manufacturer_name(const struct nand_manufacturer_desc *manufacturer_desc)
{
- return manufacturer ? manufacturer->name : "Unknown";
+ return manufacturer_desc ? manufacturer_desc->name : "Unknown";
}
/*
@@ -4890,7 +4890,7 @@ nand_manufacturer_name(const struct nand_manufacturer *manufacturer)
*/
static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
{
- const struct nand_manufacturer *manufacturer;
+ const struct nand_manufacturer_desc *manufacturer_desc;
struct mtd_info *mtd = nand_to_mtd(chip);
struct nand_memory_organization *memorg;
int busw, ret;
@@ -4947,8 +4947,8 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
chip->id.len = nand_id_len(id_data, ARRAY_SIZE(chip->id.data));
/* Try to identify manufacturer */
- manufacturer = nand_get_manufacturer(maf_id);
- chip->manufacturer.desc = manufacturer;
+ manufacturer_desc = nand_get_manufacturer_desc(maf_id);
+ chip->manufacturer.desc = manufacturer_desc;
if (!type)
type = nand_flash_ids;
@@ -5027,7 +5027,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
*/
pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
maf_id, dev_id);
- pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
+ pr_info("%s %s\n", nand_manufacturer_name(manufacturer_desc),
mtd->name);
pr_warn("bus width %d instead of %d bits\n", busw ? 16 : 8,
(chip->options & NAND_BUSWIDTH_16) ? 16 : 8);
@@ -5062,7 +5062,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
maf_id, dev_id);
- pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
+ pr_info("%s %s\n", nand_manufacturer_name(manufacturer_desc),
chip->parameters.model);
pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
(int)(targetsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
index ba27902fc54b..e0dbc2e316c7 100644
--- a/drivers/mtd/nand/raw/nand_ids.c
+++ b/drivers/mtd/nand/raw/nand_ids.c
@@ -166,7 +166,7 @@ struct nand_flash_dev nand_flash_ids[] = {
};
/* Manufacturer IDs */
-static const struct nand_manufacturer nand_manufacturers[] = {
+static const struct nand_manufacturer_desc nand_manufacturer_descs[] = {
{NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
{NAND_MFR_ATO, "ATO"},
{NAND_MFR_EON, "Eon"},
@@ -186,20 +186,20 @@ static const struct nand_manufacturer nand_manufacturers[] = {
};
/**
- * nand_get_manufacturer - Get manufacturer information from the manufacturer
- * ID
+ * nand_get_manufacturer_desc - Get manufacturer information from the
+ * manufacturer ID
* @id: manufacturer ID
*
- * Returns a pointer a nand_manufacturer object if the manufacturer is defined
+ * Returns a nand_manufacturer_desc object if the manufacturer is defined
* in the NAND manufacturers database, NULL otherwise.
*/
-const struct nand_manufacturer *nand_get_manufacturer(u8 id)
+const struct nand_manufacturer_desc *nand_get_manufacturer_desc(u8 id)
{
int i;
- for (i = 0; i < ARRAY_SIZE(nand_manufacturers); i++)
- if (nand_manufacturers[i].id == id)
- return &nand_manufacturers[i];
+ for (i = 0; i < ARRAY_SIZE(nand_manufacturer_descs); i++)
+ if (nand_manufacturer_descs[i].id == id)
+ return &nand_manufacturer_descs[i];
return NULL;
}
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index b33cd68852c4..d8492d966b40 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1163,7 +1163,7 @@ struct nand_chip {
void *priv;
struct {
- const struct nand_manufacturer *desc;
+ const struct nand_manufacturer_desc *desc;
void *priv;
} manufacturer;
};
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 05/19] mtd: rawnand: Declare the nand_manufacturer structure out of nand_chip
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (3 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 04/19] mtd: rawnand: Rename the manufacturer structure Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 18:40 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 06/19] mtd: rawnand: Reorganize the nand_chip structure Miquel Raynal
` (13 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
Now that struct nand_manufacturer type is free, use it to store the
nand_manufacturer_desc and the manufacturer's private data.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
include/linux/mtd/rawnand.h | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index d8492d966b40..2a9b5d5b315b 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1043,10 +1043,21 @@ struct nand_chip_ops {
int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
};
+/**
+ * struct nand_manufacturer - NAND manufacturer structure
+ * @desc: The manufacturer description
+ * @priv: Private information for the manufacturer driver
+ */
+struct nand_manufacturer {
+ const struct nand_manufacturer_desc *desc;
+ void *priv;
+};
+
/**
* struct nand_chip - NAND Private Flash Chip Data
* @base: Inherit from the generic NAND device
* @ops: NAND chip operations
+ * @manufacturer: Manufacturer information
* @legacy: All legacy fields/hooks. If you develop a new driver,
* don't even try to use any of these fields/hooks, and if
* you're modifying an existing driver that is using those
@@ -1106,13 +1117,11 @@ struct nand_chip_ops {
* structure which is shared among multiple independent
* devices.
* @priv: [OPTIONAL] pointer to private chip data
- * @manufacturer: [INTERN] Contains manufacturer information
- * @manufacturer.desc: [INTERN] Contains manufacturer's description
- * @manufacturer.priv: [INTERN] Contains manufacturer private information
*/
struct nand_chip {
struct nand_device base;
+ struct nand_manufacturer manufacturer;
struct nand_chip_ops ops;
struct nand_legacy legacy;
@@ -1161,11 +1170,6 @@ struct nand_chip {
struct nand_bbt_descr *badblock_pattern;
void *priv;
-
- struct {
- const struct nand_manufacturer_desc *desc;
- void *priv;
- } manufacturer;
};
extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 06/19] mtd: rawnand: Reorganize the nand_chip structure
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (4 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 05/19] mtd: rawnand: Declare the nand_manufacturer structure out of nand_chip Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 18:55 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 07/19] mtd: rawnand: Compare the actual timing values Miquel Raynal
` (12 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
Reorder fields in this structure and pack entries by theme:
* The main descriptive structures
* The data interface details
* Bad block information
* The device layout
* Extra buffers matching the device layout
* Internal values
* External objects like the ECC controller, the ECC engine and a
private data pointer.
While at it, adapt the documentation style.
I changed on purpose the description of @oob_poi which was weird.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
include/linux/mtd/rawnand.h | 166 +++++++++++++++++-------------------
1 file changed, 76 insertions(+), 90 deletions(-)
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 2a9b5d5b315b..622da6527a36 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1055,120 +1055,106 @@ struct nand_manufacturer {
/**
* struct nand_chip - NAND Private Flash Chip Data
- * @base: Inherit from the generic NAND device
- * @ops: NAND chip operations
- * @manufacturer: Manufacturer information
- * @legacy: All legacy fields/hooks. If you develop a new driver,
- * don't even try to use any of these fields/hooks, and if
- * you're modifying an existing driver that is using those
- * fields/hooks, you should consider reworking the driver
- * avoid using them.
- * @ecc: [BOARDSPECIFIC] ECC control structure
- * @buf_align: minimum buffer alignment required by a platform
- * @oob_poi: "poison value buffer," used for laying out OOB data
- * before writing
- * @page_shift: [INTERN] number of address bits in a page (column
- * address bits).
- * @phys_erase_shift: [INTERN] number of address bits in a physical eraseblock
- * @bbt_erase_shift: [INTERN] number of address bits in a bbt entry
- * @chip_shift: [INTERN] number of address bits in one chip
- * @options: [BOARDSPECIFIC] various chip options. They can partly
- * be set to inform nand_scan about special functionality.
- * See the defines for further explanation.
- * @bbt_options: [INTERN] bad block specific options. All options used
- * here must come from bbm.h. By default, these options
- * will be copied to the appropriate nand_bbt_descr's.
- * @badblockpos: [INTERN] position of the bad block marker in the oob
- * area.
- * @badblockbits: [INTERN] minimum number of set bits in a good block's
- * bad block marker position; i.e., BBM == 11110111b is
- * not bad when badblockbits == 7
- * @onfi_timing_mode_default: [INTERN] default ONFI timing mode. This field is
- * set to the actually used ONFI mode if the chip is
- * ONFI compliant or deduced from the datasheet if
- * the NAND chip is not ONFI compliant.
- * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1
- * @data_buf: [INTERN] buffer for data, size is (page size + oobsize).
- * @pagecache: Structure containing page cache related fields
- * @pagecache.bitflips: Number of bitflips of the cached page
- * @pagecache.page: Page number currently in the cache. -1 means no page is
- * currently cached
- * @subpagesize: [INTERN] holds the subpagesize
- * @id: [INTERN] holds NAND ID
- * @parameters: [INTERN] holds generic parameters under an easily
- * readable form.
- * @data_interface: [INTERN] NAND interface timing information
- * @cur_cs: currently selected target. -1 means no target selected,
- * otherwise we should always have cur_cs >= 0 &&
- * cur_cs < nanddev_ntargets(). NAND Controller drivers
- * should not modify this value, but they're allowed to
- * read it.
- * @read_retries: [INTERN] the number of read retry modes supported
- * @lock: lock protecting the suspended field. Also used to
- * serialize accesses to the NAND device.
- * @suspended: set to 1 when the device is suspended, 0 when it's not.
- * @bbt: [INTERN] bad block table pointer
- * @bbt_td: [REPLACEABLE] bad block table descriptor for flash
- * lookup.
- * @bbt_md: [REPLACEABLE] bad block table mirror descriptor
- * @badblock_pattern: [REPLACEABLE] bad block scan pattern used for initial
- * bad block scan.
- * @controller: [REPLACEABLE] a pointer to a hardware controller
- * structure which is shared among multiple independent
- * devices.
- * @priv: [OPTIONAL] pointer to private chip data
+ * @base: Inherit from the generic NAND device
+ * @id: Holds NAND ID
+ * @parameters: Holds generic parameters under an easily readable form
+ * @manufacturer: Manufacturer information
+ * @ops: NAND chip operations
+ * @legacy: All legacy fields/hooks. If you develop a new driver, don't even try
+ * to use any of these fields/hooks, and if you're modifying an
+ * existing driver that is using those fields/hooks, you should
+ * consider reworking the driver and avoid using them.
+ * @options: Various chip options. They can partly be set to inform nand_scan
+ * about special functionality. See the defines for further
+ * explanation.
+ * @onfi_timing_mode_default: Default ONFI timing mode. This field is set to the
+ * actually used ONFI mode if the chip is ONFI
+ * compliant or deduced from the datasheet otherwise
+ * @data_interface: NAND interface timing information
+ * @bbt_erase_shift: Number of address bits in a bbt entry
+ * @bbt_options: Bad block table specific options. All options used here must
+ * come from bbm.h. By default, these options will be copied to
+ * the appropriate nand_bbt_descr's.
+ * @badblockpos: Bad block marker position in the oob area
+ * @badblockbits: Minimum number of set bits in a good block's bad block marker
+ * position; i.e., BBM = 11110111b is good when badblockbits = 7
+ * @bbt_td: Bad block table descriptor for flash lookup
+ * @bbt_md: Bad block table mirror descriptor
+ * @badblock_pattern: Bad block scan pattern used for initial bad block scan
+ * @bbt: Bad block table pointer
+ * @page_shift: Number of address bits in a page (column address bits)
+ * @phys_erase_shift: Number of address bits in a physical eraseblock
+ * @chip_shift: Number of address bits in one chip
+ * @pagemask: Page number mask = number of (pages / chip) - 1
+ * @subpagesize: Holds the subpagesize
+ * @data_buf: Buffer for data, size is (page size + oobsize)
+ * @oob_poi: pointer on the OOB area covered by data_buf
+ * @pagecache: Structure containing page cache related fields
+ * @pagecache.bitflips: Number of bitflips of the cached page
+ * @pagecache.page: Page number currently in the cache. -1 means no page is
+ * currently cached
+ * @buf_align: Minimum buffer alignment required by a platform
+ * @lock: Lock protecting the suspended field. Also used to serialize accesses
+ * to the NAND device
+ * @suspended: Set to 1 when the device is suspended, 0 when it's not
+ * @cur_cs: Currently selected target. -1 means no target selected, otherwise we
+ * should always have cur_cs >= 0 && cur_cs < nanddev_ntargets().
+ * NAND Controller drivers should not modify this value, but they're
+ * allowed to read it.
+ * @read_retries: The number of read retry modes supported
+ * @controller: The hardware controller structure which is shared among multiple
+ * independent devices
+ * @ecc: The ECC controller structure
+ * @priv: Chip private data
*/
-
struct nand_chip {
struct nand_device base;
+ struct nand_id id;
+ struct nand_parameters parameters;
struct nand_manufacturer manufacturer;
struct nand_chip_ops ops;
struct nand_legacy legacy;
-
unsigned int options;
+
+ /* Data interface */
+ int onfi_timing_mode_default;
+ struct nand_data_interface data_interface;
+
+ /* Bad block information */
+ unsigned int bbt_erase_shift;
unsigned int bbt_options;
+ unsigned int badblockpos;
+ unsigned int badblockbits;
+ struct nand_bbt_descr *bbt_td;
+ struct nand_bbt_descr *bbt_md;
+ struct nand_bbt_descr *badblock_pattern;
+ u8 *bbt;
+ /* Device internal layout */
unsigned int page_shift;
unsigned int phys_erase_shift;
- unsigned int bbt_erase_shift;
unsigned int chip_shift;
unsigned int pagemask;
+ unsigned int subpagesize;
+
+ /* Buffers */
u8 *data_buf;
-
+ u8 *oob_poi;
struct {
unsigned int bitflips;
int page;
} pagecache;
+ unsigned long buf_align;
- unsigned int subpagesize;
- int onfi_timing_mode_default;
- unsigned int badblockpos;
- unsigned int badblockbits;
-
- struct nand_id id;
- struct nand_parameters parameters;
-
- struct nand_data_interface data_interface;
-
+ /* Internals */
+ struct mutex lock;
+ unsigned int suspended : 1;
int cur_cs;
-
int read_retries;
- struct mutex lock;
- unsigned int suspended : 1;
-
- u8 *oob_poi;
+ /* Externals */
struct nand_controller *controller;
-
struct nand_ecc_ctrl ecc;
- unsigned long buf_align;
-
- u8 *bbt;
- struct nand_bbt_descr *bbt_td;
- struct nand_bbt_descr *bbt_md;
-
- struct nand_bbt_descr *badblock_pattern;
-
void *priv;
};
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 07/19] mtd: rawnand: Compare the actual timing values
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (5 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 06/19] mtd: rawnand: Reorganize the nand_chip structure Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:01 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 08/19] mtd: rawnand: Rename onfi_timing_mode_default Miquel Raynal
` (11 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
Avoid relying just on the default timing mode to discriminate if the
data interface must be restored. Do a memcmp() instead.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_base.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 14f1359a60b8..7567c973964b 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -2512,7 +2512,8 @@ int nand_reset(struct nand_chip *chip, int chipnr)
* nand_setup_data_interface() uses ->set/get_features() which would
* fail anyway as the parameter page is not available yet.
*/
- if (!chip->onfi_timing_mode_default)
+ if (!memcmp(&chip->data_interface, &saved_data_intf,
+ sizeof(saved_data_intf)))
return 0;
chip->data_interface = saved_data_intf;
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 08/19] mtd: rawnand: Rename onfi_timing_mode_default
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (6 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 07/19] mtd: rawnand: Compare the actual timing values Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:07 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 09/19] mtd: rawnand: Use the data interface mode entry when relevant Miquel Raynal
` (10 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
This parameter would better be named default_timing_mode as we are
opening the way to non-ONFI timings.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_base.c | 20 +++++++++-----------
drivers/mtd/nand/raw/nand_toshiba.c | 2 +-
include/linux/mtd/rawnand.h | 8 ++++----
3 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 7567c973964b..adbc12580e2e 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -952,7 +952,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
{
u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
- chip->onfi_timing_mode_default,
+ chip->default_timing_mode,
};
int ret;
@@ -987,9 +987,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
if (ret)
goto err_reset_chip;
- if (tmode_param[0] != chip->onfi_timing_mode_default) {
+ if (tmode_param[0] != chip->default_timing_mode) {
pr_warn("timing mode %d not acknowledged by the NAND chip\n",
- chip->onfi_timing_mode_default);
+ chip->default_timing_mode);
goto err_reset_chip;
}
@@ -1016,9 +1016,8 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
* and the driver.
* First tries to retrieve supported timing modes from ONFI information,
* and if the NAND chip does not support ONFI, relies on the
- * ->onfi_timing_mode_default specified in the nand_ids table. After this
- * function nand_chip->data_interface is initialized with the best timing mode
- * available.
+ * ->default_timing_mode specified in the nand_ids table. After this function
+ * nand_chip->data_interface is initialized with the best timing mode available.
*
* Returns 0 for success or negative error code otherwise.
*/
@@ -1037,10 +1036,10 @@ static int nand_init_data_interface(struct nand_chip *chip)
if (chip->parameters.onfi) {
modes = chip->parameters.onfi->async_timing_mode;
} else {
- if (!chip->onfi_timing_mode_default)
+ if (!chip->default_timing_mode)
return 0;
- modes = GENMASK(chip->onfi_timing_mode_default, 0);
+ modes = GENMASK(chip->default_timing_mode, 0);
}
for (mode = fls(modes) - 1; mode >= 0; mode--) {
@@ -1056,7 +1055,7 @@ static int nand_init_data_interface(struct nand_chip *chip)
NAND_DATA_IFACE_CHECK_ONLY,
&chip->data_interface);
if (!ret) {
- chip->onfi_timing_mode_default = mode;
+ chip->default_timing_mode = mode;
break;
}
}
@@ -4814,8 +4813,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
chip->options |= type->options;
chip->base.eccreq.strength = NAND_ECC_STRENGTH(type);
chip->base.eccreq.step_size = NAND_ECC_STEP(type);
- chip->onfi_timing_mode_default =
- type->onfi_timing_mode_default;
+ chip->default_timing_mode = type->onfi_timing_mode_default;
chip->parameters.model = kstrdup(type->name, GFP_KERNEL);
if (!chip->parameters.model)
diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
index ae069905d7e4..b6efaf5195bb 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -198,7 +198,7 @@ static int tc58teg5dclta00_init(struct nand_chip *chip)
{
struct mtd_info *mtd = nand_to_mtd(chip);
- chip->onfi_timing_mode_default = 5;
+ chip->default_timing_mode = 5;
chip->options |= NAND_NEED_SCRAMBLING;
mtd_set_pairing_scheme(mtd, &dist3_pairing_scheme);
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index 622da6527a36..a4b68e7b246a 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1067,9 +1067,9 @@ struct nand_manufacturer {
* @options: Various chip options. They can partly be set to inform nand_scan
* about special functionality. See the defines for further
* explanation.
- * @onfi_timing_mode_default: Default ONFI timing mode. This field is set to the
- * actually used ONFI mode if the chip is ONFI
- * compliant or deduced from the datasheet otherwise
+ * @default_timing_mode: Default timing mode. This field is set to the actually
+ * used ONFI mode if the chip is ONFI compliant or deduced
+ * from the datasheet otherwise
* @data_interface: NAND interface timing information
* @bbt_erase_shift: Number of address bits in a bbt entry
* @bbt_options: Bad block table specific options. All options used here must
@@ -1117,7 +1117,7 @@ struct nand_chip {
unsigned int options;
/* Data interface */
- int onfi_timing_mode_default;
+ int default_timing_mode;
struct nand_data_interface data_interface;
/* Bad block information */
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 09/19] mtd: rawnand: Use the data interface mode entry when relevant
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (7 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 08/19] mtd: rawnand: Rename onfi_timing_mode_default Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:09 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 10/19] mtd: rawnand: Rename nand_has_setup_data_interface() Miquel Raynal
` (9 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
The data interface setup does not care about the default timing mode
but cares about the actual timing mode at the time of the call of this
helper.
Use this entry instead and let chip->default_timing_mode only be used
at initialization time.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_base.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index adbc12580e2e..514ac78899ec 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -951,9 +951,8 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
*/
static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
{
- u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
- chip->default_timing_mode,
- };
+ u8 mode = chip->data_interface.timings.mode;
+ u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { mode, };
int ret;
if (!nand_has_setup_data_iface(chip))
@@ -987,9 +986,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
if (ret)
goto err_reset_chip;
- if (tmode_param[0] != chip->default_timing_mode) {
+ if (tmode_param[0] != mode) {
pr_warn("timing mode %d not acknowledged by the NAND chip\n",
- chip->default_timing_mode);
+ mode);
goto err_reset_chip;
}
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 10/19] mtd: rawnand: Rename nand_has_setup_data_interface()
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (8 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 09/19] mtd: rawnand: Use the data interface mode entry when relevant Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:10 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 11/19] mtd: rawnand: Fix nand_setup_data_interface() description Miquel Raynal
` (8 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
This is really a NAND controller hook so call it
nand_controller_has_setup_data_interface(), which makes much more
sense.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/internals.h | 2 +-
drivers/mtd/nand/raw/nand_base.c | 6 +++---
drivers/mtd/nand/raw/nand_legacy.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index a518acfd9b3f..347d42c55353 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -130,7 +130,7 @@ static inline int nand_exec_op(struct nand_chip *chip,
return chip->controller->ops->exec_op(chip, op, false);
}
-static inline bool nand_has_setup_data_iface(struct nand_chip *chip)
+static inline bool nand_controller_has_setup_data_iface(struct nand_chip *chip)
{
if (!chip->controller || !chip->controller->ops ||
!chip->controller->ops->setup_data_interface)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 514ac78899ec..d446394a2ea0 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -910,7 +910,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
{
int ret;
- if (!nand_has_setup_data_iface(chip))
+ if (!nand_controller_has_setup_data_iface(chip))
return 0;
/*
@@ -955,7 +955,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { mode, };
int ret;
- if (!nand_has_setup_data_iface(chip))
+ if (!nand_controller_has_setup_data_iface(chip))
return 0;
/* Change the mode on the chip side (if supported by the NAND chip) */
@@ -1024,7 +1024,7 @@ static int nand_init_data_interface(struct nand_chip *chip)
{
int modes, mode, ret;
- if (!nand_has_setup_data_iface(chip))
+ if (!nand_controller_has_setup_data_iface(chip))
return 0;
/*
diff --git a/drivers/mtd/nand/raw/nand_legacy.c b/drivers/mtd/nand/raw/nand_legacy.c
index d64791c06a97..8b91aa7773d8 100644
--- a/drivers/mtd/nand/raw/nand_legacy.c
+++ b/drivers/mtd/nand/raw/nand_legacy.c
@@ -365,7 +365,7 @@ static void nand_ccs_delay(struct nand_chip *chip)
* Wait tCCS_min if it is correctly defined, otherwise wait 500ns
* (which should be safe for all NANDs).
*/
- if (nand_has_setup_data_iface(chip))
+ if (nand_controller_has_setup_data_iface(chip))
ndelay(chip->data_interface.timings.sdr.tCCS_min / 1000);
else
ndelay(500);
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 11/19] mtd: rawnand: Fix nand_setup_data_interface() description
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (9 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 10/19] mtd: rawnand: Rename nand_has_setup_data_interface() Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:13 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 12/19] mtd: rawnand: Rename nand_init_data_interface() Miquel Raynal
` (7 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
This is a copy/paste error and belongs to nand_init_data_interface()
description.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_base.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d446394a2ea0..c8f5d4557ed5 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -941,11 +941,8 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
* @chip: The NAND chip
* @chipnr: Internal die id
*
- * Find and configure the best data interface and NAND timings supported by
- * the chip and the driver.
- * First tries to retrieve supported timing modes from ONFI information,
- * and if the NAND chip does not support ONFI, relies on the
- * ->onfi_timing_mode_default specified in the nand_ids table.
+ * Configure what has been reported to be the best data interface and NAND
+ * timings supported by the chip and the driver.
*
* Returns 0 for success or negative error code otherwise.
*/
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 12/19] mtd: rawnand: Rename nand_init_data_interface()
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (10 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 11/19] mtd: rawnand: Fix nand_setup_data_interface() description Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:17 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 13/19] mtd: rawnand: timings: Update onfi_fill_data_interface() kernel doc Miquel Raynal
` (6 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
This name is a bit misleading, what we do in this helper is trying to
find the best timings supported by the controller and the chip. In
other words, we choose the data interface.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_base.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index c8f5d4557ed5..ac08d1fc710a 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1005,7 +1005,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
}
/**
- * nand_init_data_interface - find the best data interface and timings
+ * nand_choose_data_interface - find the best data interface and timings
* @chip: The NAND chip
*
* Find the best data interface and NAND timings supported by the chip
@@ -1017,7 +1017,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
*
* Returns 0 for success or negative error code otherwise.
*/
-static int nand_init_data_interface(struct nand_chip *chip)
+static int nand_choose_data_interface(struct nand_chip *chip)
{
int modes, mode, ret;
@@ -6045,8 +6045,8 @@ static int nand_scan_tail(struct nand_chip *chip)
if (!mtd->bitflip_threshold)
mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3, 4);
- /* Initialize the ->data_interface field. */
- ret = nand_init_data_interface(chip);
+ /* Find the fastest data interface for this chip */
+ ret = nand_choose_data_interface(chip);
if (ret)
goto err_nanddev_cleanup;
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 13/19] mtd: rawnand: timings: Update onfi_fill_data_interface() kernel doc
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (11 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 12/19] mtd: rawnand: Rename nand_init_data_interface() Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:18 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 14/19] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface Miquel Raynal
` (5 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
Describe all parameters and drop the legacy [NAND Interface] prefix.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_timings.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index 36d21be3dfe5..a73d934e86f9 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -274,9 +274,10 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
};
/**
- * onfi_fill_data_interface - [NAND Interface] Initialize a data interface from
- * given ONFI mode
- * @mode: The ONFI timing mode
+ * onfi_fill_data_interface - Initialize a data interface from a given ONFI mode
+ * @chip: The NAND chip
+ * @type: The data interface type
+ * @timing_mode: The ONFI timing mode
*/
int onfi_fill_data_interface(struct nand_chip *chip,
enum nand_data_interface_type type,
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 14/19] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (12 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 13/19] mtd: rawnand: timings: Update onfi_fill_data_interface() kernel doc Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:26 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 15/19] mtd: rawnand: timings: Add a helper to find the closest ONFI mode Miquel Raynal
` (4 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
We rely be default on the data interface which is in the nand_chip
structure but it should be possible to fill any other data interface.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/internals.h | 1 +
drivers/mtd/nand/raw/nand_base.c | 7 ++++---
drivers/mtd/nand/raw/nand_timings.c | 3 ++-
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index 347d42c55353..dc84e3b55d48 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -85,6 +85,7 @@ int nand_markbad_bbm(struct nand_chip *chip, loff_t ofs);
int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
int allowbbt);
int onfi_fill_data_interface(struct nand_chip *chip,
+ struct nand_data_interface *iface,
enum nand_data_interface_type type,
int timing_mode);
int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index ac08d1fc710a..776f2d119bad 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -927,7 +927,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
* timings to timing mode 0.
*/
- onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
+ onfi_fill_data_interface(chip, &chip->data_interface, NAND_SDR_IFACE, 0);
ret = chip->controller->ops->setup_data_interface(chip, chipnr,
&chip->data_interface);
if (ret)
@@ -1039,7 +1039,8 @@ static int nand_choose_data_interface(struct nand_chip *chip)
}
for (mode = fls(modes) - 1; mode >= 0; mode--) {
- ret = onfi_fill_data_interface(chip, NAND_SDR_IFACE, mode);
+ ret = onfi_fill_data_interface(chip, &chip->data_interface,
+ NAND_SDR_IFACE, mode);
if (ret)
continue;
@@ -5248,7 +5249,7 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
mutex_init(&chip->lock);
/* Enforce the right timings for reset/detection */
- onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
+ onfi_fill_data_interface(chip, &chip->data_interface, NAND_SDR_IFACE, 0);
ret = nand_dt_init(chip);
if (ret)
diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index a73d934e86f9..ce6bb87db2e8 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -276,14 +276,15 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
/**
* onfi_fill_data_interface - Initialize a data interface from a given ONFI mode
* @chip: The NAND chip
+ * @iface: The data interface to fill
* @type: The data interface type
* @timing_mode: The ONFI timing mode
*/
int onfi_fill_data_interface(struct nand_chip *chip,
+ struct nand_data_interface *iface,
enum nand_data_interface_type type,
int timing_mode)
{
- struct nand_data_interface *iface = &chip->data_interface;
struct onfi_params *onfi = chip->parameters.onfi;
if (type != NAND_SDR_IFACE)
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 15/19] mtd: rawnand: timings: Add a helper to find the closest ONFI mode
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (13 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 14/19] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:30 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 16/19] mtd: rawnand: Introduce nand_choose_best_sdr_iface() Miquel Raynal
` (3 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
Vendors are allowed to provide their own set of timings. In this case,
we provide a way to derive the "closest" timing mode so that, if the
NAND controller does not support tweaking these parameters, it will be
able to configure itself anyway.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/internals.h | 1 +
drivers/mtd/nand/raw/nand_timings.c | 52 +++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index dc84e3b55d48..ac103d8767be 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -88,6 +88,7 @@ int onfi_fill_data_interface(struct nand_chip *chip,
struct nand_data_interface *iface,
enum nand_data_interface_type type,
int timing_mode);
+unsigned int onfi_find_equivalent_sdr_mode(const struct nand_sdr_timings *vendor_timings);
int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
int nand_set_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
int nand_read_page_raw_notsupp(struct nand_chip *chip, u8 *buf,
diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
index ce6bb87db2e8..7f5caa325fbe 100644
--- a/drivers/mtd/nand/raw/nand_timings.c
+++ b/drivers/mtd/nand/raw/nand_timings.c
@@ -273,6 +273,58 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
},
};
+unsigned int onfi_find_equivalent_sdr_mode(const struct nand_sdr_timings *vendor_timings)
+{
+ const struct nand_sdr_timings *onfi_timings;
+ int mode;
+
+ for (mode = ARRAY_SIZE(onfi_sdr_timings) - 1; mode > 0; mode--) {
+ onfi_timings = &onfi_sdr_timings[mode].timings.sdr;
+
+ if (vendor_timings->tCCS_min > onfi_timings->tCCS_min ||
+ vendor_timings->tR_max < onfi_timings->tR_max ||
+ vendor_timings->tADL_min > onfi_timings->tADL_min ||
+ vendor_timings->tALH_min > onfi_timings->tALH_min ||
+ vendor_timings->tALS_min > onfi_timings->tALS_min ||
+ vendor_timings->tAR_min > onfi_timings->tAR_min ||
+ vendor_timings->tCEA_max < onfi_timings->tCEA_max ||
+ vendor_timings->tCEH_min > onfi_timings->tCEH_min ||
+ vendor_timings->tCH_min > onfi_timings->tCH_min ||
+ vendor_timings->tCHZ_max < onfi_timings->tCHZ_max ||
+ vendor_timings->tCLH_min > onfi_timings->tCLH_min ||
+ vendor_timings->tCLR_min > onfi_timings->tCLR_min ||
+ vendor_timings->tCLS_min > onfi_timings->tCLS_min ||
+ vendor_timings->tCOH_min > onfi_timings->tCOH_min ||
+ vendor_timings->tCS_min > onfi_timings->tCS_min ||
+ vendor_timings->tDH_min > onfi_timings->tDH_min ||
+ vendor_timings->tDS_min > onfi_timings->tDS_min ||
+ vendor_timings->tFEAT_max < onfi_timings->tFEAT_max ||
+ vendor_timings->tIR_min > onfi_timings->tIR_min ||
+ vendor_timings->tITC_max < onfi_timings->tITC_max ||
+ vendor_timings->tRC_min > onfi_timings->tRC_min ||
+ vendor_timings->tREA_max < onfi_timings->tREA_max ||
+ vendor_timings->tREH_min > onfi_timings->tREH_min ||
+ vendor_timings->tRHOH_min > onfi_timings->tRHOH_min ||
+ vendor_timings->tRHW_min > onfi_timings->tRHW_min ||
+ vendor_timings->tRHZ_max < onfi_timings->tRHZ_max ||
+ vendor_timings->tRLOH_min > onfi_timings->tRLOH_min ||
+ vendor_timings->tRP_min > onfi_timings->tRP_min ||
+ vendor_timings->tRR_min > onfi_timings->tRR_min ||
+ vendor_timings->tRST_max < onfi_timings->tRST_max ||
+ vendor_timings->tWB_max < onfi_timings->tWB_max ||
+ vendor_timings->tWC_min > onfi_timings->tWC_min ||
+ vendor_timings->tWH_min > onfi_timings->tWH_min ||
+ vendor_timings->tWHR_min > onfi_timings->tWHR_min ||
+ vendor_timings->tWP_min > onfi_timings->tWP_min ||
+ vendor_timings->tWW_min > onfi_timings->tWW_min)
+ continue;
+
+ return mode;
+ }
+
+ return 0;
+}
+
/**
* onfi_fill_data_interface - Initialize a data interface from a given ONFI mode
* @chip: The NAND chip
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 16/19] mtd: rawnand: Introduce nand_choose_best_sdr_iface()
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (14 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 15/19] mtd: rawnand: timings: Add a helper to find the closest ONFI mode Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:47 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface() Miquel Raynal
` (2 subsequent siblings)
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
Add a new helper: given a data interface with a specific set of
timings, check with the controller if this interface can be
supported. If not, fallback to the closest slower ONFI mode.
Extract this logic from nand_choose_data_interface() and use the new
helper instead, so that this code can be reused later.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_base.c | 74 ++++++++++++++++++++------------
1 file changed, 46 insertions(+), 28 deletions(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 776f2d119bad..15e10f045c9f 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1004,6 +1004,42 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
return ret;
}
+/**
+ * nand_choose_best_sdr_iface - given a data interface, find the closest
+ * mode/timings set for this interface supported
+ * by both the NAND controller and the NAND chip
+ * @chip: the NAND chip
+ * @best_iface: the best data interface (can eventually be updated)
+ */
+static int nand_choose_best_sdr_iface(struct nand_chip *chip,
+ struct nand_data_interface *best_iface)
+{
+ const struct nand_controller_ops *ops = chip->controller->ops;
+ int mode, ret;
+
+ /* Verify the controller supports the requested interface */
+ ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
+ best_iface);
+ if (!ret)
+ return ret;
+
+ /* Fallback to slower modes */
+ for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
+ ret = onfi_fill_data_interface(chip, best_iface,
+ NAND_SDR_IFACE, mode);
+ if (ret)
+ continue;
+
+ ret = ops->setup_data_interface(chip,
+ NAND_DATA_IFACE_CHECK_ONLY,
+ best_iface);
+ if (!ret)
+ break;
+ }
+
+ return 0;
+}
+
/**
* nand_choose_data_interface - find the best data interface and timings
* @chip: The NAND chip
@@ -1019,7 +1055,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
*/
static int nand_choose_data_interface(struct nand_chip *chip)
{
- int modes, mode, ret;
+ int best_mode, ret;
if (!nand_controller_has_setup_data_iface(chip))
return 0;
@@ -1029,35 +1065,17 @@ static int nand_choose_data_interface(struct nand_chip *chip)
* if the NAND does not support ONFI, fallback to the default ONFI
* timing mode.
*/
- if (chip->parameters.onfi) {
- modes = chip->parameters.onfi->async_timing_mode;
- } else {
- if (!chip->default_timing_mode)
- return 0;
+ if (chip->parameters.onfi)
+ best_mode = fls(chip->parameters.onfi->async_timing_mode) - 1;
+ else
+ best_mode = chip->default_timing_mode;
- modes = GENMASK(chip->default_timing_mode, 0);
- }
+ ret = onfi_fill_data_interface(chip, &chip->data_interface,
+ NAND_SDR_IFACE, best_mode);
+ if (ret)
+ return ret;
- for (mode = fls(modes) - 1; mode >= 0; mode--) {
- ret = onfi_fill_data_interface(chip, &chip->data_interface,
- NAND_SDR_IFACE, mode);
- if (ret)
- continue;
-
- /*
- * Pass NAND_DATA_IFACE_CHECK_ONLY to only check if the
- * controller supports the requested timings.
- */
- ret = chip->controller->ops->setup_data_interface(chip,
- NAND_DATA_IFACE_CHECK_ONLY,
- &chip->data_interface);
- if (!ret) {
- chip->default_timing_mode = mode;
- break;
- }
- }
-
- return 0;
+ return nand_choose_best_sdr_iface(chip, &chip->data_interface);
}
/**
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (15 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 16/19] mtd: rawnand: Introduce nand_choose_best_sdr_iface() Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:45 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook Miquel Raynal
2020-05-25 17:42 ` [PATCH v4 19/19] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Miquel Raynal
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
This helper is here to simplify the life of NAND manufacturer drivers.
Manufacturers will be allowed to propose their own set of timings and,
if they want, use this helper to:
1/ verify it is supported by the controller,
2/ fallback on a supported default ONFI mode, slower but still faster
than the default mode 0.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/internals.h | 2 ++
drivers/mtd/nand/raw/nand_base.c | 27 +++++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index ac103d8767be..9af6979257e2 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -89,6 +89,8 @@ int onfi_fill_data_interface(struct nand_chip *chip,
enum nand_data_interface_type type,
int timing_mode);
unsigned int onfi_find_equivalent_sdr_mode(const struct nand_sdr_timings *vendor_timings);
+int nand_choose_best_vendor_sdr_iface(struct nand_chip * chip,
+ struct nand_data_interface *best_iface);
int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
int nand_set_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
int nand_read_page_raw_notsupp(struct nand_chip *chip, u8 *buf,
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 15e10f045c9f..d9fe7795f183 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1078,6 +1078,33 @@ static int nand_choose_data_interface(struct nand_chip *chip)
return nand_choose_best_sdr_iface(chip, &chip->data_interface);
}
+/**
+ * nand_choose_best_vendor_sdr_iface - given a set of timings, find the closest
+ * mode/timings set for this interface
+ * supported by both the NAND controller and
+ * the NAND chip
+ * @chip: the NAND chip
+ * @best_iface: the best data interface (can eventually be updated)
+ */
+int nand_choose_best_vendor_sdr_iface(struct nand_chip * chip,
+ struct nand_data_interface *best_iface)
+{
+ int ret;
+
+ /* Pick the closest mode */
+ best_iface->timings.mode = onfi_find_equivalent_sdr_mode(&best_iface->timings.sdr);
+
+ /* Find the closest supported data interface */
+ ret = nand_choose_best_sdr_iface(chip, best_iface);
+ if (ret)
+ return ret;
+
+ chip->data_interface = *best_iface;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nand_choose_best_vendor_sdr_iface);
+
/**
* nand_fill_column_cycles - fill the column cycles of an address
* @chip: The NAND chip
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (16 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface() Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:51 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 19/19] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Miquel Raynal
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
This hook can be overloaded by NAND manufacturer drivers to propose
alternative timings when not following the main standards.
Vendors implementing this hook should:
1- choose the best timings and fill the data interface,
2- verify that the controller supports them.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/internals.h | 5 +++++
drivers/mtd/nand/raw/nand_base.c | 8 ++++++++
include/linux/mtd/rawnand.h | 2 ++
3 files changed, 15 insertions(+)
diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
index 9af6979257e2..61edbab35068 100644
--- a/drivers/mtd/nand/raw/internals.h
+++ b/drivers/mtd/nand/raw/internals.h
@@ -146,6 +146,11 @@ static inline bool nand_controller_has_setup_data_iface(struct nand_chip *chip)
return true;
}
+static inline bool nand_can_choose_data_interface(struct nand_chip *chip)
+{
+ return chip->ops.choose_data_interface;
+}
+
/* BBT functions */
int nand_markbad_bbt(struct nand_chip *chip, loff_t offs);
int nand_isreserved_bbt(struct nand_chip *chip, loff_t offs);
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d9fe7795f183..e9df339849d3 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -1060,6 +1060,14 @@ static int nand_choose_data_interface(struct nand_chip *chip)
if (!nand_controller_has_setup_data_iface(chip))
return 0;
+ /*
+ * Let the NAND vendor hook identify the best timings.
+ * ->choose_data_interface() is expected to update the entire chip's
+ * nand_data_interface structure.
+ */
+ if (nand_can_choose_data_interface(chip))
+ return chip->ops.choose_data_interface(chip);
+
/*
* First try to identify the best timings from ONFI parameters and
* if the NAND does not support ONFI, fallback to the default ONFI
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index a4b68e7b246a..5b8b94521a18 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -1034,6 +1034,7 @@ struct nand_legacy {
* @lock_area: Lock operation
* @unlock_area: Unlock operation
* @setup_read_retry: Set the read-retry mode (mostly needed for MLC NANDs)
+ * @choose_data_interface: Choose the best data interface
*/
struct nand_chip_ops {
int (*suspend)(struct nand_chip *chip);
@@ -1041,6 +1042,7 @@ struct nand_chip_ops {
int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
+ int (*choose_data_interface)(struct nand_chip *chip);
};
/**
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 19/19] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
` (17 preceding siblings ...)
2020-05-25 17:42 ` [PATCH v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook Miquel Raynal
@ 2020-05-25 17:42 ` Miquel Raynal
2020-05-25 19:53 ` Boris Brezillon
18 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-25 17:42 UTC (permalink / raw)
To: Richard Weinberger, Vignesh Raghavendra, Tudor Ambarus, linux-mtd
Cc: Rickard Andersson, Boris Brezillon, Miquel Raynal
From: Rickard x Andersson <rickaran@axis.com>
The Kioxia/Toshiba TH58NVG2S3HBAI4 NAND memory is not ONFI compliant.
The timings of the NAND chip memory are quite close to ONFI mode 4 but
is breaking that spec.
By providing our own set of timings, erase block read speed is increased
from 6910 kiB/s to 13490 kiB/s and erase block write speed is increased
from 3350 kiB/s to 4410 kiB/s.
Tested on IMX6SX which has a NAND controller supporting EDO mode.
Signed-off-by: Rickard x Andersson <rickaran@axis.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/mtd/nand/raw/nand_ids.c | 3 +++
drivers/mtd/nand/raw/nand_toshiba.c | 38 +++++++++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
index e0dbc2e316c7..8b676e8b481b 100644
--- a/drivers/mtd/nand/raw/nand_ids.c
+++ b/drivers/mtd/nand/raw/nand_ids.c
@@ -52,6 +52,9 @@ struct nand_flash_dev nand_flash_ids[] = {
{ .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
SZ_8K, SZ_8K, SZ_2M, NAND_NEED_SCRAMBLING, 6, 640,
NAND_ECC_INFO(40, SZ_1K), 4 },
+ {"TH58NVG2S3HBAI4 4G 3.3V 8-bit",
+ { .id = {0x98, 0xdc, 0x91, 0x15, 0x76} },
+ SZ_2K, SZ_512, SZ_128K, 0, 5, 128, NAND_ECC_INFO(8, SZ_512) },
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),
diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
index b6efaf5195bb..6c79464fdf34 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -205,6 +205,42 @@ static int tc58teg5dclta00_init(struct nand_chip *chip)
return 0;
}
+static int th58nvg2s3hbai4_choose_data_interface(struct nand_chip *chip)
+{
+ struct nand_data_interface iface;
+ struct nand_sdr_timings *sdr = &iface.timings.sdr;
+ int ret;
+
+ /* Start with timings from the closest timing mode, mode 4. */
+ ret = onfi_fill_data_interface(chip, &iface, NAND_SDR_IFACE, 4);
+ if (ret)
+ return ret;
+
+ /* Patch timings that differ from mode 4. */
+ sdr->tALS_min = 12000;
+ sdr->tCHZ_max = 20000;
+ sdr->tCLS_min = 12000;
+ sdr->tCOH_min = 0;
+ sdr->tDS_min = 12000;
+ sdr->tRHOH_min = 25000;
+ sdr->tRHW_min = 30000;
+ sdr->tRHZ_max = 60000;
+ sdr->tWHR_min = 60000;
+
+ /* Patch timings not part of onfi timing mode. */
+ sdr->tPROG_max = 700000000;
+ sdr->tBERS_max = 5000000000;
+
+ return nand_choose_best_vendor_sdr_iface(chip, &iface);
+}
+
+static int th58nvg2s3hbai4_init(struct nand_chip *chip)
+{
+ chip->ops.choose_data_interface = th58nvg2s3hbai4_choose_data_interface;
+
+ return 0;
+}
+
static int toshiba_nand_init(struct nand_chip *chip)
{
if (nand_is_slc(chip))
@@ -217,6 +253,8 @@ static int toshiba_nand_init(struct nand_chip *chip)
if (!strcmp("TC58TEG5DCLTA00", chip->parameters.model))
tc58teg5dclta00_init(chip);
+ if (!strncmp("TH58NVG2S3HBAI4", chip->parameters.model, 15))
+ th58nvg2s3hbai4_init(chip);
return 0;
}
--
2.20.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v4 01/19] mtd: rawnand: Use unsigned types for nand_chip unsigned values
2020-05-25 17:42 ` [PATCH v4 01/19] mtd: rawnand: Use unsigned types for nand_chip unsigned values Miquel Raynal
@ 2020-05-25 18:33 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 18:33 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:21 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> page_shift, phys_erase_shift, bbt_erase_shift, chip_shift, pagemask,
> subpagesize and badblockbits are all positive values, so declare
> them as unsigned.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
though patching code to get rid of those fields (in favor of
nanddev_xxx() calls) would be much more valuable IMHO.
> ---
> include/linux/mtd/rawnand.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 2804c13e5662..8a1e0192f78e 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1110,11 +1110,11 @@ struct nand_chip {
> unsigned int options;
> unsigned int bbt_options;
>
> - int page_shift;
> - int phys_erase_shift;
> - int bbt_erase_shift;
> - int chip_shift;
> - int pagemask;
> + unsigned int page_shift;
> + unsigned int phys_erase_shift;
> + unsigned int bbt_erase_shift;
> + unsigned int chip_shift;
> + unsigned int pagemask;
> u8 *data_buf;
>
> struct {
> @@ -1122,10 +1122,10 @@ struct nand_chip {
> int page;
> } pagecache;
>
> - int subpagesize;
> + unsigned int subpagesize;
> int onfi_timing_mode_default;
> unsigned int badblockpos;
> - int badblockbits;
> + unsigned int badblockbits;
>
> struct nand_id id;
> struct nand_parameters parameters;
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 02/19] mtd: rawnand: Only use u8 instead of uint8_t in nand_chip structure
2020-05-25 17:42 ` [PATCH v4 02/19] mtd: rawnand: Only use u8 instead of uint8_t in nand_chip structure Miquel Raynal
@ 2020-05-25 18:36 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 18:36 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:22 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Mechanical change to avoid using old types.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Same comment as for patch 1: I'd prefer to have the BBT code transition
to nanddev_bbt_xxx() and re-use the generic layer, but I guess that'll
have to wait.
> ---
> include/linux/mtd/rawnand.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 8a1e0192f78e..7d62e0e719ac 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1141,13 +1141,13 @@ struct nand_chip {
> int (*suspend)(struct nand_chip *chip);
> void (*resume)(struct nand_chip *chip);
>
> - uint8_t *oob_poi;
> + u8 *oob_poi;
> struct nand_controller *controller;
>
> struct nand_ecc_ctrl ecc;
> unsigned long buf_align;
>
> - uint8_t *bbt;
> + u8 *bbt;
> struct nand_bbt_descr *bbt_td;
> struct nand_bbt_descr *bbt_md;
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 03/19] mtd: rawnand: Create a nand_chip operations structure
2020-05-25 17:42 ` [PATCH v4 03/19] mtd: rawnand: Create a nand_chip operations structure Miquel Raynal
@ 2020-05-25 18:37 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 18:37 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:23 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> And move nand_chip hooks there.
>
> While moving entries from one structure to the other, adapt the
> documentation style.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 20 ++++++++---------
> drivers/mtd/nand/raw/nand_hynix.c | 2 +-
> drivers/mtd/nand/raw/nand_macronix.c | 10 ++++-----
> drivers/mtd/nand/raw/nand_micron.c | 2 +-
> include/linux/mtd/rawnand.h | 32 ++++++++++++++++------------
> 5 files changed, 35 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 6a6a0a36b3fd..b86f641f6d74 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -3285,10 +3285,10 @@ static int nand_setup_read_retry(struct nand_chip *chip, int retry_mode)
> if (retry_mode >= chip->read_retries)
> return -EINVAL;
>
> - if (!chip->setup_read_retry)
> + if (!chip->ops.setup_read_retry)
> return -EOPNOTSUPP;
>
> - return chip->setup_read_retry(chip, retry_mode);
> + return chip->ops.setup_read_retry(chip, retry_mode);
> }
>
> static void nand_wait_readrdy(struct nand_chip *chip)
> @@ -4532,8 +4532,8 @@ static int nand_suspend(struct mtd_info *mtd)
> int ret = 0;
>
> mutex_lock(&chip->lock);
> - if (chip->suspend)
> - ret = chip->suspend(chip);
> + if (chip->ops.suspend)
> + ret = chip->ops.suspend(chip);
> if (!ret)
> chip->suspended = 1;
> mutex_unlock(&chip->lock);
> @@ -4551,8 +4551,8 @@ static void nand_resume(struct mtd_info *mtd)
>
> mutex_lock(&chip->lock);
> if (chip->suspended) {
> - if (chip->resume)
> - chip->resume(chip);
> + if (chip->ops.resume)
> + chip->ops.resume(chip);
> chip->suspended = 0;
> } else {
> pr_err("%s called for a chip which is not in suspended state\n",
> @@ -4581,10 +4581,10 @@ static int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> {
> struct nand_chip *chip = mtd_to_nand(mtd);
>
> - if (!chip->lock_area)
> + if (!chip->ops.lock_area)
> return -ENOTSUPP;
>
> - return chip->lock_area(chip, ofs, len);
> + return chip->ops.lock_area(chip, ofs, len);
> }
>
> /**
> @@ -4597,10 +4597,10 @@ static int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> {
> struct nand_chip *chip = mtd_to_nand(mtd);
>
> - if (!chip->unlock_area)
> + if (!chip->ops.unlock_area)
> return -ENOTSUPP;
>
> - return chip->unlock_area(chip, ofs, len);
> + return chip->ops.unlock_area(chip, ofs, len);
> }
>
> /* Set default functions */
> diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
> index 7caedaa5b9e5..7d1be53f27f3 100644
> --- a/drivers/mtd/nand/raw/nand_hynix.c
> +++ b/drivers/mtd/nand/raw/nand_hynix.c
> @@ -337,7 +337,7 @@ static int hynix_mlc_1xnm_rr_init(struct nand_chip *chip,
> rr->nregs = nregs;
> rr->regs = hynix_1xnm_mlc_read_retry_regs;
> hynix->read_retry = rr;
> - chip->setup_read_retry = hynix_nand_setup_read_retry;
> + chip->ops.setup_read_retry = hynix_nand_setup_read_retry;
> chip->read_retries = nmodes;
>
> out:
> diff --git a/drivers/mtd/nand/raw/nand_macronix.c b/drivers/mtd/nand/raw/nand_macronix.c
> index 09c254c97b5c..1472f925f386 100644
> --- a/drivers/mtd/nand/raw/nand_macronix.c
> +++ b/drivers/mtd/nand/raw/nand_macronix.c
> @@ -130,7 +130,7 @@ static void macronix_nand_onfi_init(struct nand_chip *chip)
> return;
>
> chip->read_retries = MACRONIX_NUM_READ_RETRY_MODES;
> - chip->setup_read_retry = macronix_nand_setup_read_retry;
> + chip->ops.setup_read_retry = macronix_nand_setup_read_retry;
>
> if (p->supports_set_get_features) {
> bitmap_set(p->set_feature_list,
> @@ -242,8 +242,8 @@ static void macronix_nand_block_protection_support(struct nand_chip *chip)
> bitmap_set(chip->parameters.set_feature_list,
> ONFI_FEATURE_ADDR_MXIC_PROTECTION, 1);
>
> - chip->lock_area = mxic_nand_lock;
> - chip->unlock_area = mxic_nand_unlock;
> + chip->ops.lock_area = mxic_nand_lock;
> + chip->ops.unlock_area = mxic_nand_unlock;
> }
>
> static int nand_power_down_op(struct nand_chip *chip)
> @@ -312,8 +312,8 @@ static void macronix_nand_deep_power_down_support(struct nand_chip *chip)
> if (i < 0)
> return;
>
> - chip->suspend = mxic_nand_suspend;
> - chip->resume = mxic_nand_resume;
> + chip->ops.suspend = mxic_nand_suspend;
> + chip->ops.resume = mxic_nand_resume;
> }
>
> static int macronix_nand_init(struct nand_chip *chip)
> diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
> index 3589b4fce0d4..4385092a9325 100644
> --- a/drivers/mtd/nand/raw/nand_micron.c
> +++ b/drivers/mtd/nand/raw/nand_micron.c
> @@ -84,7 +84,7 @@ static int micron_nand_onfi_init(struct nand_chip *chip)
> struct nand_onfi_vendor_micron *micron = (void *)p->onfi->vendor;
>
> chip->read_retries = micron->read_retry_options;
> - chip->setup_read_retry = micron_nand_setup_read_retry;
> + chip->ops.setup_read_retry = micron_nand_setup_read_retry;
> }
>
> if (p->supports_set_get_features) {
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 7d62e0e719ac..b33cd68852c4 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1027,16 +1027,31 @@ struct nand_legacy {
> struct nand_controller dummy_controller;
> };
>
> +/**
> + * struct nand_chip_ops - NAND chip operations
> + * @suspend: Suspend operation
> + * @resume: Resume operation
> + * @lock_area: Lock operation
> + * @unlock_area: Unlock operation
> + * @setup_read_retry: Set the read-retry mode (mostly needed for MLC NANDs)
> + */
> +struct nand_chip_ops {
> + int (*suspend)(struct nand_chip *chip);
> + void (*resume)(struct nand_chip *chip);
> + int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> + int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> + int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
> +};
> +
> /**
> * struct nand_chip - NAND Private Flash Chip Data
> * @base: Inherit from the generic NAND device
> + * @ops: NAND chip operations
> * @legacy: All legacy fields/hooks. If you develop a new driver,
> * don't even try to use any of these fields/hooks, and if
> * you're modifying an existing driver that is using those
> * fields/hooks, you should consider reworking the driver
> * avoid using them.
> - * @setup_read_retry: [FLASHSPECIFIC] flash (vendor) specific function for
> - * setting the read-retry mode. Mostly needed for MLC NAND.
> * @ecc: [BOARDSPECIFIC] ECC control structure
> * @buf_align: minimum buffer alignment required by a platform
> * @oob_poi: "poison value buffer," used for laying out OOB data
> @@ -1081,8 +1096,6 @@ struct nand_legacy {
> * @lock: lock protecting the suspended field. Also used to
> * serialize accesses to the NAND device.
> * @suspended: set to 1 when the device is suspended, 0 when it's not.
> - * @suspend: [REPLACEABLE] specific NAND device suspend operation
> - * @resume: [REPLACEABLE] specific NAND device resume operation
> * @bbt: [INTERN] bad block table pointer
> * @bbt_td: [REPLACEABLE] bad block table descriptor for flash
> * lookup.
> @@ -1096,17 +1109,13 @@ struct nand_legacy {
> * @manufacturer: [INTERN] Contains manufacturer information
> * @manufacturer.desc: [INTERN] Contains manufacturer's description
> * @manufacturer.priv: [INTERN] Contains manufacturer private information
> - * @lock_area: [REPLACEABLE] specific NAND chip lock operation
> - * @unlock_area: [REPLACEABLE] specific NAND chip unlock operation
> */
>
> struct nand_chip {
> struct nand_device base;
> -
> + struct nand_chip_ops ops;
> struct nand_legacy legacy;
>
> - int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
> -
> unsigned int options;
> unsigned int bbt_options;
>
> @@ -1138,8 +1147,6 @@ struct nand_chip {
>
> struct mutex lock;
> unsigned int suspended : 1;
> - int (*suspend)(struct nand_chip *chip);
> - void (*resume)(struct nand_chip *chip);
>
> u8 *oob_poi;
> struct nand_controller *controller;
> @@ -1159,9 +1166,6 @@ struct nand_chip {
> const struct nand_manufacturer *desc;
> void *priv;
> } manufacturer;
> -
> - int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> - int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> };
>
> extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 04/19] mtd: rawnand: Rename the manufacturer structure
2020-05-25 17:42 ` [PATCH v4 04/19] mtd: rawnand: Rename the manufacturer structure Miquel Raynal
@ 2020-05-25 18:38 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 18:38 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:24 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> It is currently called nand_manufacturer but could actually be called
> nand_manufacturer_desc, like its instances, so that the former name is
> left unused for now.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/internals.h | 6 +++---
> drivers/mtd/nand/raw/nand_base.c | 14 +++++++-------
> drivers/mtd/nand/raw/nand_ids.c | 16 ++++++++--------
> include/linux/mtd/rawnand.h | 2 +-
> 4 files changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 03866b0aadea..a518acfd9b3f 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -53,12 +53,12 @@ struct nand_manufacturer_ops {
> };
>
> /**
> - * struct nand_manufacturer - NAND Flash Manufacturer structure
> + * struct nand_manufacturer_desc - NAND Flash Manufacturer descriptor
> * @name: Manufacturer name
> * @id: manufacturer ID code of device.
> * @ops: manufacturer operations
> */
> -struct nand_manufacturer {
> +struct nand_manufacturer_desc {
> int id;
> char *name;
> const struct nand_manufacturer_ops *ops;
> @@ -79,7 +79,7 @@ extern const struct nand_manufacturer_ops toshiba_nand_manuf_ops;
> extern const struct mtd_pairing_scheme dist3_pairing_scheme;
>
> /* Core functions */
> -const struct nand_manufacturer *nand_get_manufacturer(u8 id);
> +const struct nand_manufacturer_desc *nand_get_manufacturer_desc(u8 id);
> int nand_bbm_get_next_page(struct nand_chip *chip, int page);
> int nand_markbad_bbm(struct nand_chip *chip, loff_t ofs);
> int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index b86f641f6d74..14f1359a60b8 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4880,9 +4880,9 @@ static void nand_manufacturer_cleanup(struct nand_chip *chip)
> }
>
> static const char *
> -nand_manufacturer_name(const struct nand_manufacturer *manufacturer)
> +nand_manufacturer_name(const struct nand_manufacturer_desc *manufacturer_desc)
> {
> - return manufacturer ? manufacturer->name : "Unknown";
> + return manufacturer_desc ? manufacturer_desc->name : "Unknown";
> }
>
> /*
> @@ -4890,7 +4890,7 @@ nand_manufacturer_name(const struct nand_manufacturer *manufacturer)
> */
> static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> {
> - const struct nand_manufacturer *manufacturer;
> + const struct nand_manufacturer_desc *manufacturer_desc;
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct nand_memory_organization *memorg;
> int busw, ret;
> @@ -4947,8 +4947,8 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> chip->id.len = nand_id_len(id_data, ARRAY_SIZE(chip->id.data));
>
> /* Try to identify manufacturer */
> - manufacturer = nand_get_manufacturer(maf_id);
> - chip->manufacturer.desc = manufacturer;
> + manufacturer_desc = nand_get_manufacturer_desc(maf_id);
> + chip->manufacturer.desc = manufacturer_desc;
>
> if (!type)
> type = nand_flash_ids;
> @@ -5027,7 +5027,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
> */
> pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
> maf_id, dev_id);
> - pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
> + pr_info("%s %s\n", nand_manufacturer_name(manufacturer_desc),
> mtd->name);
> pr_warn("bus width %d instead of %d bits\n", busw ? 16 : 8,
> (chip->options & NAND_BUSWIDTH_16) ? 16 : 8);
> @@ -5062,7 +5062,7 @@ static int nand_detect(struct nand_chip *chip, struct nand_flash_dev *type)
>
> pr_info("device found, Manufacturer ID: 0x%02x, Chip ID: 0x%02x\n",
> maf_id, dev_id);
> - pr_info("%s %s\n", nand_manufacturer_name(manufacturer),
> + pr_info("%s %s\n", nand_manufacturer_name(manufacturer_desc),
> chip->parameters.model);
> pr_info("%d MiB, %s, erase size: %d KiB, page size: %d, OOB size: %d\n",
> (int)(targetsize >> 20), nand_is_slc(chip) ? "SLC" : "MLC",
> diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
> index ba27902fc54b..e0dbc2e316c7 100644
> --- a/drivers/mtd/nand/raw/nand_ids.c
> +++ b/drivers/mtd/nand/raw/nand_ids.c
> @@ -166,7 +166,7 @@ struct nand_flash_dev nand_flash_ids[] = {
> };
>
> /* Manufacturer IDs */
> -static const struct nand_manufacturer nand_manufacturers[] = {
> +static const struct nand_manufacturer_desc nand_manufacturer_descs[] = {
> {NAND_MFR_AMD, "AMD/Spansion", &amd_nand_manuf_ops},
> {NAND_MFR_ATO, "ATO"},
> {NAND_MFR_EON, "Eon"},
> @@ -186,20 +186,20 @@ static const struct nand_manufacturer nand_manufacturers[] = {
> };
>
> /**
> - * nand_get_manufacturer - Get manufacturer information from the manufacturer
> - * ID
> + * nand_get_manufacturer_desc - Get manufacturer information from the
> + * manufacturer ID
> * @id: manufacturer ID
> *
> - * Returns a pointer a nand_manufacturer object if the manufacturer is defined
> + * Returns a nand_manufacturer_desc object if the manufacturer is defined
> * in the NAND manufacturers database, NULL otherwise.
> */
> -const struct nand_manufacturer *nand_get_manufacturer(u8 id)
> +const struct nand_manufacturer_desc *nand_get_manufacturer_desc(u8 id)
> {
> int i;
>
> - for (i = 0; i < ARRAY_SIZE(nand_manufacturers); i++)
> - if (nand_manufacturers[i].id == id)
> - return &nand_manufacturers[i];
> + for (i = 0; i < ARRAY_SIZE(nand_manufacturer_descs); i++)
> + if (nand_manufacturer_descs[i].id == id)
> + return &nand_manufacturer_descs[i];
>
> return NULL;
> }
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index b33cd68852c4..d8492d966b40 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1163,7 +1163,7 @@ struct nand_chip {
> void *priv;
>
> struct {
> - const struct nand_manufacturer *desc;
> + const struct nand_manufacturer_desc *desc;
> void *priv;
> } manufacturer;
> };
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 05/19] mtd: rawnand: Declare the nand_manufacturer structure out of nand_chip
2020-05-25 17:42 ` [PATCH v4 05/19] mtd: rawnand: Declare the nand_manufacturer structure out of nand_chip Miquel Raynal
@ 2020-05-25 18:40 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 18:40 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:25 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Now that struct nand_manufacturer type is free, use it to store the
> nand_manufacturer_desc and the manufacturer's private data.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
I'm honestly not convinced this brings any value, but it's also
harmless, so
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> include/linux/mtd/rawnand.h | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index d8492d966b40..2a9b5d5b315b 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1043,10 +1043,21 @@ struct nand_chip_ops {
> int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
> };
>
> +/**
> + * struct nand_manufacturer - NAND manufacturer structure
> + * @desc: The manufacturer description
> + * @priv: Private information for the manufacturer driver
> + */
> +struct nand_manufacturer {
> + const struct nand_manufacturer_desc *desc;
> + void *priv;
> +};
> +
> /**
> * struct nand_chip - NAND Private Flash Chip Data
> * @base: Inherit from the generic NAND device
> * @ops: NAND chip operations
> + * @manufacturer: Manufacturer information
> * @legacy: All legacy fields/hooks. If you develop a new driver,
> * don't even try to use any of these fields/hooks, and if
> * you're modifying an existing driver that is using those
> @@ -1106,13 +1117,11 @@ struct nand_chip_ops {
> * structure which is shared among multiple independent
> * devices.
> * @priv: [OPTIONAL] pointer to private chip data
> - * @manufacturer: [INTERN] Contains manufacturer information
> - * @manufacturer.desc: [INTERN] Contains manufacturer's description
> - * @manufacturer.priv: [INTERN] Contains manufacturer private information
> */
>
> struct nand_chip {
> struct nand_device base;
> + struct nand_manufacturer manufacturer;
> struct nand_chip_ops ops;
> struct nand_legacy legacy;
>
> @@ -1161,11 +1170,6 @@ struct nand_chip {
> struct nand_bbt_descr *badblock_pattern;
>
> void *priv;
> -
> - struct {
> - const struct nand_manufacturer_desc *desc;
> - void *priv;
> - } manufacturer;
> };
>
> extern const struct mtd_ooblayout_ops nand_ooblayout_sp_ops;
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 06/19] mtd: rawnand: Reorganize the nand_chip structure
2020-05-25 17:42 ` [PATCH v4 06/19] mtd: rawnand: Reorganize the nand_chip structure Miquel Raynal
@ 2020-05-25 18:55 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 18:55 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:26 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Reorder fields in this structure and pack entries by theme:
> * The main descriptive structures
> * The data interface details
> * Bad block information
> * The device layout
> * Extra buffers matching the device layout
> * Internal values
> * External objects like the ECC controller, the ECC engine and a
> private data pointer.
>
> While at it, adapt the documentation style.
>
> I changed on purpose the description of @oob_poi which was weird.
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Just like patch 1 and 2, I'm not opposed to this patch, but I feel we're
spending time on something that's expected to be removed soon.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> include/linux/mtd/rawnand.h | 166 +++++++++++++++++-------------------
> 1 file changed, 76 insertions(+), 90 deletions(-)
>
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 2a9b5d5b315b..622da6527a36 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1055,120 +1055,106 @@ struct nand_manufacturer {
>
> /**
> * struct nand_chip - NAND Private Flash Chip Data
> - * @base: Inherit from the generic NAND device
> - * @ops: NAND chip operations
> - * @manufacturer: Manufacturer information
> - * @legacy: All legacy fields/hooks. If you develop a new driver,
> - * don't even try to use any of these fields/hooks, and if
> - * you're modifying an existing driver that is using those
> - * fields/hooks, you should consider reworking the driver
> - * avoid using them.
> - * @ecc: [BOARDSPECIFIC] ECC control structure
> - * @buf_align: minimum buffer alignment required by a platform
> - * @oob_poi: "poison value buffer," used for laying out OOB data
> - * before writing
> - * @page_shift: [INTERN] number of address bits in a page (column
> - * address bits).
> - * @phys_erase_shift: [INTERN] number of address bits in a physical eraseblock
> - * @bbt_erase_shift: [INTERN] number of address bits in a bbt entry
> - * @chip_shift: [INTERN] number of address bits in one chip
> - * @options: [BOARDSPECIFIC] various chip options. They can partly
> - * be set to inform nand_scan about special functionality.
> - * See the defines for further explanation.
> - * @bbt_options: [INTERN] bad block specific options. All options used
> - * here must come from bbm.h. By default, these options
> - * will be copied to the appropriate nand_bbt_descr's.
> - * @badblockpos: [INTERN] position of the bad block marker in the oob
> - * area.
> - * @badblockbits: [INTERN] minimum number of set bits in a good block's
> - * bad block marker position; i.e., BBM == 11110111b is
> - * not bad when badblockbits == 7
> - * @onfi_timing_mode_default: [INTERN] default ONFI timing mode. This field is
> - * set to the actually used ONFI mode if the chip is
> - * ONFI compliant or deduced from the datasheet if
> - * the NAND chip is not ONFI compliant.
> - * @pagemask: [INTERN] page number mask = number of (pages / chip) - 1
> - * @data_buf: [INTERN] buffer for data, size is (page size + oobsize).
> - * @pagecache: Structure containing page cache related fields
> - * @pagecache.bitflips: Number of bitflips of the cached page
> - * @pagecache.page: Page number currently in the cache. -1 means no page is
> - * currently cached
> - * @subpagesize: [INTERN] holds the subpagesize
> - * @id: [INTERN] holds NAND ID
> - * @parameters: [INTERN] holds generic parameters under an easily
> - * readable form.
> - * @data_interface: [INTERN] NAND interface timing information
> - * @cur_cs: currently selected target. -1 means no target selected,
> - * otherwise we should always have cur_cs >= 0 &&
> - * cur_cs < nanddev_ntargets(). NAND Controller drivers
> - * should not modify this value, but they're allowed to
> - * read it.
> - * @read_retries: [INTERN] the number of read retry modes supported
> - * @lock: lock protecting the suspended field. Also used to
> - * serialize accesses to the NAND device.
> - * @suspended: set to 1 when the device is suspended, 0 when it's not.
> - * @bbt: [INTERN] bad block table pointer
> - * @bbt_td: [REPLACEABLE] bad block table descriptor for flash
> - * lookup.
> - * @bbt_md: [REPLACEABLE] bad block table mirror descriptor
> - * @badblock_pattern: [REPLACEABLE] bad block scan pattern used for initial
> - * bad block scan.
> - * @controller: [REPLACEABLE] a pointer to a hardware controller
> - * structure which is shared among multiple independent
> - * devices.
> - * @priv: [OPTIONAL] pointer to private chip data
> + * @base: Inherit from the generic NAND device
> + * @id: Holds NAND ID
> + * @parameters: Holds generic parameters under an easily readable form
> + * @manufacturer: Manufacturer information
> + * @ops: NAND chip operations
> + * @legacy: All legacy fields/hooks. If you develop a new driver, don't even try
> + * to use any of these fields/hooks, and if you're modifying an
> + * existing driver that is using those fields/hooks, you should
> + * consider reworking the driver and avoid using them.
> + * @options: Various chip options. They can partly be set to inform nand_scan
> + * about special functionality. See the defines for further
> + * explanation.
> + * @onfi_timing_mode_default: Default ONFI timing mode. This field is set to the
> + * actually used ONFI mode if the chip is ONFI
> + * compliant or deduced from the datasheet otherwise
> + * @data_interface: NAND interface timing information
> + * @bbt_erase_shift: Number of address bits in a bbt entry
> + * @bbt_options: Bad block table specific options. All options used here must
> + * come from bbm.h. By default, these options will be copied to
> + * the appropriate nand_bbt_descr's.
> + * @badblockpos: Bad block marker position in the oob area
> + * @badblockbits: Minimum number of set bits in a good block's bad block marker
> + * position; i.e., BBM = 11110111b is good when badblockbits = 7
> + * @bbt_td: Bad block table descriptor for flash lookup
> + * @bbt_md: Bad block table mirror descriptor
> + * @badblock_pattern: Bad block scan pattern used for initial bad block scan
> + * @bbt: Bad block table pointer
> + * @page_shift: Number of address bits in a page (column address bits)
> + * @phys_erase_shift: Number of address bits in a physical eraseblock
> + * @chip_shift: Number of address bits in one chip
> + * @pagemask: Page number mask = number of (pages / chip) - 1
> + * @subpagesize: Holds the subpagesize
> + * @data_buf: Buffer for data, size is (page size + oobsize)
> + * @oob_poi: pointer on the OOB area covered by data_buf
> + * @pagecache: Structure containing page cache related fields
> + * @pagecache.bitflips: Number of bitflips of the cached page
> + * @pagecache.page: Page number currently in the cache. -1 means no page is
> + * currently cached
> + * @buf_align: Minimum buffer alignment required by a platform
> + * @lock: Lock protecting the suspended field. Also used to serialize accesses
> + * to the NAND device
> + * @suspended: Set to 1 when the device is suspended, 0 when it's not
> + * @cur_cs: Currently selected target. -1 means no target selected, otherwise we
> + * should always have cur_cs >= 0 && cur_cs < nanddev_ntargets().
> + * NAND Controller drivers should not modify this value, but they're
> + * allowed to read it.
> + * @read_retries: The number of read retry modes supported
> + * @controller: The hardware controller structure which is shared among multiple
> + * independent devices
> + * @ecc: The ECC controller structure
> + * @priv: Chip private data
> */
> -
> struct nand_chip {
> struct nand_device base;
> + struct nand_id id;
> + struct nand_parameters parameters;
> struct nand_manufacturer manufacturer;
> struct nand_chip_ops ops;
> struct nand_legacy legacy;
> -
> unsigned int options;
> +
> + /* Data interface */
> + int onfi_timing_mode_default;
> + struct nand_data_interface data_interface;
> +
> + /* Bad block information */
> + unsigned int bbt_erase_shift;
> unsigned int bbt_options;
> + unsigned int badblockpos;
> + unsigned int badblockbits;
> + struct nand_bbt_descr *bbt_td;
> + struct nand_bbt_descr *bbt_md;
> + struct nand_bbt_descr *badblock_pattern;
> + u8 *bbt;
>
> + /* Device internal layout */
> unsigned int page_shift;
> unsigned int phys_erase_shift;
> - unsigned int bbt_erase_shift;
> unsigned int chip_shift;
> unsigned int pagemask;
> + unsigned int subpagesize;
> +
> + /* Buffers */
> u8 *data_buf;
> -
> + u8 *oob_poi;
> struct {
> unsigned int bitflips;
> int page;
> } pagecache;
> + unsigned long buf_align;
>
> - unsigned int subpagesize;
> - int onfi_timing_mode_default;
> - unsigned int badblockpos;
> - unsigned int badblockbits;
> -
> - struct nand_id id;
> - struct nand_parameters parameters;
> -
> - struct nand_data_interface data_interface;
> -
> + /* Internals */
> + struct mutex lock;
> + unsigned int suspended : 1;
> int cur_cs;
> -
> int read_retries;
>
> - struct mutex lock;
> - unsigned int suspended : 1;
> -
> - u8 *oob_poi;
> + /* Externals */
> struct nand_controller *controller;
> -
> struct nand_ecc_ctrl ecc;
> - unsigned long buf_align;
> -
> - u8 *bbt;
> - struct nand_bbt_descr *bbt_td;
> - struct nand_bbt_descr *bbt_md;
> -
> - struct nand_bbt_descr *badblock_pattern;
> -
> void *priv;
> };
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 07/19] mtd: rawnand: Compare the actual timing values
2020-05-25 17:42 ` [PATCH v4 07/19] mtd: rawnand: Compare the actual timing values Miquel Raynal
@ 2020-05-25 19:01 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:01 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:27 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Avoid relying just on the default timing mode to discriminate if the
> data interface must be restored. Do a memcmp() instead.
Maybe you should explain why you do that.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 14f1359a60b8..7567c973964b 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -2512,7 +2512,8 @@ int nand_reset(struct nand_chip *chip, int chipnr)
> * nand_setup_data_interface() uses ->set/get_features() which would
> * fail anyway as the parameter page is not available yet.
> */
> - if (!chip->onfi_timing_mode_default)
> + if (!memcmp(&chip->data_interface, &saved_data_intf,
> + sizeof(saved_data_intf)))
> return 0;
>
> chip->data_interface = saved_data_intf;
We should probably have the data_interface object allocated at some
point, and play with pointers instead of copying the data around.
That's possible now that you've patched all drivers to use nand_scan()
and control the cleanup path.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 08/19] mtd: rawnand: Rename onfi_timing_mode_default
2020-05-25 17:42 ` [PATCH v4 08/19] mtd: rawnand: Rename onfi_timing_mode_default Miquel Raynal
@ 2020-05-25 19:07 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:07 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:28 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This parameter would better be named default_timing_mode as we are
> opening the way to non-ONFI timings.
We should patch the toshiba driver to implement the interface you
introduce in this patch series. Along with the suggestion to allocate
the 'ideal' data_interface instead of resetting the interface to mode 0,
you shouldn't need this field anymore.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 20 +++++++++-----------
> drivers/mtd/nand/raw/nand_toshiba.c | 2 +-
> include/linux/mtd/rawnand.h | 8 ++++----
> 3 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 7567c973964b..adbc12580e2e 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -952,7 +952,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
> static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> {
> u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
> - chip->onfi_timing_mode_default,
> + chip->default_timing_mode,
> };
> int ret;
>
> @@ -987,9 +987,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> if (ret)
> goto err_reset_chip;
>
> - if (tmode_param[0] != chip->onfi_timing_mode_default) {
> + if (tmode_param[0] != chip->default_timing_mode) {
> pr_warn("timing mode %d not acknowledged by the NAND chip\n",
> - chip->onfi_timing_mode_default);
> + chip->default_timing_mode);
> goto err_reset_chip;
> }
>
> @@ -1016,9 +1016,8 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> * and the driver.
> * First tries to retrieve supported timing modes from ONFI information,
> * and if the NAND chip does not support ONFI, relies on the
> - * ->onfi_timing_mode_default specified in the nand_ids table. After this
> - * function nand_chip->data_interface is initialized with the best timing mode
> - * available.
> + * ->default_timing_mode specified in the nand_ids table. After this function
> + * nand_chip->data_interface is initialized with the best timing mode available.
> *
> * Returns 0 for success or negative error code otherwise.
> */
> @@ -1037,10 +1036,10 @@ static int nand_init_data_interface(struct nand_chip *chip)
> if (chip->parameters.onfi) {
> modes = chip->parameters.onfi->async_timing_mode;
> } else {
> - if (!chip->onfi_timing_mode_default)
> + if (!chip->default_timing_mode)
> return 0;
>
> - modes = GENMASK(chip->onfi_timing_mode_default, 0);
> + modes = GENMASK(chip->default_timing_mode, 0);
> }
>
> for (mode = fls(modes) - 1; mode >= 0; mode--) {
> @@ -1056,7 +1055,7 @@ static int nand_init_data_interface(struct nand_chip *chip)
> NAND_DATA_IFACE_CHECK_ONLY,
> &chip->data_interface);
> if (!ret) {
> - chip->onfi_timing_mode_default = mode;
> + chip->default_timing_mode = mode;
> break;
> }
> }
> @@ -4814,8 +4813,7 @@ static bool find_full_id_nand(struct nand_chip *chip,
> chip->options |= type->options;
> chip->base.eccreq.strength = NAND_ECC_STRENGTH(type);
> chip->base.eccreq.step_size = NAND_ECC_STEP(type);
> - chip->onfi_timing_mode_default =
> - type->onfi_timing_mode_default;
> + chip->default_timing_mode = type->onfi_timing_mode_default;
>
> chip->parameters.model = kstrdup(type->name, GFP_KERNEL);
> if (!chip->parameters.model)
> diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
> index ae069905d7e4..b6efaf5195bb 100644
> --- a/drivers/mtd/nand/raw/nand_toshiba.c
> +++ b/drivers/mtd/nand/raw/nand_toshiba.c
> @@ -198,7 +198,7 @@ static int tc58teg5dclta00_init(struct nand_chip *chip)
> {
> struct mtd_info *mtd = nand_to_mtd(chip);
>
> - chip->onfi_timing_mode_default = 5;
> + chip->default_timing_mode = 5;
> chip->options |= NAND_NEED_SCRAMBLING;
> mtd_set_pairing_scheme(mtd, &dist3_pairing_scheme);
>
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index 622da6527a36..a4b68e7b246a 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1067,9 +1067,9 @@ struct nand_manufacturer {
> * @options: Various chip options. They can partly be set to inform nand_scan
> * about special functionality. See the defines for further
> * explanation.
> - * @onfi_timing_mode_default: Default ONFI timing mode. This field is set to the
> - * actually used ONFI mode if the chip is ONFI
> - * compliant or deduced from the datasheet otherwise
> + * @default_timing_mode: Default timing mode. This field is set to the actually
> + * used ONFI mode if the chip is ONFI compliant or deduced
> + * from the datasheet otherwise
> * @data_interface: NAND interface timing information
> * @bbt_erase_shift: Number of address bits in a bbt entry
> * @bbt_options: Bad block table specific options. All options used here must
> @@ -1117,7 +1117,7 @@ struct nand_chip {
> unsigned int options;
>
> /* Data interface */
> - int onfi_timing_mode_default;
> + int default_timing_mode;
> struct nand_data_interface data_interface;
>
> /* Bad block information */
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 09/19] mtd: rawnand: Use the data interface mode entry when relevant
2020-05-25 17:42 ` [PATCH v4 09/19] mtd: rawnand: Use the data interface mode entry when relevant Miquel Raynal
@ 2020-05-25 19:09 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:09 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:29 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> The data interface setup does not care about the default timing mode
> but cares about the actual timing mode at the time of the call of this
> helper.
>
> Use this entry instead and let chip->default_timing_mode only be used
> at initialization time.
Yep, that's the right thing to do
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index adbc12580e2e..514ac78899ec 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -951,9 +951,8 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
> */
> static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> {
> - u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = {
> - chip->default_timing_mode,
> - };
> + u8 mode = chip->data_interface.timings.mode;
> + u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { mode, };
> int ret;
>
> if (!nand_has_setup_data_iface(chip))
> @@ -987,9 +986,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> if (ret)
> goto err_reset_chip;
>
> - if (tmode_param[0] != chip->default_timing_mode) {
> + if (tmode_param[0] != mode) {
> pr_warn("timing mode %d not acknowledged by the NAND chip\n",
> - chip->default_timing_mode);
> + mode);
> goto err_reset_chip;
> }
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 10/19] mtd: rawnand: Rename nand_has_setup_data_interface()
2020-05-25 17:42 ` [PATCH v4 10/19] mtd: rawnand: Rename nand_has_setup_data_interface() Miquel Raynal
@ 2020-05-25 19:10 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:10 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:30 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This is really a NAND controller hook so call it
> nand_controller_has_setup_data_interface(), which makes much more
> sense.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/internals.h | 2 +-
> drivers/mtd/nand/raw/nand_base.c | 6 +++---
> drivers/mtd/nand/raw/nand_legacy.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index a518acfd9b3f..347d42c55353 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -130,7 +130,7 @@ static inline int nand_exec_op(struct nand_chip *chip,
> return chip->controller->ops->exec_op(chip, op, false);
> }
>
> -static inline bool nand_has_setup_data_iface(struct nand_chip *chip)
> +static inline bool nand_controller_has_setup_data_iface(struct nand_chip *chip)
> {
> if (!chip->controller || !chip->controller->ops ||
> !chip->controller->ops->setup_data_interface)
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 514ac78899ec..d446394a2ea0 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -910,7 +910,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
> {
> int ret;
>
> - if (!nand_has_setup_data_iface(chip))
> + if (!nand_controller_has_setup_data_iface(chip))
> return 0;
>
> /*
> @@ -955,7 +955,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { mode, };
> int ret;
>
> - if (!nand_has_setup_data_iface(chip))
> + if (!nand_controller_has_setup_data_iface(chip))
> return 0;
>
> /* Change the mode on the chip side (if supported by the NAND chip) */
> @@ -1024,7 +1024,7 @@ static int nand_init_data_interface(struct nand_chip *chip)
> {
> int modes, mode, ret;
>
> - if (!nand_has_setup_data_iface(chip))
> + if (!nand_controller_has_setup_data_iface(chip))
> return 0;
>
> /*
> diff --git a/drivers/mtd/nand/raw/nand_legacy.c b/drivers/mtd/nand/raw/nand_legacy.c
> index d64791c06a97..8b91aa7773d8 100644
> --- a/drivers/mtd/nand/raw/nand_legacy.c
> +++ b/drivers/mtd/nand/raw/nand_legacy.c
> @@ -365,7 +365,7 @@ static void nand_ccs_delay(struct nand_chip *chip)
> * Wait tCCS_min if it is correctly defined, otherwise wait 500ns
> * (which should be safe for all NANDs).
> */
> - if (nand_has_setup_data_iface(chip))
> + if (nand_controller_has_setup_data_iface(chip))
> ndelay(chip->data_interface.timings.sdr.tCCS_min / 1000);
> else
> ndelay(500);
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 11/19] mtd: rawnand: Fix nand_setup_data_interface() description
2020-05-25 17:42 ` [PATCH v4 11/19] mtd: rawnand: Fix nand_setup_data_interface() description Miquel Raynal
@ 2020-05-25 19:13 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:13 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:31 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This is a copy/paste error and belongs to nand_init_data_interface()
> description.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index d446394a2ea0..c8f5d4557ed5 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -941,11 +941,8 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
> * @chip: The NAND chip
> * @chipnr: Internal die id
> *
> - * Find and configure the best data interface and NAND timings supported by
> - * the chip and the driver.
> - * First tries to retrieve supported timing modes from ONFI information,
> - * and if the NAND chip does not support ONFI, relies on the
> - * ->onfi_timing_mode_default specified in the nand_ids table.
> + * Configure what has been reported to be the best data interface and NAND
> + * timings supported by the chip and the driver.
> *
> * Returns 0 for success or negative error code otherwise.
> */
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 12/19] mtd: rawnand: Rename nand_init_data_interface()
2020-05-25 17:42 ` [PATCH v4 12/19] mtd: rawnand: Rename nand_init_data_interface() Miquel Raynal
@ 2020-05-25 19:17 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:17 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:32 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This name is a bit misleading, what we do in this helper is trying to
> find the best timings supported by the controller and the chip. In
> other words, we choose the data interface.
Well, we currently only choose the best SDR timings, not the best data
interface config (which would imply selecting between SDR and DDR). I
do agree that it's what we're aiming at, but saying that we choose the
best interface config is a bit of lie :-).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Other than the inaccuracy in the commit message, the change itself
makes sense:
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index c8f5d4557ed5..ac08d1fc710a 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1005,7 +1005,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> }
>
> /**
> - * nand_init_data_interface - find the best data interface and timings
> + * nand_choose_data_interface - find the best data interface and timings
> * @chip: The NAND chip
> *
> * Find the best data interface and NAND timings supported by the chip
> @@ -1017,7 +1017,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> *
> * Returns 0 for success or negative error code otherwise.
> */
> -static int nand_init_data_interface(struct nand_chip *chip)
> +static int nand_choose_data_interface(struct nand_chip *chip)
> {
> int modes, mode, ret;
>
> @@ -6045,8 +6045,8 @@ static int nand_scan_tail(struct nand_chip *chip)
> if (!mtd->bitflip_threshold)
> mtd->bitflip_threshold = DIV_ROUND_UP(mtd->ecc_strength * 3, 4);
>
> - /* Initialize the ->data_interface field. */
> - ret = nand_init_data_interface(chip);
> + /* Find the fastest data interface for this chip */
> + ret = nand_choose_data_interface(chip);
> if (ret)
> goto err_nanddev_cleanup;
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 13/19] mtd: rawnand: timings: Update onfi_fill_data_interface() kernel doc
2020-05-25 17:42 ` [PATCH v4 13/19] mtd: rawnand: timings: Update onfi_fill_data_interface() kernel doc Miquel Raynal
@ 2020-05-25 19:18 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:18 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:33 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Describe all parameters and drop the legacy [NAND Interface] prefix.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/nand_timings.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index 36d21be3dfe5..a73d934e86f9 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -274,9 +274,10 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> };
>
> /**
> - * onfi_fill_data_interface - [NAND Interface] Initialize a data interface from
> - * given ONFI mode
> - * @mode: The ONFI timing mode
> + * onfi_fill_data_interface - Initialize a data interface from a given ONFI mode
> + * @chip: The NAND chip
> + * @type: The data interface type
> + * @timing_mode: The ONFI timing mode
> */
> int onfi_fill_data_interface(struct nand_chip *chip,
> enum nand_data_interface_type type,
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 14/19] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface
2020-05-25 17:42 ` [PATCH v4 14/19] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface Miquel Raynal
@ 2020-05-25 19:26 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:26 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:34 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> We rely be default on the data interface which is in the nand_chip
> structure but it should be possible to fill any other data interface.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/internals.h | 1 +
> drivers/mtd/nand/raw/nand_base.c | 7 ++++---
> drivers/mtd/nand/raw/nand_timings.c | 3 ++-
> 3 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 347d42c55353..dc84e3b55d48 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -85,6 +85,7 @@ int nand_markbad_bbm(struct nand_chip *chip, loff_t ofs);
> int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
> int allowbbt);
> int onfi_fill_data_interface(struct nand_chip *chip,
> + struct nand_data_interface *iface,
> enum nand_data_interface_type type,
> int timing_mode);
> int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index ac08d1fc710a..776f2d119bad 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -927,7 +927,7 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr)
> * timings to timing mode 0.
> */
>
> - onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
> + onfi_fill_data_interface(chip, &chip->data_interface, NAND_SDR_IFACE, 0);
> ret = chip->controller->ops->setup_data_interface(chip, chipnr,
> &chip->data_interface);
> if (ret)
> @@ -1039,7 +1039,8 @@ static int nand_choose_data_interface(struct nand_chip *chip)
> }
>
> for (mode = fls(modes) - 1; mode >= 0; mode--) {
> - ret = onfi_fill_data_interface(chip, NAND_SDR_IFACE, mode);
> + ret = onfi_fill_data_interface(chip, &chip->data_interface,
> + NAND_SDR_IFACE, mode);
> if (ret)
> continue;
>
> @@ -5248,7 +5249,7 @@ static int nand_scan_ident(struct nand_chip *chip, unsigned int maxchips,
> mutex_init(&chip->lock);
>
> /* Enforce the right timings for reset/detection */
> - onfi_fill_data_interface(chip, NAND_SDR_IFACE, 0);
> + onfi_fill_data_interface(chip, &chip->data_interface, NAND_SDR_IFACE, 0);
>
> ret = nand_dt_init(chip);
> if (ret)
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index a73d934e86f9..ce6bb87db2e8 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -276,14 +276,15 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> /**
> * onfi_fill_data_interface - Initialize a data interface from a given ONFI mode
> * @chip: The NAND chip
> + * @iface: The data interface to fill
> * @type: The data interface type
> * @timing_mode: The ONFI timing mode
> */
> int onfi_fill_data_interface(struct nand_chip *chip,
> + struct nand_data_interface *iface,
> enum nand_data_interface_type type,
> int timing_mode)
> {
> - struct nand_data_interface *iface = &chip->data_interface;
> struct onfi_params *onfi = chip->parameters.onfi;
>
> if (type != NAND_SDR_IFACE)
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 15/19] mtd: rawnand: timings: Add a helper to find the closest ONFI mode
2020-05-25 17:42 ` [PATCH v4 15/19] mtd: rawnand: timings: Add a helper to find the closest ONFI mode Miquel Raynal
@ 2020-05-25 19:30 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:30 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:35 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Vendors are allowed to provide their own set of timings. In this case,
> we provide a way to derive the "closest" timing mode so that, if the
> NAND controller does not support tweaking these parameters, it will be
> able to configure itself anyway.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/internals.h | 1 +
> drivers/mtd/nand/raw/nand_timings.c | 52 +++++++++++++++++++++++++++++
> 2 files changed, 53 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index dc84e3b55d48..ac103d8767be 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -88,6 +88,7 @@ int onfi_fill_data_interface(struct nand_chip *chip,
> struct nand_data_interface *iface,
> enum nand_data_interface_type type,
> int timing_mode);
> +unsigned int onfi_find_equivalent_sdr_mode(const struct nand_sdr_timings *vendor_timings);
> int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
> int nand_set_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
> int nand_read_page_raw_notsupp(struct nand_chip *chip, u8 *buf,
> diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c
> index ce6bb87db2e8..7f5caa325fbe 100644
> --- a/drivers/mtd/nand/raw/nand_timings.c
> +++ b/drivers/mtd/nand/raw/nand_timings.c
> @@ -273,6 +273,58 @@ static const struct nand_data_interface onfi_sdr_timings[] = {
> },
> };
>
> +unsigned int onfi_find_equivalent_sdr_mode(const struct nand_sdr_timings *vendor_timings)
I wouldn't call that vendor timings, but spec timings, and why not
closest instead of equivalent. You also lack a kernel doc, and the
could get things fitting in the 80 chars limit if you declare it
like:
unsigned int
onfi_find_closest_sdr_mode(const struct nand_sdr_timings *spec_timings)
> +{
> + const struct nand_sdr_timings *onfi_timings;
> + int mode;
> +
> + for (mode = ARRAY_SIZE(onfi_sdr_timings) - 1; mode > 0; mode--) {
> + onfi_timings = &onfi_sdr_timings[mode].timings.sdr;
> +
> + if (vendor_timings->tCCS_min > onfi_timings->tCCS_min ||
> + vendor_timings->tR_max < onfi_timings->tR_max ||
Do we really care about _max values? Also not sure the < condition is
correct.
> + vendor_timings->tADL_min > onfi_timings->tADL_min ||
> + vendor_timings->tALH_min > onfi_timings->tALH_min ||
> + vendor_timings->tALS_min > onfi_timings->tALS_min ||
> + vendor_timings->tAR_min > onfi_timings->tAR_min ||
> + vendor_timings->tCEA_max < onfi_timings->tCEA_max ||
> + vendor_timings->tCEH_min > onfi_timings->tCEH_min ||
> + vendor_timings->tCH_min > onfi_timings->tCH_min ||
> + vendor_timings->tCHZ_max < onfi_timings->tCHZ_max ||
> + vendor_timings->tCLH_min > onfi_timings->tCLH_min ||
> + vendor_timings->tCLR_min > onfi_timings->tCLR_min ||
> + vendor_timings->tCLS_min > onfi_timings->tCLS_min ||
> + vendor_timings->tCOH_min > onfi_timings->tCOH_min ||
> + vendor_timings->tCS_min > onfi_timings->tCS_min ||
> + vendor_timings->tDH_min > onfi_timings->tDH_min ||
> + vendor_timings->tDS_min > onfi_timings->tDS_min ||
> + vendor_timings->tFEAT_max < onfi_timings->tFEAT_max ||
> + vendor_timings->tIR_min > onfi_timings->tIR_min ||
> + vendor_timings->tITC_max < onfi_timings->tITC_max ||
> + vendor_timings->tRC_min > onfi_timings->tRC_min ||
> + vendor_timings->tREA_max < onfi_timings->tREA_max ||
> + vendor_timings->tREH_min > onfi_timings->tREH_min ||
> + vendor_timings->tRHOH_min > onfi_timings->tRHOH_min ||
> + vendor_timings->tRHW_min > onfi_timings->tRHW_min ||
> + vendor_timings->tRHZ_max < onfi_timings->tRHZ_max ||
> + vendor_timings->tRLOH_min > onfi_timings->tRLOH_min ||
> + vendor_timings->tRP_min > onfi_timings->tRP_min ||
> + vendor_timings->tRR_min > onfi_timings->tRR_min ||
> + vendor_timings->tRST_max < onfi_timings->tRST_max ||
> + vendor_timings->tWB_max < onfi_timings->tWB_max ||
> + vendor_timings->tWC_min > onfi_timings->tWC_min ||
> + vendor_timings->tWH_min > onfi_timings->tWH_min ||
> + vendor_timings->tWHR_min > onfi_timings->tWHR_min ||
> + vendor_timings->tWP_min > onfi_timings->tWP_min ||
> + vendor_timings->tWW_min > onfi_timings->tWW_min)
> + continue;
> +
> + return mode;
> + }
> +
> + return 0;
> +}
> +
> /**
> * onfi_fill_data_interface - Initialize a data interface from a given ONFI mode
> * @chip: The NAND chip
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
2020-05-25 17:42 ` [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface() Miquel Raynal
@ 2020-05-25 19:45 ` Boris Brezillon
2020-05-26 9:35 ` SV: " Rickard X Andersson
0 siblings, 1 reply; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:45 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd, Vignesh Raghavendra
On Mon, 25 May 2020 19:42:37 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This helper is here to simplify the life of NAND manufacturer drivers.
>
> Manufacturers will be allowed to propose their own set of timings and,
> if they want, use this helper to:
> 1/ verify it is supported by the controller,
> 2/ fallback on a supported default ONFI mode, slower but still faster
> than the default mode 0.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/internals.h | 2 ++
> drivers/mtd/nand/raw/nand_base.c | 27 +++++++++++++++++++++++++++
> 2 files changed, 29 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index ac103d8767be..9af6979257e2 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -89,6 +89,8 @@ int onfi_fill_data_interface(struct nand_chip *chip,
> enum nand_data_interface_type type,
> int timing_mode);
> unsigned int onfi_find_equivalent_sdr_mode(const struct nand_sdr_timings *vendor_timings);
> +int nand_choose_best_vendor_sdr_iface(struct nand_chip * chip,
> + struct nand_data_interface *best_iface);
> int nand_get_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
> int nand_set_features(struct nand_chip *chip, int addr, u8 *subfeature_param);
> int nand_read_page_raw_notsupp(struct nand_chip *chip, u8 *buf,
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 15e10f045c9f..d9fe7795f183 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1078,6 +1078,33 @@ static int nand_choose_data_interface(struct nand_chip *chip)
> return nand_choose_best_sdr_iface(chip, &chip->data_interface);
> }
>
> +/**
> + * nand_choose_best_vendor_sdr_iface - given a set of timings, find the closest
> + * mode/timings set for this interface
> + * supported by both the NAND controller and
> + * the NAND chip
> + * @chip: the NAND chip
> + * @best_iface: the best data interface (can eventually be updated)
> + */
> +int nand_choose_best_vendor_sdr_iface(struct nand_chip * chip,
> + struct nand_data_interface *best_iface)
> +{
> + int ret;
> +
> + /* Pick the closest mode */
> + best_iface->timings.mode = onfi_find_equivalent_sdr_mode(&best_iface->timings.sdr);
> +
> + /* Find the closest supported data interface */
> + ret = nand_choose_best_sdr_iface(chip, best_iface);
> + if (ret)
> + return ret;
> +
> + chip->data_interface = *best_iface;
> +
> + return 0;
> +}
Can't we just merge nand_choose_best_vendor_sdr_iface() and
nand_choose_best_sdr_iface()?
int nand_choose_best_sdr_timings(struct nand_chip * chip,
struct nand_data_interface *iface,
const struct nand_sdr_timing *spec_timings)
{
iface->type = SDR;
if (spec_timings) {
iface->timings.sdr = spec_timings;
iface->timings.mode = onfi_find_closest_sdr_mode(spec_timings);
} else {
unsigned int best_mode;
if (chip->parameters.onfi)
best_mode = fls(chip->parameters.onfi->async_timing_mode) - 1;
else
best_mode = chip->default_timing_mode;
onfi_fill_data_interface(chip, iface,
NAND_SDR_IFACE, best_mode);
}
/* Verify the controller supports the requested interface */
ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
iface);
if (!ret)
return ret;
/* Fallback to slower modes */
for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
ret = onfi_fill_data_interface(chip, iface,
NAND_SDR_IFACE, mode);
if (ret)
continue;
ret = ops->setup_data_interface(chip,
NAND_DATA_IFACE_CHECK_ONLY,
iface);
if (!ret)
break;
}
return 0;
}
> +EXPORT_SYMBOL_GPL(nand_choose_best_vendor_sdr_iface);
> +
> /**
> * nand_fill_column_cycles - fill the column cycles of an address
> * @chip: The NAND chip
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 16/19] mtd: rawnand: Introduce nand_choose_best_sdr_iface()
2020-05-25 17:42 ` [PATCH v4 16/19] mtd: rawnand: Introduce nand_choose_best_sdr_iface() Miquel Raynal
@ 2020-05-25 19:47 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:47 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd, Vignesh Raghavendra
On Mon, 25 May 2020 19:42:36 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Add a new helper: given a data interface with a specific set of
> timings, check with the controller if this interface can be
> supported. If not, fallback to the closest slower ONFI mode.
>
> Extract this logic from nand_choose_data_interface() and use the new
> helper instead, so that this code can be reused later.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/nand_base.c | 74 ++++++++++++++++++++------------
> 1 file changed, 46 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 776f2d119bad..15e10f045c9f 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1004,6 +1004,42 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> return ret;
> }
>
> +/**
> + * nand_choose_best_sdr_iface - given a data interface, find the closest
> + * mode/timings set for this interface supported
> + * by both the NAND controller and the NAND chip
> + * @chip: the NAND chip
> + * @best_iface: the best data interface (can eventually be updated)
> + */
> +static int nand_choose_best_sdr_iface(struct nand_chip *chip,
> + struct nand_data_interface *best_iface)
You're just choosing the timings here, not the interface
configuration (SDR/DDR+timings), so I'd recommend renaming this function
nand_choose_best_sdr_timings() and passing a nand_sdr_timings object.
> +{
> + const struct nand_controller_ops *ops = chip->controller->ops;
> + int mode, ret;
> +
> + /* Verify the controller supports the requested interface */
> + ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
> + best_iface);
> + if (!ret)
> + return ret;
> +
> + /* Fallback to slower modes */
> + for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
> + ret = onfi_fill_data_interface(chip, best_iface,
> + NAND_SDR_IFACE, mode);
> + if (ret)
> + continue;
> +
> + ret = ops->setup_data_interface(chip,
> + NAND_DATA_IFACE_CHECK_ONLY,
> + best_iface);
> + if (!ret)
> + break;
> + }
> +
> + return 0;
> +}
> +
> /**
> * nand_choose_data_interface - find the best data interface and timings
> * @chip: The NAND chip
> @@ -1019,7 +1055,7 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr)
> */
> static int nand_choose_data_interface(struct nand_chip *chip)
> {
> - int modes, mode, ret;
> + int best_mode, ret;
>
> if (!nand_controller_has_setup_data_iface(chip))
> return 0;
> @@ -1029,35 +1065,17 @@ static int nand_choose_data_interface(struct nand_chip *chip)
> * if the NAND does not support ONFI, fallback to the default ONFI
> * timing mode.
> */
> - if (chip->parameters.onfi) {
> - modes = chip->parameters.onfi->async_timing_mode;
> - } else {
> - if (!chip->default_timing_mode)
> - return 0;
> + if (chip->parameters.onfi)
> + best_mode = fls(chip->parameters.onfi->async_timing_mode) - 1;
> + else
> + best_mode = chip->default_timing_mode;
>
> - modes = GENMASK(chip->default_timing_mode, 0);
> - }
> + ret = onfi_fill_data_interface(chip, &chip->data_interface,
> + NAND_SDR_IFACE, best_mode);
> + if (ret)
> + return ret;
>
> - for (mode = fls(modes) - 1; mode >= 0; mode--) {
> - ret = onfi_fill_data_interface(chip, &chip->data_interface,
> - NAND_SDR_IFACE, mode);
> - if (ret)
> - continue;
> -
> - /*
> - * Pass NAND_DATA_IFACE_CHECK_ONLY to only check if the
> - * controller supports the requested timings.
> - */
> - ret = chip->controller->ops->setup_data_interface(chip,
> - NAND_DATA_IFACE_CHECK_ONLY,
> - &chip->data_interface);
> - if (!ret) {
> - chip->default_timing_mode = mode;
> - break;
> - }
> - }
> -
> - return 0;
> + return nand_choose_best_sdr_iface(chip, &chip->data_interface);
> }
>
> /**
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook
2020-05-25 17:42 ` [PATCH v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook Miquel Raynal
@ 2020-05-25 19:51 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:51 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:38 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> This hook can be overloaded by NAND manufacturer drivers to propose
> alternative timings when not following the main standards.
>
> Vendors implementing this hook should:
"Manufacturer drivers ..."
> 1- choose the best timings and fill the data interface,
> 2- verify that the controller supports them.
#1 clearly depends on #2, so I'd just say that the manufacturer driver
is responsible for choosing the data interface config that fits both
the controller and chip capabilities (ideally the best one :)).
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/internals.h | 5 +++++
> drivers/mtd/nand/raw/nand_base.c | 8 ++++++++
> include/linux/mtd/rawnand.h | 2 ++
> 3 files changed, 15 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/internals.h b/drivers/mtd/nand/raw/internals.h
> index 9af6979257e2..61edbab35068 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -146,6 +146,11 @@ static inline bool nand_controller_has_setup_data_iface(struct nand_chip *chip)
> return true;
> }
>
> +static inline bool nand_can_choose_data_interface(struct nand_chip *chip)
> +{
> + return chip->ops.choose_data_interface;
> +}
> +
> /* BBT functions */
> int nand_markbad_bbt(struct nand_chip *chip, loff_t offs);
> int nand_isreserved_bbt(struct nand_chip *chip, loff_t offs);
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index d9fe7795f183..e9df339849d3 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -1060,6 +1060,14 @@ static int nand_choose_data_interface(struct nand_chip *chip)
> if (!nand_controller_has_setup_data_iface(chip))
> return 0;
>
> + /*
> + * Let the NAND vendor hook identify the best timings.
> + * ->choose_data_interface() is expected to update the entire chip's
> + * nand_data_interface structure.
> + */
> + if (nand_can_choose_data_interface(chip))
> + return chip->ops.choose_data_interface(chip);
> +
> /*
> * First try to identify the best timings from ONFI parameters and
> * if the NAND does not support ONFI, fallback to the default ONFI
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index a4b68e7b246a..5b8b94521a18 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1034,6 +1034,7 @@ struct nand_legacy {
> * @lock_area: Lock operation
> * @unlock_area: Unlock operation
> * @setup_read_retry: Set the read-retry mode (mostly needed for MLC NANDs)
> + * @choose_data_interface: Choose the best data interface
> */
> struct nand_chip_ops {
> int (*suspend)(struct nand_chip *chip);
> @@ -1041,6 +1042,7 @@ struct nand_chip_ops {
> int (*lock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> int (*unlock_area)(struct nand_chip *chip, loff_t ofs, uint64_t len);
> int (*setup_read_retry)(struct nand_chip *chip, int retry_mode);
> + int (*choose_data_interface)(struct nand_chip *chip);
> };
>
> /**
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 19/19] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4
2020-05-25 17:42 ` [PATCH v4 19/19] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Miquel Raynal
@ 2020-05-25 19:53 ` Boris Brezillon
0 siblings, 0 replies; 44+ messages in thread
From: Boris Brezillon @ 2020-05-25 19:53 UTC (permalink / raw)
To: Miquel Raynal
Cc: Rickard Andersson, Richard Weinberger, linux-mtd,
Vignesh Raghavendra, Tudor Ambarus
On Mon, 25 May 2020 19:42:39 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> From: Rickard x Andersson <rickaran@axis.com>
>
> The Kioxia/Toshiba TH58NVG2S3HBAI4 NAND memory is not ONFI compliant.
> The timings of the NAND chip memory are quite close to ONFI mode 4 but
> is breaking that spec.
>
> By providing our own set of timings, erase block read speed is increased
> from 6910 kiB/s to 13490 kiB/s and erase block write speed is increased
> from 3350 kiB/s to 4410 kiB/s.
>
> Tested on IMX6SX which has a NAND controller supporting EDO mode.
>
> Signed-off-by: Rickard x Andersson <rickaran@axis.com>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
> drivers/mtd/nand/raw/nand_ids.c | 3 +++
> drivers/mtd/nand/raw/nand_toshiba.c | 38 +++++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nand_ids.c b/drivers/mtd/nand/raw/nand_ids.c
> index e0dbc2e316c7..8b676e8b481b 100644
> --- a/drivers/mtd/nand/raw/nand_ids.c
> +++ b/drivers/mtd/nand/raw/nand_ids.c
> @@ -52,6 +52,9 @@ struct nand_flash_dev nand_flash_ids[] = {
> { .id = {0xad, 0xde, 0x94, 0xda, 0x74, 0xc4} },
> SZ_8K, SZ_8K, SZ_2M, NAND_NEED_SCRAMBLING, 6, 640,
> NAND_ECC_INFO(40, SZ_1K), 4 },
> + {"TH58NVG2S3HBAI4 4G 3.3V 8-bit",
> + { .id = {0x98, 0xdc, 0x91, 0x15, 0x76} },
> + SZ_2K, SZ_512, SZ_128K, 0, 5, 128, NAND_ECC_INFO(8, SZ_512) },
>
> 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),
> diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
> index b6efaf5195bb..6c79464fdf34 100644
> --- a/drivers/mtd/nand/raw/nand_toshiba.c
> +++ b/drivers/mtd/nand/raw/nand_toshiba.c
> @@ -205,6 +205,42 @@ static int tc58teg5dclta00_init(struct nand_chip *chip)
> return 0;
> }
>
> +static int th58nvg2s3hbai4_choose_data_interface(struct nand_chip *chip)
> +{
> + struct nand_data_interface iface;
> + struct nand_sdr_timings *sdr = &iface.timings.sdr;
> + int ret;
> +
> + /* Start with timings from the closest timing mode, mode 4. */
> + ret = onfi_fill_data_interface(chip, &iface, NAND_SDR_IFACE, 4);
> + if (ret)
> + return ret;
> +
> + /* Patch timings that differ from mode 4. */
> + sdr->tALS_min = 12000;
> + sdr->tCHZ_max = 20000;
> + sdr->tCLS_min = 12000;
> + sdr->tCOH_min = 0;
> + sdr->tDS_min = 12000;
> + sdr->tRHOH_min = 25000;
> + sdr->tRHW_min = 30000;
> + sdr->tRHZ_max = 60000;
> + sdr->tWHR_min = 60000;
> +
> + /* Patch timings not part of onfi timing mode. */
> + sdr->tPROG_max = 700000000;
> + sdr->tBERS_max = 5000000000;
Yay! That's much better than redefining all timings.
> +
> + return nand_choose_best_vendor_sdr_iface(chip, &iface);
> +}
> +
> +static int th58nvg2s3hbai4_init(struct nand_chip *chip)
> +{
> + chip->ops.choose_data_interface = th58nvg2s3hbai4_choose_data_interface;
> +
> + return 0;
> +}
> +
> static int toshiba_nand_init(struct nand_chip *chip)
> {
> if (nand_is_slc(chip))
> @@ -217,6 +253,8 @@ static int toshiba_nand_init(struct nand_chip *chip)
>
> if (!strcmp("TC58TEG5DCLTA00", chip->parameters.model))
> tc58teg5dclta00_init(chip);
As said previously, I'd prefer to have a patch using the same approach
for TC58TEG5DCLTA00.
> + if (!strncmp("TH58NVG2S3HBAI4", chip->parameters.model, 15))
> + th58nvg2s3hbai4_init(chip);
>
> return 0;
> }
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* SV: [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
2020-05-25 19:45 ` Boris Brezillon
@ 2020-05-26 9:35 ` Rickard X Andersson
2020-05-26 9:46 ` Miquel Raynal
0 siblings, 1 reply; 44+ messages in thread
From: Rickard X Andersson @ 2020-05-26 9:35 UTC (permalink / raw)
To: Boris Brezillon, Miquel Raynal
Cc: Richard Weinberger, linux-mtd, Vignesh Raghavendra
On Mon, May 25, 2020 at 7:47 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> +/**
> + * nand_choose_best_sdr_iface - given a data interface, find the closest
> + * mode/timings set for this interface supported
> + * by both the NAND controller and the NAND chip
> + * @chip: the NAND chip
> + * @best_iface: the best data interface (can eventually be updated)
> + */
> +static int nand_choose_best_sdr_iface(struct nand_chip *chip,
> + struct nand_data_interface *best_iface)
> +{
> + const struct nand_controller_ops *ops = chip->controller->ops;
> + int mode, ret;
> +
> + /* Verify the controller supports the requested interface */
> + ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
> + best_iface);
> + if (!ret)
> + return ret;
> +
> + /* Fallback to slower modes */
> + for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
> + ret = onfi_fill_data_interface(chip, best_iface,
> + NAND_SDR_IFACE, mode);
> + if (ret)
> + continue;
> +
> + ret = ops->setup_data_interface(chip,
> + NAND_DATA_IFACE_CHECK_ONLY,
> + best_iface);
> + if (!ret)
> + break;
> + }
> +
> + return 0;
> +}
> +
Should we not start looping from "mode = best_iface->timings.mode" ? The first setup_data_interface call in the above function tests the specific timings or am I missing something?
BR,
Rickard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
2020-05-26 9:35 ` SV: " Rickard X Andersson
@ 2020-05-26 9:46 ` Miquel Raynal
2020-05-26 10:10 ` SV: " Rickard X Andersson
0 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-26 9:46 UTC (permalink / raw)
To: Rickard X Andersson
Cc: Richard Weinberger, Boris Brezillon, linux-mtd, Vignesh Raghavendra
Hi Rickard,
Rickard X Andersson <Rickard.Andersson@axis.com> wrote on Tue, 26 May
2020 09:35:38 +0000:
> On Mon, May 25, 2020 at 7:47 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > +/**
> > + * nand_choose_best_sdr_iface - given a data interface, find the closest
> > + * mode/timings set for this interface supported
> > + * by both the NAND controller and the NAND chip
> > + * @chip: the NAND chip
> > + * @best_iface: the best data interface (can eventually be updated)
> > + */
> > +static int nand_choose_best_sdr_iface(struct nand_chip *chip,
> > + struct nand_data_interface *best_iface)
> > +{
> > + const struct nand_controller_ops *ops = chip->controller->ops;
> > + int mode, ret;
> > +
> > + /* Verify the controller supports the requested interface */
> > + ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
> > + best_iface);
> > + if (!ret)
> > + return ret;
> > +
> > + /* Fallback to slower modes */
> > + for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
> > + ret = onfi_fill_data_interface(chip, best_iface,
> > + NAND_SDR_IFACE, mode);
> > + if (ret)
> > + continue;
> > +
> > + ret = ops->setup_data_interface(chip,
> > + NAND_DATA_IFACE_CHECK_ONLY,
> > + best_iface);
> > + if (!ret)
> > + break;
> > + }
> > +
> > + return 0;
> > +}
> > +
>
> Should we not start looping from "mode = best_iface->timings.mode" ? The first setup_data_interface call in the above function tests the specific timings or am I missing something?
Indeed, we assume that the controller driver will not support the
"official" ONFI timings mode X if it did not support the specific
timings close to mode X.
This is an assumption, but I don't think we are far from the reality
here.
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* SV: [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
2020-05-26 9:46 ` Miquel Raynal
@ 2020-05-26 10:10 ` Rickard X Andersson
2020-05-26 10:43 ` Miquel Raynal
0 siblings, 1 reply; 44+ messages in thread
From: Rickard X Andersson @ 2020-05-26 10:10 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Boris Brezillon, linux-mtd, Vignesh Raghavendra
On Tue, May 26, 2020 at 11:46 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > +/**
> > > + * nand_choose_best_sdr_iface - given a data interface, find the closest
> > > + * mode/timings set for this interface supported
> > > + * by both the NAND controller and the NAND chip
> > > + * @chip: the NAND chip
> > > + * @best_iface: the best data interface (can eventually be updated)
> > > + */
> > > +static int nand_choose_best_sdr_iface(struct nand_chip *chip,
> > > + struct nand_data_interface *best_iface)
> > > +{
> > > + const struct nand_controller_ops *ops = chip->controller->ops;
> > > + int mode, ret;
> > > +
> > > + /* Verify the controller supports the requested interface */
> > > + ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
> > > + best_iface);
> > > + if (!ret)
> > > + return ret;
> > > +
> > > + /* Fallback to slower modes */
> > > + for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
> > > + ret = onfi_fill_data_interface(chip, best_iface,
> > > + NAND_SDR_IFACE, mode);
> > > + if (ret)
> > > + continue;
> > > +
> > > + ret = ops->setup_data_interface(chip,
> > > + NAND_DATA_IFACE_CHECK_ONLY,
> > > + best_iface);
> > > + if (!ret)
> > > + break;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> >
> > Should we not start looping from "mode = best_iface->timings.mode" ? The first setup_data_interface call in the above function tests the specific timings or am I missing something?
>
> Indeed, we assume that the controller driver will not support the
> "official" ONFI timings mode X if it did not support the specific
> timings close to mode X.
>
> This is an assumption, but I don't think we are far from the reality
> here.
>
> Miquèl
It could be that the "corresponding onfi mode" is quite low due to deviation of some parameter that are actually not checked/used by the controller drivers.
Another thing, should we not check in order to be sure that mode does not become negative? I.e if best_iface->timings.mode is zero before executing the loop.
Best regards,
Rickard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
2020-05-26 10:10 ` SV: " Rickard X Andersson
@ 2020-05-26 10:43 ` Miquel Raynal
2020-05-26 11:42 ` SV: " Rickard X Andersson
0 siblings, 1 reply; 44+ messages in thread
From: Miquel Raynal @ 2020-05-26 10:43 UTC (permalink / raw)
To: Rickard X Andersson
Cc: Richard Weinberger, Boris Brezillon, linux-mtd, Vignesh Raghavendra
Hi Rickard,
Rickard X Andersson <Rickard.Andersson@axis.com> wrote on Tue, 26 May
2020 10:10:58 +0000:
> On Tue, May 26, 2020 at 11:46 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > > > +/**
> > > > + * nand_choose_best_sdr_iface - given a data interface, find the closest
> > > > + * mode/timings set for this interface supported
> > > > + * by both the NAND controller and the NAND chip
> > > > + * @chip: the NAND chip
> > > > + * @best_iface: the best data interface (can eventually be updated)
> > > > + */
> > > > +static int nand_choose_best_sdr_iface(struct nand_chip *chip,
> > > > + struct nand_data_interface *best_iface)
> > > > +{
> > > > + const struct nand_controller_ops *ops = chip->controller->ops;
> > > > + int mode, ret;
> > > > +
> > > > + /* Verify the controller supports the requested interface */
> > > > + ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
> > > > + best_iface);
> > > > + if (!ret)
> > > > + return ret;
> > > > +
> > > > + /* Fallback to slower modes */
> > > > + for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
> > > > + ret = onfi_fill_data_interface(chip, best_iface,
> > > > + NAND_SDR_IFACE, mode);
> > > > + if (ret)
> > > > + continue;
> > > > +
> > > > + ret = ops->setup_data_interface(chip,
> > > > + NAND_DATA_IFACE_CHECK_ONLY,
> > > > + best_iface);
> > > > + if (!ret)
> > > > + break;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > >
> > > Should we not start looping from "mode = best_iface->timings.mode" ? The first setup_data_interface call in the above function tests the specific timings or am I missing something?
> >
> > Indeed, we assume that the controller driver will not support the
> > "official" ONFI timings mode X if it did not support the specific
> > timings close to mode X.
> >
> > This is an assumption, but I don't think we are far from the reality
> > here.
> >
> > Miquèl
>
> It could be that the "corresponding onfi mode" is quite low due to deviation of some parameter that are actually not checked/used by the controller drivers.
That might happen. I'll change it.
> Another thing, should we not check in order to be sure that mode does not become negative? I.e if best_iface->timings.mode is zero before executing the loop.
I think the for-loop "mode >= 0" condition ensures this will never
happen, right?
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
* SV: [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface()
2020-05-26 10:43 ` Miquel Raynal
@ 2020-05-26 11:42 ` Rickard X Andersson
0 siblings, 0 replies; 44+ messages in thread
From: Rickard X Andersson @ 2020-05-26 11:42 UTC (permalink / raw)
To: Miquel Raynal
Cc: Richard Weinberger, Boris Brezillon, linux-mtd, Vignesh Raghavendra
On Tue, May 26, 2020 at 12:44 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > > +/**
> > > > > + * nand_choose_best_sdr_iface - given a data interface, find the closest
> > > > > + * mode/timings set for this interface supported
> > > > > + * by both the NAND controller and the NAND chip
> > > > > + * @chip: the NAND chip
> > > > > + * @best_iface: the best data interface (can eventually be updated)
> > > > > + */
> > > > > +static int nand_choose_best_sdr_iface(struct nand_chip *chip,
> > > > > + struct nand_data_interface *best_iface)
> > > > > +{
> > > > > + const struct nand_controller_ops *ops = chip->controller->ops;
> > > > > + int mode, ret;
> > > > > +
> > > > > + /* Verify the controller supports the requested interface */
> > > > > + ret = ops->setup_data_interface(chip, NAND_DATA_IFACE_CHECK_ONLY,
> > > > > + best_iface);
> > > > > + if (!ret)
> > > > > + return ret;
> > > > > +
> > > > > + /* Fallback to slower modes */
> > > > > + for (mode = best_iface->timings.mode - 1; mode >= 0; mode--) {
> > > > > + ret = onfi_fill_data_interface(chip, best_iface,
> > > > > + NAND_SDR_IFACE, mode);
> > > > > + if (ret)
> > > > > + continue;
> > > > > +
> > > > > + ret = ops->setup_data_interface(chip,
> > > > > + NAND_DATA_IFACE_CHECK_ONLY,
> > > > > + best_iface);
> > > > > + if (!ret)
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > >
> > > > Should we not start looping from "mode = best_iface->timings.mode" ? The first setup_data_interface call in the above function tests the specific timings or am I missing something?
> > >
> > > Indeed, we assume that the controller driver will not support the
> > > "official" ONFI timings mode X if it did not support the specific
> > > timings close to mode X.
> > >
> > > This is an assumption, but I don't think we are far from the reality
> > > here.
> > >
> > > Miquèl
> >
> > It could be that the "corresponding onfi mode" is quite low due to deviation of some parameter that are actually not checked/used by the controller drivers.
>
> That might happen. I'll change it.
>
> > Another thing, should we not check in order to be sure that mode does not become negative? I.e if best_iface->timings.mode is zero before executing the loop.
>
> I think the for-loop "mode >= 0" condition ensures this will never
> happen, right?
>
> Thanks,
> Miquèl
If best_iface->timings.mode is zero before entering the loop we will not call onfi_fill_data_interface at all.
BR,
Rickard
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2020-05-26 11:42 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-25 17:42 [PATCH v4 00/19] Allow vendor drivers to propose their own timings Miquel Raynal
2020-05-25 17:42 ` [PATCH v4 01/19] mtd: rawnand: Use unsigned types for nand_chip unsigned values Miquel Raynal
2020-05-25 18:33 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 02/19] mtd: rawnand: Only use u8 instead of uint8_t in nand_chip structure Miquel Raynal
2020-05-25 18:36 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 03/19] mtd: rawnand: Create a nand_chip operations structure Miquel Raynal
2020-05-25 18:37 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 04/19] mtd: rawnand: Rename the manufacturer structure Miquel Raynal
2020-05-25 18:38 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 05/19] mtd: rawnand: Declare the nand_manufacturer structure out of nand_chip Miquel Raynal
2020-05-25 18:40 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 06/19] mtd: rawnand: Reorganize the nand_chip structure Miquel Raynal
2020-05-25 18:55 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 07/19] mtd: rawnand: Compare the actual timing values Miquel Raynal
2020-05-25 19:01 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 08/19] mtd: rawnand: Rename onfi_timing_mode_default Miquel Raynal
2020-05-25 19:07 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 09/19] mtd: rawnand: Use the data interface mode entry when relevant Miquel Raynal
2020-05-25 19:09 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 10/19] mtd: rawnand: Rename nand_has_setup_data_interface() Miquel Raynal
2020-05-25 19:10 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 11/19] mtd: rawnand: Fix nand_setup_data_interface() description Miquel Raynal
2020-05-25 19:13 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 12/19] mtd: rawnand: Rename nand_init_data_interface() Miquel Raynal
2020-05-25 19:17 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 13/19] mtd: rawnand: timings: Update onfi_fill_data_interface() kernel doc Miquel Raynal
2020-05-25 19:18 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 14/19] mtd: rawnand: timings: Provide onfi_fill_data_interface() with a data interface Miquel Raynal
2020-05-25 19:26 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 15/19] mtd: rawnand: timings: Add a helper to find the closest ONFI mode Miquel Raynal
2020-05-25 19:30 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 16/19] mtd: rawnand: Introduce nand_choose_best_sdr_iface() Miquel Raynal
2020-05-25 19:47 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 17/19] mtd: rawnand: Introduce nand_choose_best_vendor_sdr_iface() Miquel Raynal
2020-05-25 19:45 ` Boris Brezillon
2020-05-26 9:35 ` SV: " Rickard X Andersson
2020-05-26 9:46 ` Miquel Raynal
2020-05-26 10:10 ` SV: " Rickard X Andersson
2020-05-26 10:43 ` Miquel Raynal
2020-05-26 11:42 ` SV: " Rickard X Andersson
2020-05-25 17:42 ` [PATCH v4 18/19] mtd: rawnand: Add the ->choose_data_interface() hook Miquel Raynal
2020-05-25 19:51 ` Boris Brezillon
2020-05-25 17:42 ` [PATCH v4 19/19] mtd: rawnand: Add timings for Kioxia TH58NVG2S3HBAI4 Miquel Raynal
2020-05-25 19:53 ` Boris Brezillon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).