* [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch)
@ 2018-10-01 13:43 Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 1/7] mtd: uclass: add probe function Miquel Raynal
` (6 more replies)
0 siblings, 7 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
During the last months, Boris Brezillon shared his work to support
serial flashes within Linux. First, he delivered (and merged) a new
layer called spi-mem. He also initiated in Linux MTD subsystem the move
of all 'raw' NAND related code to a raw/ subdirectory, adding at the
same time a NAND core that would be shared with all NAND devices. Then,
he contributed a generic SPI-NAND driver, making use of this NAND core,
as well as some vendor code to drive a few chips.
On top of this work, I made some cleanups in the MTD layer and added an
'mtd' U-Boot command to handle all sort of MTD devices. This should
become the default command instead of having one per flash flavor
('sf', 'nand', 'spi-nand' ?).
The series has been tested on an Ocelot board PCB123 (VSC7514),
featuring a Macronix SPI NAND chip.
TL;DR: the series contains (stripped version since ~30 patches have
already been taken):
- Support for spi-nand devices in mtdparts.
- Generics mtdparts/mtdids parsers.
- A new 'mtd' command.
- A note to set mtdparts command legacy.
To test your SPI-NAND device with U-Boot, you can test someting like:
> setenv mtdparts 'spi-nand0:1m(foo),-(bar)'
> setenv mtdids 'spi-nand0=spi0.0' # spi0.0 is Linux MTD name for this device
> ubi part bar # create a static UBI volume in the bar partition
> mtd list # show the current MTD devices/partitions
Thanks,
Miquèl
NB1: If UBI refuses to attach, verify the partition is epty with
# mtd erase bar
NB2: If your U-Boot crashes and you are using a non SPI-NAND device,
don't forget to probe your device *first* (sf probe, ...).
Changes since v12:
------------------
* Dropped the patch removing a DM-helper without users that was breaking
the build when included from a file that can be compiled either with
and without DM support. Instead, added a forward declaration of struct
udevice, then enclosed the declarations in a #if defined(CONFIG_DM)
conditional with dummy helpers on the other side.
* Build test with Travis CI:
https://travis-ci.org/miquelraynal/u-boot/builds/435575048
Changes since v11:
------------------
* Due to the number of small changes I had to do to handle the various
build failures reported by Travis, I decided to send a full v12 so
that all the patches can be replaced and are known to compile
properly.
* Adding a patch to drop an unused helper in mtd_uboot.c that in some
cases causes build issues (if compiled without DM support enabled).
* Travis CI status (build success except for one unrelated issue):
https://travis-ci.org/miquelraynal/u-boot/builds/434796123
Changes since v10:
------------------
* Only two patches sent independently with small fixes to handle build
failures for some configurations (reported by Travis)
Changes since v9:
-----------------
* mtd_search_alternate_name() is moved in mtd_uboot.c (generic code,
everybody wants to use mtdids).
* mtd_parse_partitions() is still in mtdparts.c because it depends on
partitions support, but the header file declaring it
(include/linux/mtd/partitions.h) also has a dummy function if
#if IS_ENABLED(CONFIG_MTD_PARTITIONS) is false.
* Typo corrected in mtd_parse_partitions prototype
(s/_nb_parts/_nparts/).
* Added Boris' R-b tags.
Changes since v8 (called v7 by mistake):
----------------------------------------
* Moved most of the generic logic to the core (mtd_uboot.c) so that it
can be reused by other parts of U-Boot without depending on anything
else than the MTD core itself.
* Removed the "#ifdef CONFIG_MTD" around mtd_probe_devices() calls now
that the code is in the core.
* Created an helper for partitions deletion (as there is one to
parse/create partition objects).
* Drop "cmd: mtdparts: try to probe the MTD devices as a fallback" to
actually deprecate the command.
* Enhanced a bit the text in Kconfig about deprecating mtdparts.
* Fixed checkpatch.pl warnings in the mtdparts driver.
cmd/mtd.c:
* The logic in the mtd_is_aligned_with_*() functions was wrong, inversed
it so the result is what the reader expects (the logic in the end was
valid, though).
* Discriminate two path: for NAND-like devices (pages, OOB, raw mode,
bad blocks) and the others.
* Add the "bad" feature to display all bad blocks (if any).
* Fix some typos/display strings.
* Moved the refcounting put_mtd_device() below in the code as suggested
by Boris.
* Avoid writing empty pages by default (add an option to force the
writing).
* Always skip bad blocks.
* Lightened the code by creating helpers.
Changes since v7:
-----------------
* Added Stefan R-b tags.
* Used tolower() as suggested by Stefan in lib/strto.c.
* Added a mention saying that we continue to abuse the kB suffix in
lib/strto.c by not making a difference between kB and kiB which both
mean "* 1024" for us. This has been added in the commit log.
* Fixed various bugs in the mtd command and in the parsers, they should
be pretty stable now (tested with two MTD devices probed and several
different partition configurations).
* mtdids is not deprecated at all as opposed at what I wrote in
v6. U-Boot still uses its own MTD names (being <type><idx> like nor0
or spi-nand1) and mtdids is still needed to do the glue.
* Removed a useless patch removing a Kconfig entry I added myself since
during a rebase someone else also added this symbol.
Changes since v6:
-----------------
* Squashed Boris' fixes about build issues in Travis.
* Merged Stefan Roese's contributions.
* cmd: mtdparts:
* added fallthrough comments in switch blocks deriving sizes.
* balanced debug messages to compare freed and allocated partitions.
* move mtdparts_init() declaration in mtd/partitions.h to be used in
mtd command as well.
* introduced a "compatibility" function to return what the MTD device
name could be thanks to the mtdids environment variable (which is
useless with the new partitioning logic).
* mtd: mtdpart:
* reworked the way partitions are created/handled.
* cmd: mtd:
* implement a partitioning logic that takes the best out of the MTD
layer without re-inventing the wheel. Partitions are all freed
correctly when the value of mtdparts changes.
* balance get_mtd_device() by put_mtd_device() calls.
* prevent memory leak by reorganizing the code a bit in do_mtd.
* do not reconstruct partition list when a partition is already in use
(complain about that instead).
* add support for the legacy mtdids (not needed anymore if mtdparts
directly uses the MTD device names as in Linux.
* cmd: ubi:
* clean and simplify the partition handling a bit to make use the best
use of the MTD layer as well and keep the code simple.
Changes since v5:
-----------------
* Included Boris fixup about the build issues.
* Added Rb/Ab tags from Jagan on patchs 20/21.
Changes since v4:
-----------------
* Added Jagan's Acked-by tags to every patch related to the
SPI-mem/SPI-NAND addition.
* Rebased on top of master.
Changes since v3:
-----------------
* Fixed the debug messages in spi-mem to print either Rx or Tx data.
* Fixed a Kconfig error that prevented to build mtdparts with plain
defconfig.
* Fixed a compilation error due to the above error that prevented one
file to be compiled.
* Adapted the mtd command to probe MTD partitions also.
* Declared mtd_probe_devices() in a header so mtdparts or UBI could
use it too (to probe all devices and MTD partitions in a clean way).
* As I worked on mtdparts, I found annoying and completely useless the
fact that we need to prefix the environment variable with
"mtdparts=". Canceled this obligation.
* Added one patch to allow spi-nand devices to be recognized by mtdparts
(this is purely useless but needed to be done in order to use this
command).
* Removed useless definitions of MTD device types in UBI code.
* Wrote a generic mtdparts environment variable parser, used by the mtd
command.
* Used the mtd_probe_devices() function from get_mtd_info() in
cmd/mtdparts.c to be sure the desired partition really does not exist
(otherwise it will be probed and then found).
Changes since v2:
-----------------
* Rebased on u-boot master branch.
* Removed extra-parenthesis in
"mtd: Fallback to ->_read/write() when ->_read/write_oob() is missing"
* s/fiels/files/ in "mtd: move NAND fiels into a raw/ subdirectory"
* Do not describe generic SPI device properties in SPI NAND bindings.
* Changes in the mtd command:
* Printing more information in 'mtd list' (device type, device
characteristics)
* Switch to do_div() instead of '(u32)value64b % value32b' which only
worked because value32b was a power of 2.
* Removed erase.chip option.
* By default, erase/read/write happen on the full MTD device while a
dump will only work on a single page.
Changes since v1:
-----------------
* Fixed the nand_memorg structure of the MX35LF2GE4AB chip.
* Added Reviewed-by tags from Jagan.
* Backported and squashed two patches fixing things in the SPI NAND core
received on the Linux ML.
* Backported more changes in mtdcore.c from Linux.
* Added a patch to add a fallback on mtd->_read/_write() in mtdcore.c
when mtd->_read/write_oob() is not supported.
* Removed the DT changes, useless as the DTs are not available in
mainline yet.
* Addressed Boris/Stefan comments on the 'mtd' command.
* Added support for multi-pages OOB read/write.
Miquel Raynal (7):
mtd: uclass: add probe function
mtd: mtdpart: add a generic mtdparts-like parser
mtd: uboot: search for an equivalent MTD name with the mtdids
mtd: mtdpart: implement proper partition handling
cmd: mtd: add 'mtd' command
cmd: ubi: clean the partition handling
cmd: mtdparts: describe as legacy
cmd/Kconfig | 18 +-
cmd/Makefile | 1 +
cmd/mtd.c | 473 +++++++++++++++++++++++++
cmd/ubi.c | 96 ++---
drivers/mtd/Makefile | 2 +-
drivers/mtd/mtd-uclass.c | 16 +
drivers/mtd/mtd_uboot.c | 224 +++++++++++-
drivers/mtd/mtdcore.c | 2 +
drivers/mtd/mtdpart.c | 623 ++++++++++++++++++++-------------
include/linux/mtd/mtd.h | 37 ++
include/linux/mtd/partitions.h | 22 +-
include/mtd.h | 18 +
12 files changed, 1217 insertions(+), 315 deletions(-)
create mode 100644 cmd/mtd.c
--
2.17.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 1/7] mtd: uclass: add probe function
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
@ 2018-10-01 13:43 ` Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 2/7] mtd: mtdpart: add a generic mtdparts-like parser Miquel Raynal
` (5 subsequent siblings)
6 siblings, 0 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
The user might want to trigger the probe of any MTD device, export these
functions so they can be called from a command source file.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Acked-by: Jagan Teki <jagan@openedev.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/mtd-uclass.c | 16 ++++++++++++++++
include/mtd.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/mtd/mtd-uclass.c b/drivers/mtd/mtd-uclass.c
index 9ca049c437..5418217431 100644
--- a/drivers/mtd/mtd-uclass.c
+++ b/drivers/mtd/mtd-uclass.c
@@ -5,9 +5,25 @@
#include <common.h>
#include <dm.h>
+#include <dm/device-internal.h>
#include <errno.h>
#include <mtd.h>
+/**
+ * mtd_probe - Probe the device @dev if not already done
+ *
+ * @dev: U-Boot device to probe
+ *
+ * @return 0 on success, an error otherwise.
+ */
+int mtd_probe(struct udevice *dev)
+{
+ if (device_active(dev))
+ return 0;
+
+ return device_probe(dev);
+}
+
/*
* Implement a MTD uclass which should include most flash drivers.
* The uclass private is pointed to mtd_info.
diff --git a/include/mtd.h b/include/mtd.h
index 548e7f191b..6e6da3002f 100644
--- a/include/mtd.h
+++ b/include/mtd.h
@@ -19,4 +19,6 @@ static inline struct mtd_info *mtd_get_info(struct udevice *dev)
return dev_get_uclass_priv(dev);
}
+int mtd_probe(struct udevice *dev);
+
#endif /* _MTD_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 2/7] mtd: mtdpart: add a generic mtdparts-like parser
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 1/7] mtd: uclass: add probe function Miquel Raynal
@ 2018-10-01 13:43 ` Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 3/7] mtd: uboot: search for an equivalent MTD name with the mtdids Miquel Raynal
` (4 subsequent siblings)
6 siblings, 0 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
The current parser is very specific to U-Boot mtdparts implementation.
It does not use MTD structures like mtd_info and mtd_partition. Copy
and adapt the current parser in drivers/mtd/mtd-uclass.c (to not break
the current use of mtdparts.c itself) and write some kind of a wrapper
around the current implementation to allow other commands to benefit
from this parsing in a user-friendly way.
This new function will allocate an mtd_partition array for each
successful call. This array must be freed after use by the caller.
The given 'mtdparts' buffer pointer will be moved forward to the next
MTD device (if any, it will point towards a '\0' character otherwise).
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Acked-by: Jagan Teki <jagan@openedev.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/mtdpart.c | 210 +++++++++++++++++++++++++++++++++
include/linux/mtd/partitions.h | 21 ++++
2 files changed, 231 insertions(+)
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 9ccb1b3361..49353b038a 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -26,6 +26,7 @@
#include <linux/mtd/mtd.h>
#include <linux/mtd/partitions.h>
#include <linux/err.h>
+#include <linux/sizes.h>
#include "mtdcore.h"
@@ -76,6 +77,215 @@ char *kstrdup(const char *s, gfp_t gfp)
}
#endif
+#define MTD_SIZE_REMAINING (~0LLU)
+#define MTD_OFFSET_NOT_SPECIFIED (~0LLU)
+
+/**
+ * mtd_parse_partition - Parse @mtdparts partition definition, fill @partition
+ * with it and update the @mtdparts string pointer.
+ *
+ * The partition name is allocated and must be freed by the caller.
+ *
+ * This function is widely inspired from part_parse (mtdparts.c).
+ *
+ * @mtdparts: String describing the partition with mtdparts command syntax
+ * @partition: MTD partition structure to fill
+ *
+ * @return 0 on success, an error otherwise.
+ */
+static int mtd_parse_partition(const char **_mtdparts,
+ struct mtd_partition *partition)
+{
+ const char *mtdparts = *_mtdparts;
+ const char *name = NULL;
+ int name_len;
+ char *buf;
+
+ /* Ensure the partition structure is empty */
+ memset(partition, 0, sizeof(struct mtd_partition));
+
+ /* Fetch the partition size */
+ if (*mtdparts == '-') {
+ /* Assign all remaining space to this partition */
+ partition->size = MTD_SIZE_REMAINING;
+ mtdparts++;
+ } else {
+ partition->size = ustrtoull(mtdparts, (char **)&mtdparts, 0);
+ if (partition->size < SZ_4K) {
+ printf("Minimum partition size 4kiB, %lldB requested\n",
+ partition->size);
+ return -EINVAL;
+ }
+ }
+
+ /* Check for the offset */
+ partition->offset = MTD_OFFSET_NOT_SPECIFIED;
+ if (*mtdparts == '@') {
+ mtdparts++;
+ partition->offset = ustrtoull(mtdparts, (char **)&mtdparts, 0);
+ }
+
+ /* Now look for the name */
+ if (*mtdparts == '(') {
+ name = ++mtdparts;
+ mtdparts = strchr(name, ')');
+ if (!mtdparts) {
+ printf("No closing ')' found in partition name\n");
+ return -EINVAL;
+ }
+ name_len = mtdparts - name + 1;
+ if ((name_len - 1) == 0) {
+ printf("Empty partition name\n");
+ return -EINVAL;
+ }
+ mtdparts++;
+ } else {
+ /* Name will be of the form size at offset */
+ name_len = 22;
+ }
+
+ /* Check if the partition is read-only */
+ if (strncmp(mtdparts, "ro", 2) == 0) {
+ partition->mask_flags |= MTD_WRITEABLE;
+ mtdparts += 2;
+ }
+
+ /* Check for a potential next partition definition */
+ if (*mtdparts == ',') {
+ if (partition->size == MTD_SIZE_REMAINING) {
+ printf("No partitions allowed after a fill-up\n");
+ return -EINVAL;
+ }
+ ++mtdparts;
+ } else if ((*mtdparts == ';') || (*mtdparts == '\0')) {
+ /* NOP */
+ } else {
+ printf("Unexpected character '%c' in mtdparts\n", *mtdparts);
+ return -EINVAL;
+ }
+
+ /*
+ * Allocate a buffer for the name and either copy the provided name or
+ * auto-generate it with the form 'size at offset'.
+ */
+ buf = malloc(name_len);
+ if (!buf)
+ return -ENOMEM;
+
+ if (name)
+ strncpy(buf, name, name_len - 1);
+ else
+ snprintf(buf, name_len, "0x%08llx at 0x%08llx",
+ partition->size, partition->offset);
+
+ buf[name_len - 1] = '\0';
+ partition->name = buf;
+
+ *_mtdparts = mtdparts;
+
+ return 0;
+}
+
+/**
+ * mtd_parse_partitions - Create a partition array from an mtdparts definition
+ *
+ * Stateless function that takes a @parent MTD device, a string @_mtdparts
+ * describing the partitions (with the "mtdparts" command syntax) and creates
+ * the corresponding MTD partition structure array @_parts. Both the name and
+ * the structure partition itself must be freed freed, the caller may use
+ * @mtd_free_parsed_partitions() for this purpose.
+ *
+ * @parent: MTD device which contains the partitions
+ * @_mtdparts: Pointer to a string describing the partitions with "mtdparts"
+ * command syntax.
+ * @_parts: Allocated array containing the partitions, must be freed by the
+ * caller.
+ * @_nparts: Size of @_parts array.
+ *
+ * @return 0 on success, an error otherwise.
+ */
+int mtd_parse_partitions(struct mtd_info *parent, const char **_mtdparts,
+ struct mtd_partition **_parts, int *_nparts)
+{
+ struct mtd_partition partition = {}, *parts;
+ const char *mtdparts = *_mtdparts;
+ int cur_off = 0, cur_sz = 0;
+ int nparts = 0;
+ int ret, idx;
+ u64 sz;
+
+ /* First, iterate over the partitions until we know their number */
+ while (mtdparts[0] != '\0' && mtdparts[0] != ';') {
+ ret = mtd_parse_partition(&mtdparts, &partition);
+ if (ret)
+ return ret;
+
+ free((char *)partition.name);
+ nparts++;
+ }
+
+ /* Allocate an array of partitions to give back to the caller */
+ parts = malloc(sizeof(*parts) * nparts);
+ if (!parts) {
+ printf("Not enough space to save partitions meta-data\n");
+ return -ENOMEM;
+ }
+
+ /* Iterate again over each partition to save the data in our array */
+ for (idx = 0; idx < nparts; idx++) {
+ ret = mtd_parse_partition(_mtdparts, &parts[idx]);
+ if (ret)
+ return ret;
+
+ if (parts[idx].size == MTD_SIZE_REMAINING)
+ parts[idx].size = parent->size - cur_sz;
+ cur_sz += parts[idx].size;
+
+ sz = parts[idx].size;
+ if (sz < parent->writesize || do_div(sz, parent->writesize)) {
+ printf("Partition size must be a multiple of %d\n",
+ parent->writesize);
+ return -EINVAL;
+ }
+
+ if (parts[idx].offset == MTD_OFFSET_NOT_SPECIFIED)
+ parts[idx].offset = cur_off;
+ cur_off += parts[idx].size;
+
+ parts[idx].ecclayout = parent->ecclayout;
+ }
+
+ /* Offset by one mtdparts to point to the next device if any */
+ if (*_mtdparts[0] == ';')
+ (*_mtdparts)++;
+
+ *_parts = parts;
+ *_nparts = nparts;
+
+ return 0;
+}
+
+/**
+ * mtd_free_parsed_partitions - Free dynamically allocated partitions
+ *
+ * Each successful call to @mtd_parse_partitions must be followed by a call to
+ * @mtd_free_parsed_partitions to free any allocated array during the parsing
+ * process.
+ *
+ * @parts: Array containing the partitions that will be freed.
+ * @nparts: Size of @parts array.
+ */
+void mtd_free_parsed_partitions(struct mtd_partition *parts,
+ unsigned int nparts)
+{
+ int i;
+
+ for (i = 0; i < nparts; i++)
+ free((char *)parts[i].name);
+
+ free(parts);
+}
+
/*
* MTD methods which simply translate the effective address and pass through
* to the _real_ device.
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index ce0e8dbee4..6eea0a547a 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -87,4 +87,25 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
int mtd_del_partition(struct mtd_info *master, int partno);
uint64_t mtd_get_device_size(const struct mtd_info *mtd);
+#if defined(CONFIG_MTD_PARTITIONS)
+int mtd_parse_partitions(struct mtd_info *parent, const char **_mtdparts,
+ struct mtd_partition **_parts, int *_nparts);
+void mtd_free_parsed_partitions(struct mtd_partition *parts,
+ unsigned int nparts);
+#else
+static inline int
+mtd_parse_partitions(struct mtd_info *parent, const char **_mtdparts,
+ struct mtd_partition **_parts, int *_nparts)
+{
+ *_nparts = 0;
+
+ return 0;
+}
+static inline void
+mtd_free_parsed_partitions(struct mtd_partition *parts, unsigned int nparts)
+{
+ return;
+}
+#endif /* defined(MTD_PARTITIONS) */
+
#endif
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 3/7] mtd: uboot: search for an equivalent MTD name with the mtdids
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 1/7] mtd: uclass: add probe function Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 2/7] mtd: mtdpart: add a generic mtdparts-like parser Miquel Raynal
@ 2018-10-01 13:43 ` Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 4/7] mtd: mtdpart: implement proper partition handling Miquel Raynal
` (3 subsequent siblings)
6 siblings, 0 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
Using an MTD device (resp. partition) name in mtdparts is simple and
straightforward. However, for a long time already, another name was
given in mtdparts to indicate a device (resp. partition) so the
"mtdids" environment variable was created to do the match.
Let's create a function that, from an MTD device (resp. partition)
name, search for the equivalent name in the "mtdparts" environment
variable thanks to the "mtdids" string.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/mtd_uboot.c | 65 ++++++++++++++++++++++++++++++++++++++++-
include/linux/mtd/mtd.h | 5 ++++
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 2b3b2eecca..8d7e7890b7 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -5,7 +5,70 @@
*/
#include <common.h>
#include <linux/mtd/mtd.h>
-#include <jffs2/jffs2.h>
+#include <jffs2/jffs2.h> /* Legacy */
+
+/**
+ * mtd_search_alternate_name - Search an alternate name for @mtdname thanks to
+ * the mtdids legacy environment variable.
+ *
+ * The mtdids string is a list of comma-separated 'dev_id=mtd_id' tupples.
+ * Check if one of the mtd_id matches mtdname, in this case save dev_id in
+ * altname.
+ *
+ * @mtdname: Current MTD device name
+ * @altname: Alternate name to return
+ * @max_len: Length of the alternate name buffer
+ *
+ * @return 0 on success, an error otherwise.
+ */
+int mtd_search_alternate_name(const char *mtdname, char *altname,
+ unsigned int max_len)
+{
+ const char *mtdids, *equal, *comma, *dev_id, *mtd_id;
+ int dev_id_len, mtd_id_len;
+
+ mtdids = env_get("mtdids");
+ if (!mtdids)
+ return -EINVAL;
+
+ do {
+ /* Find the '=' sign */
+ dev_id = mtdids;
+ equal = strchr(dev_id, '=');
+ if (!equal)
+ break;
+ dev_id_len = equal - mtdids;
+ mtd_id = equal + 1;
+
+ /* Find the end of the tupple */
+ comma = strchr(mtdids, ',');
+ if (comma)
+ mtd_id_len = comma - mtd_id;
+ else
+ mtd_id_len = &mtdids[strlen(mtdids)] - mtd_id + 1;
+
+ if (!dev_id_len || !mtd_id_len)
+ return -EINVAL;
+
+ if (dev_id_len + 1 > max_len)
+ continue;
+
+ /* Compare the name we search with the current mtd_id */
+ if (!strncmp(mtdname, mtd_id, mtd_id_len)) {
+ strncpy(altname, dev_id, dev_id_len);
+ altname[dev_id_len] = 0;
+
+ return 0;
+ }
+
+ /* Go to the next tupple */
+ mtdids = comma + 1;
+ } while (comma);
+
+ return -EINVAL;
+}
+
+/* Legacy */
static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
loff_t *maxsize, int devtype)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index b8c2c3fd59..af6f4a61f8 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -549,5 +549,10 @@ int mtd_arg_off_size(int argc, char *const argv[], int *idx, loff_t *off,
void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset,
const uint64_t length, uint64_t *len_incl_bad,
int *truncated);
+
+/* drivers/mtd/mtd_uboot.c */
+int mtd_search_alternate_name(const char *mtdname, char *altname,
+ unsigned int max_len);
+
#endif
#endif /* __MTD_MTD_H__ */
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 4/7] mtd: mtdpart: implement proper partition handling
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
` (2 preceding siblings ...)
2018-10-01 13:43 ` [U-Boot] [PATCH v13 3/7] mtd: uboot: search for an equivalent MTD name with the mtdids Miquel Raynal
@ 2018-10-01 13:43 ` Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command Miquel Raynal
` (2 subsequent siblings)
6 siblings, 0 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
Instead of collecting partitions in a flat list, create a hierarchy
within the mtd_info structure: use a partitions list to keep track of
the partitions of an MTD device (which might be itself a partition of
another MTD device), a pointer to the parent device (NULL when the MTD
device is the root one, not a partition).
By also saving directly in mtd_info the offset of the partition, we
can get rid of the mtd_part structure.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
drivers/mtd/mtdcore.c | 2 +
drivers/mtd/mtdpart.c | 413 ++++++++++++++-------------------
include/linux/mtd/mtd.h | 32 +++
include/linux/mtd/partitions.h | 1 -
4 files changed, 209 insertions(+), 239 deletions(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index fb1d68d5e2..fb6c779abb 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -426,6 +426,8 @@ int add_mtd_device(struct mtd_info *mtd)
mtd->index = i;
mtd->usecount = 0;
+ INIT_LIST_HEAD(&mtd->partitions);
+
/* default value if not set by driver */
if (mtd->bitflip_threshold == 0)
mtd->bitflip_threshold = mtd->ecc_strength;
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index 49353b038a..4d2ac8107f 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -30,29 +30,12 @@
#include "mtdcore.h"
-/* Our partition linked list */
-static LIST_HEAD(mtd_partitions);
#ifndef __UBOOT__
static DEFINE_MUTEX(mtd_partitions_mutex);
#else
DEFINE_MUTEX(mtd_partitions_mutex);
#endif
-/* Our partition node structure */
-struct mtd_part {
- struct mtd_info mtd;
- struct mtd_info *master;
- uint64_t offset;
- struct list_head list;
-};
-
-/*
- * Given a pointer to the MTD object in the mtd_part structure, we can retrieve
- * the pointer to that structure with this macro.
- */
-#define PART(x) ((struct mtd_part *)(x))
-
-
#ifdef __UBOOT__
/* from mm/util.c */
@@ -294,19 +277,18 @@ void mtd_free_parsed_partitions(struct mtd_partition *parts,
static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, u_char *buf)
{
- struct mtd_part *part = PART(mtd);
struct mtd_ecc_stats stats;
int res;
- stats = part->master->ecc_stats;
- res = part->master->_read(part->master, from + part->offset, len,
- retlen, buf);
+ stats = mtd->parent->ecc_stats;
+ res = mtd->parent->_read(mtd->parent, from + mtd->offset, len,
+ retlen, buf);
if (unlikely(mtd_is_eccerr(res)))
mtd->ecc_stats.failed +=
- part->master->ecc_stats.failed - stats.failed;
+ mtd->parent->ecc_stats.failed - stats.failed;
else
mtd->ecc_stats.corrected +=
- part->master->ecc_stats.corrected - stats.corrected;
+ mtd->parent->ecc_stats.corrected - stats.corrected;
return res;
}
@@ -314,17 +296,13 @@ static int part_read(struct mtd_info *mtd, loff_t from, size_t len,
static int part_point(struct mtd_info *mtd, loff_t from, size_t len,
size_t *retlen, void **virt, resource_size_t *phys)
{
- struct mtd_part *part = PART(mtd);
-
- return part->master->_point(part->master, from + part->offset, len,
- retlen, virt, phys);
+ return mtd->parent->_point(mtd->parent, from + mtd->offset, len,
+ retlen, virt, phys);
}
static int part_unpoint(struct mtd_info *mtd, loff_t from, size_t len)
{
- struct mtd_part *part = PART(mtd);
-
- return part->master->_unpoint(part->master, from + part->offset, len);
+ return mtd->parent->_unpoint(mtd->parent, from + mtd->offset, len);
}
#endif
@@ -333,17 +311,13 @@ static unsigned long part_get_unmapped_area(struct mtd_info *mtd,
unsigned long offset,
unsigned long flags)
{
- struct mtd_part *part = PART(mtd);
-
- offset += part->offset;
- return part->master->_get_unmapped_area(part->master, len, offset,
- flags);
+ offset += mtd->offset;
+ return mtd->parent->_get_unmapped_area(mtd->parent, len, offset, flags);
}
static int part_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
- struct mtd_part *part = PART(mtd);
int res;
if (from >= mtd->size)
@@ -368,7 +342,7 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
return -EINVAL;
}
- res = part->master->_read_oob(part->master, from + part->offset, ops);
+ res = mtd->parent->_read_oob(mtd->parent, from + mtd->offset, ops);
if (unlikely(res)) {
if (mtd_is_bitflip(res))
mtd->ecc_stats.corrected++;
@@ -381,99 +355,87 @@ static int part_read_oob(struct mtd_info *mtd, loff_t from,
static int part_read_user_prot_reg(struct mtd_info *mtd, loff_t from,
size_t len, size_t *retlen, u_char *buf)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_read_user_prot_reg(part->master, from, len,
- retlen, buf);
+ return mtd->parent->_read_user_prot_reg(mtd->parent, from, len,
+ retlen, buf);
}
static int part_get_user_prot_info(struct mtd_info *mtd, size_t len,
size_t *retlen, struct otp_info *buf)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_get_user_prot_info(part->master, len, retlen,
- buf);
+ return mtd->parent->_get_user_prot_info(mtd->parent, len, retlen,
+ buf);
}
static int part_read_fact_prot_reg(struct mtd_info *mtd, loff_t from,
size_t len, size_t *retlen, u_char *buf)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_read_fact_prot_reg(part->master, from, len,
- retlen, buf);
+ return mtd->parent->_read_fact_prot_reg(mtd->parent, from, len,
+ retlen, buf);
}
static int part_get_fact_prot_info(struct mtd_info *mtd, size_t len,
size_t *retlen, struct otp_info *buf)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_get_fact_prot_info(part->master, len, retlen,
- buf);
+ return mtd->parent->_get_fact_prot_info(mtd->parent, len, retlen,
+ buf);
}
static int part_write(struct mtd_info *mtd, loff_t to, size_t len,
size_t *retlen, const u_char *buf)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_write(part->master, to + part->offset, len,
- retlen, buf);
+ return mtd->parent->_write(mtd->parent, to + mtd->offset, len,
+ retlen, buf);
}
static int part_panic_write(struct mtd_info *mtd, loff_t to, size_t len,
size_t *retlen, const u_char *buf)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_panic_write(part->master, to + part->offset, len,
- retlen, buf);
+ return mtd->parent->_panic_write(mtd->parent, to + mtd->offset, len,
+ retlen, buf);
}
static int part_write_oob(struct mtd_info *mtd, loff_t to,
struct mtd_oob_ops *ops)
{
- struct mtd_part *part = PART(mtd);
-
if (to >= mtd->size)
return -EINVAL;
if (ops->datbuf && to + ops->len > mtd->size)
return -EINVAL;
- return part->master->_write_oob(part->master, to + part->offset, ops);
+ return mtd->parent->_write_oob(mtd->parent, to + mtd->offset, ops);
}
static int part_write_user_prot_reg(struct mtd_info *mtd, loff_t from,
size_t len, size_t *retlen, u_char *buf)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_write_user_prot_reg(part->master, from, len,
- retlen, buf);
+ return mtd->parent->_write_user_prot_reg(mtd->parent, from, len,
+ retlen, buf);
}
static int part_lock_user_prot_reg(struct mtd_info *mtd, loff_t from,
size_t len)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_lock_user_prot_reg(part->master, from, len);
+ return mtd->parent->_lock_user_prot_reg(mtd->parent, from, len);
}
#ifndef __UBOOT__
static int part_writev(struct mtd_info *mtd, const struct kvec *vecs,
unsigned long count, loff_t to, size_t *retlen)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_writev(part->master, vecs, count,
- to + part->offset, retlen);
+ return mtd->parent->_writev(mtd->parent, vecs, count,
+ to + mtd->offset, retlen);
}
#endif
static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
{
- struct mtd_part *part = PART(mtd);
int ret;
- instr->addr += part->offset;
- ret = part->master->_erase(part->master, instr);
+ instr->addr += mtd->offset;
+ ret = mtd->parent->_erase(mtd->parent, instr);
if (ret) {
if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
- instr->fail_addr -= part->offset;
- instr->addr -= part->offset;
+ instr->fail_addr -= mtd->offset;
+ instr->addr -= mtd->offset;
}
return ret;
}
@@ -481,11 +443,9 @@ static int part_erase(struct mtd_info *mtd, struct erase_info *instr)
void mtd_erase_callback(struct erase_info *instr)
{
if (instr->mtd->_erase == part_erase) {
- struct mtd_part *part = PART(instr->mtd);
-
if (instr->fail_addr != MTD_FAIL_ADDR_UNKNOWN)
- instr->fail_addr -= part->offset;
- instr->addr -= part->offset;
+ instr->fail_addr -= instr->mtd->offset;
+ instr->addr -= instr->mtd->offset;
}
if (instr->callback)
instr->callback(instr);
@@ -494,107 +454,112 @@ EXPORT_SYMBOL_GPL(mtd_erase_callback);
static int part_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_lock(part->master, ofs + part->offset, len);
+ return mtd->parent->_lock(mtd->parent, ofs + mtd->offset, len);
}
static int part_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_unlock(part->master, ofs + part->offset, len);
+ return mtd->parent->_unlock(mtd->parent, ofs + mtd->offset, len);
}
static int part_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_is_locked(part->master, ofs + part->offset, len);
+ return mtd->parent->_is_locked(mtd->parent, ofs + mtd->offset, len);
}
static void part_sync(struct mtd_info *mtd)
{
- struct mtd_part *part = PART(mtd);
- part->master->_sync(part->master);
+ mtd->parent->_sync(mtd->parent);
}
#ifndef __UBOOT__
static int part_suspend(struct mtd_info *mtd)
{
- struct mtd_part *part = PART(mtd);
- return part->master->_suspend(part->master);
+ return mtd->parent->_suspend(mtd->parent);
}
static void part_resume(struct mtd_info *mtd)
{
- struct mtd_part *part = PART(mtd);
- part->master->_resume(part->master);
+ mtd->parent->_resume(mtd->parent);
}
#endif
static int part_block_isreserved(struct mtd_info *mtd, loff_t ofs)
{
- struct mtd_part *part = PART(mtd);
- ofs += part->offset;
- return part->master->_block_isreserved(part->master, ofs);
+ ofs += mtd->offset;
+ return mtd->parent->_block_isreserved(mtd->parent, ofs);
}
static int part_block_isbad(struct mtd_info *mtd, loff_t ofs)
{
- struct mtd_part *part = PART(mtd);
- ofs += part->offset;
- return part->master->_block_isbad(part->master, ofs);
+ ofs += mtd->offset;
+ return mtd->parent->_block_isbad(mtd->parent, ofs);
}
static int part_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
- struct mtd_part *part = PART(mtd);
int res;
- ofs += part->offset;
- res = part->master->_block_markbad(part->master, ofs);
+ ofs += mtd->offset;
+ res = mtd->parent->_block_markbad(mtd->parent, ofs);
if (!res)
mtd->ecc_stats.badblocks++;
return res;
}
-static inline void free_partition(struct mtd_part *p)
+static inline void free_partition(struct mtd_info *p)
{
- kfree(p->mtd.name);
+ kfree(p->name);
kfree(p);
}
/*
* This function unregisters and destroy all slave MTD objects which are
- * attached to the given master MTD object.
+ * attached to the given master MTD object, recursively.
*/
-
-int del_mtd_partitions(struct mtd_info *master)
+static int do_del_mtd_partitions(struct mtd_info *master)
{
- struct mtd_part *slave, *next;
+ struct mtd_info *slave, *next;
int ret, err = 0;
- debug("Deleting MTD partitions on \"%s\":\n", master->name);
+ list_for_each_entry_safe(slave, next, &master->partitions, node) {
+ if (mtd_has_partitions(slave))
+ del_mtd_partitions(slave);
- mutex_lock(&mtd_partitions_mutex);
- list_for_each_entry_safe(slave, next, &mtd_partitions, list)
- if (slave->master == master) {
- ret = del_mtd_device(&slave->mtd);
- if (ret < 0) {
- err = ret;
- continue;
- }
- list_del(&slave->list);
- free_partition(slave);
+ debug("Deleting %s MTD partition\n", slave->name);
+ ret = del_mtd_device(slave);
+ if (ret < 0) {
+ printf("Error when deleting partition \"%s\" (%d)\n",
+ slave->name, ret);
+ err = ret;
+ continue;
}
- mutex_unlock(&mtd_partitions_mutex);
+
+ list_del(&slave->node);
+ free_partition(slave);
+ }
return err;
}
-static struct mtd_part *allocate_partition(struct mtd_info *master,
- const struct mtd_partition *part, int partno,
- uint64_t cur_offset)
+int del_mtd_partitions(struct mtd_info *master)
{
- struct mtd_part *slave;
+ int ret;
+
+ debug("Deleting MTD partitions on \"%s\":\n", master->name);
+
+ mutex_lock(&mtd_partitions_mutex);
+ ret = do_del_mtd_partitions(master);
+ mutex_unlock(&mtd_partitions_mutex);
+
+ return ret;
+}
+
+static struct mtd_info *allocate_partition(struct mtd_info *master,
+ const struct mtd_partition *part,
+ int partno, uint64_t cur_offset)
+{
+ struct mtd_info *slave;
char *name;
/* allocate the partition structure */
@@ -609,85 +574,87 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
}
/* set up the MTD object for this partition */
- slave->mtd.type = master->type;
- slave->mtd.flags = master->flags & ~part->mask_flags;
- slave->mtd.size = part->size;
- slave->mtd.writesize = master->writesize;
- slave->mtd.writebufsize = master->writebufsize;
- slave->mtd.oobsize = master->oobsize;
- slave->mtd.oobavail = master->oobavail;
- slave->mtd.subpage_sft = master->subpage_sft;
+ slave->type = master->type;
+ slave->flags = master->flags & ~part->mask_flags;
+ slave->size = part->size;
+ slave->writesize = master->writesize;
+ slave->writebufsize = master->writebufsize;
+ slave->oobsize = master->oobsize;
+ slave->oobavail = master->oobavail;
+ slave->subpage_sft = master->subpage_sft;
- slave->mtd.name = name;
- slave->mtd.owner = master->owner;
+ slave->name = name;
+ slave->owner = master->owner;
#ifndef __UBOOT__
- slave->mtd.backing_dev_info = master->backing_dev_info;
+ slave->backing_dev_info = master->backing_dev_info;
/* NOTE: we don't arrange MTDs as a tree; it'd be error-prone
* to have the same data be in two different partitions.
*/
- slave->mtd.dev.parent = master->dev.parent;
+ slave->dev.parent = master->dev.parent;
#endif
if (master->_read)
- slave->mtd._read = part_read;
+ slave->_read = part_read;
if (master->_write)
- slave->mtd._write = part_write;
+ slave->_write = part_write;
if (master->_panic_write)
- slave->mtd._panic_write = part_panic_write;
+ slave->_panic_write = part_panic_write;
#ifndef __UBOOT__
if (master->_point && master->_unpoint) {
- slave->mtd._point = part_point;
- slave->mtd._unpoint = part_unpoint;
+ slave->_point = part_point;
+ slave->_unpoint = part_unpoint;
}
#endif
if (master->_get_unmapped_area)
- slave->mtd._get_unmapped_area = part_get_unmapped_area;
+ slave->_get_unmapped_area = part_get_unmapped_area;
if (master->_read_oob)
- slave->mtd._read_oob = part_read_oob;
+ slave->_read_oob = part_read_oob;
if (master->_write_oob)
- slave->mtd._write_oob = part_write_oob;
+ slave->_write_oob = part_write_oob;
if (master->_read_user_prot_reg)
- slave->mtd._read_user_prot_reg = part_read_user_prot_reg;
+ slave->_read_user_prot_reg = part_read_user_prot_reg;
if (master->_read_fact_prot_reg)
- slave->mtd._read_fact_prot_reg = part_read_fact_prot_reg;
+ slave->_read_fact_prot_reg = part_read_fact_prot_reg;
if (master->_write_user_prot_reg)
- slave->mtd._write_user_prot_reg = part_write_user_prot_reg;
+ slave->_write_user_prot_reg = part_write_user_prot_reg;
if (master->_lock_user_prot_reg)
- slave->mtd._lock_user_prot_reg = part_lock_user_prot_reg;
+ slave->_lock_user_prot_reg = part_lock_user_prot_reg;
if (master->_get_user_prot_info)
- slave->mtd._get_user_prot_info = part_get_user_prot_info;
+ slave->_get_user_prot_info = part_get_user_prot_info;
if (master->_get_fact_prot_info)
- slave->mtd._get_fact_prot_info = part_get_fact_prot_info;
+ slave->_get_fact_prot_info = part_get_fact_prot_info;
if (master->_sync)
- slave->mtd._sync = part_sync;
+ slave->_sync = part_sync;
#ifndef __UBOOT__
if (!partno && !master->dev.class && master->_suspend &&
master->_resume) {
- slave->mtd._suspend = part_suspend;
- slave->mtd._resume = part_resume;
+ slave->_suspend = part_suspend;
+ slave->_resume = part_resume;
}
if (master->_writev)
- slave->mtd._writev = part_writev;
+ slave->_writev = part_writev;
#endif
if (master->_lock)
- slave->mtd._lock = part_lock;
+ slave->_lock = part_lock;
if (master->_unlock)
- slave->mtd._unlock = part_unlock;
+ slave->_unlock = part_unlock;
if (master->_is_locked)
- slave->mtd._is_locked = part_is_locked;
+ slave->_is_locked = part_is_locked;
if (master->_block_isreserved)
- slave->mtd._block_isreserved = part_block_isreserved;
+ slave->_block_isreserved = part_block_isreserved;
if (master->_block_isbad)
- slave->mtd._block_isbad = part_block_isbad;
+ slave->_block_isbad = part_block_isbad;
if (master->_block_markbad)
- slave->mtd._block_markbad = part_block_markbad;
- slave->mtd._erase = part_erase;
- slave->master = master;
+ slave->_block_markbad = part_block_markbad;
+ slave->_erase = part_erase;
+ slave->parent = master;
slave->offset = part->offset;
+ INIT_LIST_HEAD(&slave->partitions);
+ INIT_LIST_HEAD(&slave->node);
if (slave->offset == MTDPART_OFS_APPEND)
slave->offset = cur_offset;
@@ -703,41 +670,41 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
}
if (slave->offset == MTDPART_OFS_RETAIN) {
slave->offset = cur_offset;
- if (master->size - slave->offset >= slave->mtd.size) {
- slave->mtd.size = master->size - slave->offset
- - slave->mtd.size;
+ if (master->size - slave->offset >= slave->size) {
+ slave->size = master->size - slave->offset
+ - slave->size;
} else {
debug("mtd partition \"%s\" doesn't have enough space: %#llx < %#llx, disabled\n",
part->name, master->size - slave->offset,
- slave->mtd.size);
+ slave->size);
/* register to preserve ordering */
goto out_register;
}
}
- if (slave->mtd.size == MTDPART_SIZ_FULL)
- slave->mtd.size = master->size - slave->offset;
+ if (slave->size == MTDPART_SIZ_FULL)
+ slave->size = master->size - slave->offset;
debug("0x%012llx-0x%012llx : \"%s\"\n", (unsigned long long)slave->offset,
- (unsigned long long)(slave->offset + slave->mtd.size), slave->mtd.name);
+ (unsigned long long)(slave->offset + slave->size), slave->name);
/* let's do some sanity checks */
if (slave->offset >= master->size) {
/* let's register it anyway to preserve ordering */
slave->offset = 0;
- slave->mtd.size = 0;
+ slave->size = 0;
printk(KERN_ERR"mtd: partition \"%s\" is out of reach -- disabled\n",
part->name);
goto out_register;
}
- if (slave->offset + slave->mtd.size > master->size) {
- slave->mtd.size = master->size - slave->offset;
+ if (slave->offset + slave->size > master->size) {
+ slave->size = master->size - slave->offset;
printk(KERN_WARNING"mtd: partition \"%s\" extends beyond the end of device \"%s\" -- size truncated to %#llx\n",
- part->name, master->name, (unsigned long long)slave->mtd.size);
+ part->name, master->name, slave->size);
}
if (master->numeraseregions > 1) {
/* Deal with variable erase size stuff */
int i, max = master->numeraseregions;
- u64 end = slave->offset + slave->mtd.size;
+ u64 end = slave->offset + slave->size;
struct mtd_erase_region_info *regions = master->eraseregions;
/* Find the first erase regions which is part of this
@@ -750,44 +717,43 @@ static struct mtd_part *allocate_partition(struct mtd_info *master,
/* Pick biggest erasesize */
for (; i < max && regions[i].offset < end; i++) {
- if (slave->mtd.erasesize < regions[i].erasesize) {
- slave->mtd.erasesize = regions[i].erasesize;
- }
+ if (slave->erasesize < regions[i].erasesize)
+ slave->erasesize = regions[i].erasesize;
}
- BUG_ON(slave->mtd.erasesize == 0);
+ WARN_ON(slave->erasesize == 0);
} else {
/* Single erase size */
- slave->mtd.erasesize = master->erasesize;
+ slave->erasesize = master->erasesize;
}
- if ((slave->mtd.flags & MTD_WRITEABLE) &&
- mtd_mod_by_eb(slave->offset, &slave->mtd)) {
+ if ((slave->flags & MTD_WRITEABLE) &&
+ mtd_mod_by_eb(slave->offset, slave)) {
/* Doesn't start on a boundary of major erase size */
/* FIXME: Let it be writable if it is on a boundary of
* _minor_ erase size though */
- slave->mtd.flags &= ~MTD_WRITEABLE;
+ slave->flags &= ~MTD_WRITEABLE;
printk(KERN_WARNING"mtd: partition \"%s\" doesn't start on an erase block boundary -- force read-only\n",
part->name);
}
- if ((slave->mtd.flags & MTD_WRITEABLE) &&
- mtd_mod_by_eb(slave->mtd.size, &slave->mtd)) {
- slave->mtd.flags &= ~MTD_WRITEABLE;
+ if ((slave->flags & MTD_WRITEABLE) &&
+ mtd_mod_by_eb(slave->size, slave)) {
+ slave->flags &= ~MTD_WRITEABLE;
printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase block -- force read-only\n",
part->name);
}
- slave->mtd.ecclayout = master->ecclayout;
- slave->mtd.ecc_step_size = master->ecc_step_size;
- slave->mtd.ecc_strength = master->ecc_strength;
- slave->mtd.bitflip_threshold = master->bitflip_threshold;
+ slave->ecclayout = master->ecclayout;
+ slave->ecc_step_size = master->ecc_step_size;
+ slave->ecc_strength = master->ecc_strength;
+ slave->bitflip_threshold = master->bitflip_threshold;
if (master->_block_isbad) {
uint64_t offs = 0;
- while (offs < slave->mtd.size) {
+ while (offs < slave->size) {
if (mtd_block_isbad(master, offs + slave->offset))
- slave->mtd.ecc_stats.badblocks++;
- offs += slave->mtd.erasesize;
+ slave->ecc_stats.badblocks++;
+ offs += slave->erasesize;
}
}
@@ -800,7 +766,7 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
long long offset, long long length)
{
struct mtd_partition part;
- struct mtd_part *p, *new;
+ struct mtd_info *p, *new;
uint64_t start, end;
int ret = 0;
@@ -829,21 +795,20 @@ int mtd_add_partition(struct mtd_info *master, const char *name,
end = offset + length;
mutex_lock(&mtd_partitions_mutex);
- list_for_each_entry(p, &mtd_partitions, list)
- if (p->master == master) {
- if ((start >= p->offset) &&
- (start < (p->offset + p->mtd.size)))
- goto err_inv;
+ list_for_each_entry(p, &master->partitions, node) {
+ if (start >= p->offset &&
+ (start < (p->offset + p->size)))
+ goto err_inv;
- if ((end >= p->offset) &&
- (end < (p->offset + p->mtd.size)))
- goto err_inv;
- }
+ if (end >= p->offset &&
+ (end < (p->offset + p->size)))
+ goto err_inv;
+ }
- list_add(&new->list, &mtd_partitions);
+ list_add_tail(&new->node, &master->partitions);
mutex_unlock(&mtd_partitions_mutex);
- add_mtd_device(&new->mtd);
+ add_mtd_device(new);
return ret;
err_inv:
@@ -855,18 +820,17 @@ EXPORT_SYMBOL_GPL(mtd_add_partition);
int mtd_del_partition(struct mtd_info *master, int partno)
{
- struct mtd_part *slave, *next;
+ struct mtd_info *slave, *next;
int ret = -EINVAL;
mutex_lock(&mtd_partitions_mutex);
- list_for_each_entry_safe(slave, next, &mtd_partitions, list)
- if ((slave->master == master) &&
- (slave->mtd.index == partno)) {
- ret = del_mtd_device(&slave->mtd);
+ list_for_each_entry_safe(slave, next, &master->partitions, node)
+ if (slave->index == partno) {
+ ret = del_mtd_device(slave);
if (ret < 0)
break;
- list_del(&slave->list);
+ list_del(&slave->node);
free_partition(slave);
break;
}
@@ -890,20 +854,10 @@ int add_mtd_partitions(struct mtd_info *master,
const struct mtd_partition *parts,
int nbparts)
{
- struct mtd_part *slave;
+ struct mtd_info *slave;
uint64_t cur_offset = 0;
int i;
-#ifdef __UBOOT__
- /*
- * Need to init the list here, since LIST_INIT() does not
- * work on platforms where relocation has problems (like MIPS
- * & PPC).
- */
- if (mtd_partitions.next == NULL)
- INIT_LIST_HEAD(&mtd_partitions);
-#endif
-
debug("Creating %d MTD partitions on \"%s\":\n", nbparts, master->name);
for (i = 0; i < nbparts; i++) {
@@ -912,12 +866,12 @@ int add_mtd_partitions(struct mtd_info *master,
return PTR_ERR(slave);
mutex_lock(&mtd_partitions_mutex);
- list_add(&slave->list, &mtd_partitions);
+ list_add_tail(&slave->node, &master->partitions);
mutex_unlock(&mtd_partitions_mutex);
- add_mtd_device(&slave->mtd);
+ add_mtd_device(slave);
- cur_offset = slave->offset + slave->mtd.size;
+ cur_offset = slave->offset + slave->size;
}
return 0;
@@ -1020,29 +974,12 @@ int parse_mtd_partitions(struct mtd_info *master, const char *const *types,
}
#endif
-int mtd_is_partition(const struct mtd_info *mtd)
-{
- struct mtd_part *part;
- int ispart = 0;
-
- mutex_lock(&mtd_partitions_mutex);
- list_for_each_entry(part, &mtd_partitions, list)
- if (&part->mtd == mtd) {
- ispart = 1;
- break;
- }
- mutex_unlock(&mtd_partitions_mutex);
-
- return ispart;
-}
-EXPORT_SYMBOL_GPL(mtd_is_partition);
-
/* Returns the size of the entire flash chip */
uint64_t mtd_get_device_size(const struct mtd_info *mtd)
{
- if (!mtd_is_partition(mtd))
- return mtd->size;
+ if (mtd_is_partition(mtd))
+ return mtd->parent->size;
- return PART(mtd)->master->size;
+ return mtd->size;
}
EXPORT_SYMBOL_GPL(mtd_get_device_size);
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index af6f4a61f8..68e5915324 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -20,6 +20,7 @@
#include <linux/compat.h>
#include <mtd/mtd-abi.h>
#include <linux/errno.h>
+#include <linux/list.h>
#include <div64.h>
#if IS_ENABLED(CONFIG_DM)
#include <dm/device.h>
@@ -307,6 +308,27 @@ struct mtd_info {
struct udevice *dev;
#endif
int usecount;
+
+ /* MTD devices do not have any parent. MTD partitions do. */
+ struct mtd_info *parent;
+
+ /*
+ * Offset of the partition relatively to the parent offset.
+ * Is 0 for real MTD devices (ie. not partitions).
+ */
+ u64 offset;
+
+ /*
+ * List node used to add an MTD partition to the parent
+ * partition list.
+ */
+ struct list_head node;
+
+ /*
+ * List of partitions attached to this MTD device (the parent
+ * MTD device can itself be a partition).
+ */
+ struct list_head partitions;
};
#if IS_ENABLED(CONFIG_DM)
@@ -334,6 +356,16 @@ static inline const struct device_node *mtd_get_of_node(struct mtd_info *mtd)
}
#endif
+static inline bool mtd_is_partition(const struct mtd_info *mtd)
+{
+ return mtd->parent;
+}
+
+static inline bool mtd_has_partitions(const struct mtd_info *mtd)
+{
+ return !list_empty(&mtd->partitions);
+}
+
int mtd_ooblayout_ecc(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobecc);
int mtd_ooblayout_find_eccregion(struct mtd_info *mtd, int eccbyte,
diff --git a/include/linux/mtd/partitions.h b/include/linux/mtd/partitions.h
index 6eea0a547a..3822237f2a 100644
--- a/include/linux/mtd/partitions.h
+++ b/include/linux/mtd/partitions.h
@@ -81,7 +81,6 @@ extern void register_mtd_parser(struct mtd_part_parser *parser);
extern void deregister_mtd_parser(struct mtd_part_parser *parser);
#endif
-int mtd_is_partition(const struct mtd_info *mtd);
int mtd_add_partition(struct mtd_info *master, const char *name,
long long offset, long long length);
int mtd_del_partition(struct mtd_info *master, int partno);
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
` (3 preceding siblings ...)
2018-10-01 13:43 ` [U-Boot] [PATCH v13 4/7] mtd: mtdpart: implement proper partition handling Miquel Raynal
@ 2018-10-01 13:43 ` Miquel Raynal
2018-10-01 16:19 ` Jagan Teki
` (2 more replies)
2018-10-01 13:43 ` [U-Boot] [PATCH v13 6/7] cmd: ubi: clean the partition handling Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 7/7] cmd: mtdparts: describe as legacy Miquel Raynal
6 siblings, 3 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
There should not be a 'nand' command, a 'sf' command and certainly not
a new 'spi-nand' command. Write a 'mtd' command instead to manage all
MTD devices/partitions at once. This should be the preferred way to
access any MTD device.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Acked-by: Jagan Teki <jagan@openedev.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
cmd/Kconfig | 10 +-
cmd/Makefile | 1 +
cmd/mtd.c | 473 ++++++++++++++++++++++++++++++++++++++++
drivers/mtd/Makefile | 2 +-
drivers/mtd/mtd_uboot.c | 161 +++++++++++++-
include/mtd.h | 16 ++
6 files changed, 659 insertions(+), 4 deletions(-)
create mode 100644 cmd/mtd.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 13d4c991bf..c9be85989c 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -864,6 +864,12 @@ config CMD_MMC_SWRITE
Enable support for the "mmc swrite" command to write Android sparse
images to eMMC.
+config CMD_MTD
+ bool "mtd"
+ select MTD_PARTITIONS
+ help
+ MTD commands support.
+
config CMD_NAND
bool "nand"
default y if NAND_SUNXI
@@ -1697,14 +1703,14 @@ config CMD_MTDPARTS
config MTDIDS_DEFAULT
string "Default MTD IDs"
- depends on CMD_MTDPARTS || CMD_NAND || CMD_FLASH
+ depends on CMD_MTD || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
help
Defines a default MTD IDs list for use with MTD partitions in the
Linux MTD command line partitions format.
config MTDPARTS_DEFAULT
string "Default MTD partition scheme"
- depends on CMD_MTDPARTS || CMD_NAND || CMD_FLASH
+ depends on CMD_MTD || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
help
Defines a default MTD partitioning scheme in the Linux MTD command
line partitions format
diff --git a/cmd/Makefile b/cmd/Makefile
index a61fab6583..3ba65d4d93 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_CMD_MISC) += misc.o
obj-$(CONFIG_CMD_MMC) += mmc.o
obj-$(CONFIG_CMD_MMC_SPI) += mmc_spi.o
obj-$(CONFIG_MP) += mp.o
+obj-$(CONFIG_CMD_MTD) += mtd.o
obj-$(CONFIG_CMD_MTDPARTS) += mtdparts.o
obj-$(CONFIG_CMD_NAND) += nand.o
obj-$(CONFIG_CMD_NET) += net.o
diff --git a/cmd/mtd.c b/cmd/mtd.c
new file mode 100644
index 0000000000..6142223984
--- /dev/null
+++ b/cmd/mtd.c
@@ -0,0 +1,473 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * mtd.c
+ *
+ * Generic command to handle basic operations on any memory device.
+ *
+ * Copyright: Bootlin, 2018
+ * Author: Miquèl Raynal <miquel.raynal@bootlin.com>
+ */
+
+#include <command.h>
+#include <common.h>
+#include <console.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <mtd.h>
+
+static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
+{
+ do_div(len, mtd->writesize);
+
+ return len;
+}
+
+static bool mtd_is_aligned_with_min_io_size(struct mtd_info *mtd, u64 size)
+{
+ return !do_div(size, mtd->writesize);
+}
+
+static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
+{
+ return !do_div(size, mtd->erasesize);
+}
+
+static void mtd_dump_buf(const u8 *buf, uint len, uint offset)
+{
+ int i, j;
+
+ for (i = 0; i < len; ) {
+ printf("0x%08x:\t", offset + i);
+ for (j = 0; j < 8; j++)
+ printf("%02x ", buf[i + j]);
+ printf(" ");
+ i += 8;
+ for (j = 0; j < 8; j++)
+ printf("%02x ", buf[i + j]);
+ printf("\n");
+ i += 8;
+ }
+}
+
+static void mtd_dump_device_buf(struct mtd_info *mtd, u64 start_off,
+ const u8 *buf, u64 len, bool woob)
+{
+ bool has_pages = mtd->type == MTD_NANDFLASH ||
+ mtd->type == MTD_MLCNANDFLASH;
+ int npages = mtd_len_to_pages(mtd, len);
+ uint page;
+
+ if (has_pages) {
+ for (page = 0; page < npages; page++) {
+ u64 data_off = page * mtd->writesize;
+
+ printf("\nDump %d data bytes from 0x%08llx:\n",
+ mtd->writesize, start_off + data_off);
+ mtd_dump_buf(&buf[data_off],
+ mtd->writesize, start_off + data_off);
+
+ if (woob) {
+ u64 oob_off = page * mtd->oobsize;
+
+ printf("Dump %d OOB bytes from page at 0x%08llx:\n",
+ mtd->oobsize, start_off + data_off);
+ mtd_dump_buf(&buf[len + oob_off],
+ mtd->oobsize, 0);
+ }
+ }
+ } else {
+ printf("\nDump %lld data bytes from 0x%llx:\n",
+ len, start_off);
+ mtd_dump_buf(buf, len, start_off);
+ }
+}
+
+static void mtd_show_parts(struct mtd_info *mtd, int level)
+{
+ struct mtd_info *part;
+ int i;
+
+ list_for_each_entry(part, &mtd->partitions, node) {
+ for (i = 0; i < level; i++)
+ printf("\t");
+ printf(" - 0x%012llx-0x%012llx : \"%s\"\n",
+ part->offset, part->offset + part->size, part->name);
+
+ mtd_show_parts(part, level + 1);
+ }
+}
+
+static void mtd_show_device(struct mtd_info *mtd)
+{
+ /* Device */
+ printf("* %s\n", mtd->name);
+#if defined(CONFIG_DM)
+ if (mtd->dev) {
+ printf(" - device: %s\n", mtd->dev->name);
+ printf(" - parent: %s\n", mtd->dev->parent->name);
+ printf(" - driver: %s\n", mtd->dev->driver->name);
+ }
+#endif
+
+ /* MTD device information */
+ printf(" - type: ");
+ switch (mtd->type) {
+ case MTD_RAM:
+ printf("RAM\n");
+ break;
+ case MTD_ROM:
+ printf("ROM\n");
+ break;
+ case MTD_NORFLASH:
+ printf("NOR flash\n");
+ break;
+ case MTD_NANDFLASH:
+ printf("NAND flash\n");
+ break;
+ case MTD_DATAFLASH:
+ printf("Data flash\n");
+ break;
+ case MTD_UBIVOLUME:
+ printf("UBI volume\n");
+ break;
+ case MTD_MLCNANDFLASH:
+ printf("MLC NAND flash\n");
+ break;
+ case MTD_ABSENT:
+ default:
+ printf("Unknown\n");
+ break;
+ }
+
+ printf(" - block size: 0x%x bytes\n", mtd->erasesize);
+ printf(" - min I/O: 0x%x bytes\n", mtd->writesize);
+
+ if (mtd->oobsize) {
+ printf(" - OOB size: %u bytes\n", mtd->oobsize);
+ printf(" - OOB available: %u bytes\n", mtd->oobavail);
+ }
+
+ if (mtd->ecc_strength) {
+ printf(" - ECC strength: %u bits\n", mtd->ecc_strength);
+ printf(" - ECC step size: %u bytes\n", mtd->ecc_step_size);
+ printf(" - bitflip threshold: %u bits\n",
+ mtd->bitflip_threshold);
+ }
+
+ printf(" - 0x%012llx-0x%012llx : \"%s\"\n",
+ mtd->offset, mtd->offset + mtd->size, mtd->name);
+
+ /* MTD partitions, if any */
+ mtd_show_parts(mtd, 1);
+}
+
+/* Logic taken from fs/ubifs/recovery.c:is_empty() */
+static bool mtd_oob_write_is_empty(struct mtd_oob_ops *op)
+{
+ int i;
+
+ for (i = 0; i < op->len; i++)
+ if (op->datbuf[i] != 0xff)
+ return false;
+
+ for (i = 0; i < op->ooblen; i++)
+ if (op->oobbuf[i] != 0xff)
+ return false;
+
+ return true;
+}
+
+static int do_mtd_list(void)
+{
+ struct mtd_info *mtd;
+ int dev_nb = 0;
+
+ /* Ensure all devices (and their partitions) are probed */
+ mtd_probe_devices();
+
+ printf("List of MTD devices:\n");
+ mtd_for_each_device(mtd) {
+ if (!mtd_is_partition(mtd))
+ mtd_show_device(mtd);
+
+ dev_nb++;
+ }
+
+ if (!dev_nb) {
+ printf("No MTD device found\n");
+ return CMD_RET_FAILURE;
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
+static int mtd_special_write_oob(struct mtd_info *mtd, u64 off,
+ struct mtd_oob_ops *io_op,
+ bool write_empty_pages, bool woob)
+{
+ int ret = 0;
+
+ /*
+ * By default, do not write an empty page.
+ * Skip it by simulating a successful write.
+ */
+ if (!write_empty_pages && mtd_oob_write_is_empty(io_op)) {
+ io_op->retlen = mtd->writesize;
+ io_op->oobretlen = woob ? mtd->oobsize : 0;
+ } else {
+ ret = mtd_write_oob(mtd, off, io_op);
+ }
+
+ return ret;
+}
+
+static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+ struct mtd_info *mtd;
+ const char *cmd;
+ char *mtd_name;
+
+ /* All MTD commands need at least two arguments */
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
+ /* Parse the command name and its optional suffixes */
+ cmd = argv[1];
+
+ /* List the MTD devices if that is what the user wants */
+ if (strcmp(cmd, "list") == 0)
+ return do_mtd_list();
+
+ /*
+ * The remaining commands require also@least a device ID.
+ * Check the selected device is valid. Ensure it is probed.
+ */
+ if (argc < 3)
+ return CMD_RET_USAGE;
+
+ mtd_name = argv[2];
+ mtd_probe_devices();
+ mtd = get_mtd_device_nm(mtd_name);
+ if (IS_ERR_OR_NULL(mtd)) {
+ printf("MTD device %s not found, ret %ld\n",
+ mtd_name, PTR_ERR(mtd));
+ return CMD_RET_FAILURE;
+ }
+ put_mtd_device(mtd);
+
+ argc -= 3;
+ argv += 3;
+
+ /* Do the parsing */
+ if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "dump", 4) ||
+ !strncmp(cmd, "write", 5)) {
+ bool has_pages = mtd->type == MTD_NANDFLASH ||
+ mtd->type == MTD_MLCNANDFLASH;
+ bool dump, read, raw, woob, write_empty_pages;
+ struct mtd_oob_ops io_op = {};
+ uint user_addr = 0, npages;
+ u64 start_off, off, len, remaining, default_len;
+ u32 oob_len;
+ u8 *buf;
+ int ret;
+
+ dump = !strncmp(cmd, "dump", 4);
+ read = dump || !strncmp(cmd, "read", 4);
+ raw = strstr(cmd, ".raw");
+ woob = strstr(cmd, ".oob");
+ write_empty_pages = !has_pages || strstr(cmd, ".dontskipff");
+
+ if (!dump) {
+ if (!argc)
+ return CMD_RET_USAGE;
+
+ user_addr = simple_strtoul(argv[0], NULL, 16);
+ argc--;
+ argv++;
+ }
+
+ start_off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
+ if (!mtd_is_aligned_with_min_io_size(mtd, start_off)) {
+ printf("Offset not aligned with a page (0x%x)\n",
+ mtd->writesize);
+ return CMD_RET_FAILURE;
+ }
+
+ default_len = dump ? mtd->writesize : mtd->size;
+ len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) :
+ default_len;
+ if (!mtd_is_aligned_with_min_io_size(mtd, len)) {
+ len = round_up(len, mtd->writesize);
+ printf("Size not on a page boundary (0x%x), rounding to 0x%llx\n",
+ mtd->writesize, len);
+ }
+
+ remaining = len;
+ npages = mtd_len_to_pages(mtd, len);
+ oob_len = woob ? npages * mtd->oobsize : 0;
+
+ if (dump)
+ buf = kmalloc(len + oob_len, GFP_KERNEL);
+ else
+ buf = map_sysmem(user_addr, 0);
+
+ if (!buf) {
+ printf("Could not map/allocate the user buffer\n");
+ return CMD_RET_FAILURE;
+ }
+
+ if (has_pages)
+ printf("%s %lld byte(s) (%d page(s)) at offset 0x%08llx%s%s%s\n",
+ read ? "Reading" : "Writing", len, npages, start_off,
+ raw ? " [raw]" : "", woob ? " [oob]" : "",
+ !read && write_empty_pages ? " [dontskipff]" : "");
+ else
+ printf("%s %lld byte(s) at offset 0x%08llx\n",
+ read ? "Reading" : "Writing", len, start_off);
+
+ io_op.mode = raw ? MTD_OPS_RAW : MTD_OPS_AUTO_OOB;
+ io_op.len = has_pages ? mtd->writesize : len;
+ io_op.ooblen = woob ? mtd->oobsize : 0;
+ io_op.datbuf = buf;
+ io_op.oobbuf = woob ? &buf[len] : NULL;
+
+ /* Search for the first good block after the given offset */
+ off = start_off;
+ while (mtd_block_isbad(mtd, off))
+ off += mtd->erasesize;
+
+ /* Loop over the pages to do the actual read/write */
+ while (remaining) {
+ /* Skip the block if it is bad */
+ if (mtd_is_aligned_with_block_size(mtd, off) &&
+ mtd_block_isbad(mtd, off)) {
+ off += mtd->erasesize;
+ continue;
+ }
+
+ if (read)
+ ret = mtd_read_oob(mtd, off, &io_op);
+ else
+ ret = mtd_special_write_oob(mtd, off, &io_op,
+ write_empty_pages,
+ woob);
+
+ if (ret) {
+ printf("Failure while %s at offset 0x%llx\n",
+ read ? "reading" : "writing", off);
+ return CMD_RET_FAILURE;
+ }
+
+ off += io_op.retlen;
+ remaining -= io_op.retlen;
+ io_op.datbuf += io_op.retlen;
+ io_op.oobbuf += io_op.oobretlen;
+ }
+
+ if (!ret && dump)
+ mtd_dump_device_buf(mtd, start_off, buf, len, woob);
+
+ if (dump)
+ kfree(buf);
+ else
+ unmap_sysmem(buf);
+
+ if (ret) {
+ printf("%s on %s failed with error %d\n",
+ read ? "Read" : "Write", mtd->name, ret);
+ return CMD_RET_FAILURE;
+ }
+
+ } else if (!strcmp(cmd, "erase")) {
+ bool scrub = strstr(cmd, ".dontskipbad");
+ struct erase_info erase_op = {};
+ u64 off, len;
+ int ret;
+
+ off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
+ len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) : mtd->size;
+
+ if (!mtd_is_aligned_with_block_size(mtd, off)) {
+ printf("Offset not aligned with a block (0x%x)\n",
+ mtd->erasesize);
+ return CMD_RET_FAILURE;
+ }
+
+ if (!mtd_is_aligned_with_block_size(mtd, len)) {
+ printf("Size not a multiple of a block (0x%x)\n",
+ mtd->erasesize);
+ return CMD_RET_FAILURE;
+ }
+
+ printf("Erasing 0x%08llx ... 0x%08llx (%d eraseblock(s))\n",
+ off, off + len - 1, mtd_div_by_eb(len, mtd));
+
+ erase_op.mtd = mtd;
+ erase_op.addr = off;
+ erase_op.len = len;
+ erase_op.scrub = scrub;
+
+ while (erase_op.len) {
+ ret = mtd_erase(mtd, &erase_op);
+
+ /* Abort if its not a bad block error */
+ if (ret != -EIO)
+ break;
+
+ printf("Skipping bad block at 0x%08llx\n",
+ erase_op.fail_addr);
+
+ /* Skip bad block and continue behind it */
+ erase_op.len -= erase_op.fail_addr - erase_op.addr;
+ erase_op.len -= mtd->erasesize;
+ erase_op.addr = erase_op.fail_addr + mtd->erasesize;
+ }
+
+ if (ret && ret != -EIO)
+ return CMD_RET_FAILURE;
+ } else if (!strcmp(cmd, "bad")) {
+ loff_t off;
+
+ if (!mtd_can_have_bb(mtd)) {
+ printf("Only NAND-based devices can have bad blocks\n");
+ return CMD_RET_SUCCESS;
+ }
+
+ printf("MTD device %s bad blocks list:\n", mtd->name);
+ for (off = 0; off < mtd->size; off += mtd->erasesize)
+ if (mtd_block_isbad(mtd, off))
+ printf("\t0x%08llx\n", off);
+ } else {
+ return CMD_RET_USAGE;
+ }
+
+ return CMD_RET_SUCCESS;
+}
+
+static char mtd_help_text[] =
+#ifdef CONFIG_SYS_LONGHELP
+ "- generic operations on memory technology devices\n\n"
+ "mtd list\n"
+ "mtd read[.raw][.oob] <name> <addr> [<off> [<size>]]\n"
+ "mtd dump[.raw][.oob] <name> [<off> [<size>]]\n"
+ "mtd write[.raw][.oob][.dontskipff] <name> <addr> [<off> [<size>]]\n"
+ "mtd erase[.dontskipbad] <name> [<off> [<size>]]\n"
+ "\n"
+ "Specific functions:\n"
+ "mtd bad <name>\n"
+ "\n"
+ "With:\n"
+ "\t<name>: NAND partition/chip name\n"
+ "\t<addr>: user address from/to which data will be retrieved/stored\n"
+ "\t<off>: offset in <name> in bytes (default: start of the part)\n"
+ "\t\t* must be block-aligned for erase\n"
+ "\t\t* must be page-aligned otherwise\n"
+ "\t<size>: length of the operation in bytes (default: the entire device)\n"
+ "\t\t* must be a multiple of a block for erase\n"
+ "\t\t* must be a multiple of a page otherwise (special case: default is a page with dump)\n"
+ "\n"
+ "The .dontskipff option forces writing empty pages, don't use it if unsure.\n"
+#endif
+ "";
+
+U_BOOT_CMD(mtd, 10, 1, do_mtd, "MTD utils", mtd_help_text);
diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
index cdf515256a..22ceda93c0 100644
--- a/drivers/mtd/Makefile
+++ b/drivers/mtd/Makefile
@@ -3,7 +3,7 @@
# (C) Copyright 2000-2007
# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
-ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)))
+ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)$(CONFIG_CMD_MTD)))
obj-y += mtdcore.o mtd_uboot.o
endif
obj-$(CONFIG_MTD) += mtd-uclass.o
diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
index 8d7e7890b7..6a211d52ff 100644
--- a/drivers/mtd/mtd_uboot.c
+++ b/drivers/mtd/mtd_uboot.c
@@ -4,8 +4,15 @@
* Heiko Schocher, DENX Software Engineering, hs at denx.de.
*/
#include <common.h>
+#include <dm/device.h>
+#include <dm/uclass-internal.h>
+#include <jffs2/jffs2.h> /* LEGACY */
#include <linux/mtd/mtd.h>
-#include <jffs2/jffs2.h> /* Legacy */
+#include <linux/mtd/partitions.h>
+#include <mtd.h>
+
+#define MTD_NAME_MAX_LEN 20
+
/**
* mtd_search_alternate_name - Search an alternate name for @mtdname thanks to
@@ -68,6 +75,158 @@ int mtd_search_alternate_name(const char *mtdname, char *altname,
return -EINVAL;
}
+#if IS_ENABLED(CONFIG_MTD)
+static void mtd_probe_uclass_mtd_devs(void)
+{
+ struct udevice *dev;
+ int idx = 0;
+
+ /* Probe devices with DM compliant drivers */
+ while (!uclass_find_device(UCLASS_MTD, idx, &dev) && dev) {
+ mtd_probe(dev);
+ idx++;
+ }
+}
+#else
+static void mtd_probe_uclass_mtd_devs(void) { }
+#endif
+
+#if defined(CONFIG_MTD_PARTITIONS)
+int mtd_probe_devices(void)
+{
+ static char *old_mtdparts;
+ static char *old_mtdids;
+ const char *mtdparts = env_get("mtdparts");
+ const char *mtdids = env_get("mtdids");
+ bool remaining_partitions = true;
+ struct mtd_info *mtd;
+
+ mtd_probe_uclass_mtd_devs();
+
+ /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
+ if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
+ return 0;
+
+ /* Update the local copy of mtdparts */
+ free(old_mtdparts);
+ free(old_mtdids);
+ old_mtdparts = strdup(mtdparts);
+ old_mtdids = strdup(mtdids);
+
+ /* If at least one partition is still in use, do not delete anything */
+ mtd_for_each_device(mtd) {
+ if (mtd->usecount) {
+ printf("Partition \"%s\" already in use, aborting\n",
+ mtd->name);
+ return -EACCES;
+ }
+ }
+
+ /*
+ * Everything looks clear, remove all partitions. It is not safe to
+ * remove entries from the mtd_for_each_device loop as it uses idr
+ * indexes and the partitions removal is done in bulk (all partitions of
+ * one device at the same time), so break and iterate from start each
+ * time a new partition is found and deleted.
+ */
+ while (remaining_partitions) {
+ remaining_partitions = false;
+ mtd_for_each_device(mtd) {
+ if (!mtd_is_partition(mtd) && mtd_has_partitions(mtd)) {
+ del_mtd_partitions(mtd);
+ remaining_partitions = true;
+ break;
+ }
+ }
+ }
+
+ /* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
+ if (strstr(mtdparts, "mtdparts="))
+ mtdparts += 9;
+
+ /* For each MTD device in mtdparts */
+ while (mtdparts[0] != '\0') {
+ char mtd_name[MTD_NAME_MAX_LEN], *colon;
+ struct mtd_partition *parts;
+ int mtd_name_len, nparts;
+ int ret;
+
+ colon = strchr(mtdparts, ':');
+ if (!colon) {
+ printf("Wrong mtdparts: %s\n", mtdparts);
+ return -EINVAL;
+ }
+
+ mtd_name_len = colon - mtdparts;
+ strncpy(mtd_name, mtdparts, mtd_name_len);
+ mtd_name[mtd_name_len] = '\0';
+ /* Move the pointer forward (including the ':') */
+ mtdparts += mtd_name_len + 1;
+ mtd = get_mtd_device_nm(mtd_name);
+ if (IS_ERR_OR_NULL(mtd)) {
+ char linux_name[MTD_NAME_MAX_LEN];
+
+ /*
+ * The MTD device named "mtd_name" does not exist. Try
+ * to find a correspondance with an MTD device having
+ * the same type and number as defined in the mtdids.
+ */
+ debug("No device named %s\n", mtd_name);
+ ret = mtd_search_alternate_name(mtd_name, linux_name,
+ MTD_NAME_MAX_LEN);
+ if (!ret)
+ mtd = get_mtd_device_nm(linux_name);
+
+ /*
+ * If no device could be found, move the mtdparts
+ * pointer forward until the next set of partitions.
+ */
+ if (ret || IS_ERR_OR_NULL(mtd)) {
+ printf("Could not find a valid device for %s\n",
+ mtd_name);
+ mtdparts = strchr(mtdparts, ';');
+ if (mtdparts)
+ mtdparts++;
+
+ continue;
+ }
+ }
+
+ /*
+ * Parse the MTD device partitions. It will update the mtdparts
+ * pointer, create an array of parts (that must be freed), and
+ * return the number of partition structures in the array.
+ */
+ ret = mtd_parse_partitions(mtd, &mtdparts, &parts, &nparts);
+ if (ret) {
+ printf("Could not parse device %s\n", mtd->name);
+ put_mtd_device(mtd);
+ return -EINVAL;
+ }
+
+ if (!nparts)
+ continue;
+
+ /* Create the new MTD partitions */
+ add_mtd_partitions(mtd, parts, nparts);
+
+ /* Free the structures allocated during the parsing */
+ mtd_free_parsed_partitions(parts, nparts);
+
+ put_mtd_device(mtd);
+ }
+
+ return 0;
+}
+#else
+int mtd_probe_devices(void)
+{
+ mtd_probe_uclass_mtd_devs();
+
+ return 0;
+}
+#endif /* defined(CONFIG_MTD_PARTITIONS) */
+
/* Legacy */
static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
diff --git a/include/mtd.h b/include/mtd.h
index 6e6da3002f..011f26b3e1 100644
--- a/include/mtd.h
+++ b/include/mtd.h
@@ -8,6 +8,9 @@
#include <linux/mtd/mtd.h>
+struct udevice;
+
+#if defined(CONFIG_DM)
/*
* Get mtd_info structure of the dev, which is stored as uclass private.
*
@@ -20,5 +23,18 @@ static inline struct mtd_info *mtd_get_info(struct udevice *dev)
}
int mtd_probe(struct udevice *dev);
+#else
+static inline struct mtd_info *mtd_get_info(struct udevice *dev)
+{
+ return NULL;
+}
+
+static inline int mtd_probe(struct udevice *dev)
+{
+ return 0;
+}
+#endif
+
+int mtd_probe_devices(void);
#endif /* _MTD_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 6/7] cmd: ubi: clean the partition handling
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
` (4 preceding siblings ...)
2018-10-01 13:43 ` [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command Miquel Raynal
@ 2018-10-01 13:43 ` Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 7/7] cmd: mtdparts: describe as legacy Miquel Raynal
6 siblings, 0 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
UBI should not mess with MTD partitions, now that the partitions are
handled in a clean way, clean the ubi command and avoid using this
uneeded extra-glue to reference the devices.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
cmd/Kconfig | 2 ++
cmd/ubi.c | 96 ++++++++++++++---------------------------------------
2 files changed, 27 insertions(+), 71 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index c9be85989c..b571e84cc1 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1840,6 +1840,8 @@ config CMD_UBI
capabilities. Please, consult the MTD web site for more details
(www.linux-mtd.infradead.org). Activate this option if you want
to use U-Boot UBI commands.
+ It is also strongly encouraged to also enable CONFIG_MTD to get full
+ partition support.
config CMD_UBIFS
tristate "Enable UBIFS - Unsorted block images filesystem commands"
diff --git a/cmd/ubi.c b/cmd/ubi.c
index 0a3405a3b1..bb8f97fc28 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -15,6 +15,7 @@
#include <command.h>
#include <exports.h>
#include <memalign.h>
+#include <mtd.h>
#include <nand.h>
#include <onenand_uboot.h>
#include <linux/mtd/mtd.h>
@@ -29,17 +30,6 @@
/* Private own data */
static struct ubi_device *ubi;
-static char buffer[80];
-static int ubi_initialized;
-
-struct selected_dev {
- char part_name[80];
- int selected;
- int nr;
- struct mtd_info *mtd_info;
-};
-
-static struct selected_dev ubi_dev;
#ifdef CONFIG_CMD_UBIFS
int ubifs_is_mounted(void);
@@ -404,43 +394,24 @@ int ubi_volume_read(char *volume, char *buf, size_t size)
return err;
}
-static int ubi_dev_scan(struct mtd_info *info, char *ubidev,
- const char *vid_header_offset)
+static int ubi_dev_scan(struct mtd_info *info, const char *vid_header_offset)
{
- struct mtd_device *dev;
- struct part_info *part;
- struct mtd_partition mtd_part;
char ubi_mtd_param_buffer[80];
- u8 pnum;
int err;
- if (find_dev_and_part(ubidev, &dev, &pnum, &part) != 0)
- return 1;
+ if (!vid_header_offset)
+ sprintf(ubi_mtd_param_buffer, "%s", info->name);
+ else
+ sprintf(ubi_mtd_param_buffer, "%s,%s", info->name,
+ vid_header_offset);
- sprintf(buffer, "mtd=%d", pnum);
- memset(&mtd_part, 0, sizeof(mtd_part));
- mtd_part.name = buffer;
- mtd_part.size = part->size;
- mtd_part.offset = part->offset;
- add_mtd_partitions(info, &mtd_part, 1);
-
- strcpy(ubi_mtd_param_buffer, buffer);
- if (vid_header_offset)
- sprintf(ubi_mtd_param_buffer, "mtd=%d,%s", pnum,
- vid_header_offset);
err = ubi_mtd_param_parse(ubi_mtd_param_buffer, NULL);
- if (err) {
- del_mtd_partitions(info);
+ if (err)
return -err;
- }
err = ubi_init();
- if (err) {
- del_mtd_partitions(info);
+ if (err)
return -err;
- }
-
- ubi_initialized = 1;
return 0;
}
@@ -465,50 +436,33 @@ int ubi_detach(void)
/*
* Call ubi_exit() before re-initializing the UBI subsystem
*/
- if (ubi_initialized) {
+ if (ubi)
ubi_exit();
- del_mtd_partitions(ubi_dev.mtd_info);
- ubi_initialized = 0;
- }
- ubi_dev.selected = 0;
+ ubi = NULL;
+
return 0;
}
int ubi_part(char *part_name, const char *vid_header_offset)
{
+ struct mtd_info *mtd;
int err = 0;
- char mtd_dev[16];
- struct mtd_device *dev;
- struct part_info *part;
- u8 pnum;
ubi_detach();
- /*
- * Search the mtd device number where this partition
- * is located
- */
- if (find_dev_and_part(part_name, &dev, &pnum, &part)) {
+
+ mtd_probe_devices();
+ mtd = get_mtd_device_nm(part_name);
+ if (IS_ERR(mtd)) {
printf("Partition %s not found!\n", part_name);
return 1;
}
- sprintf(mtd_dev, "%s%d", MTD_DEV_TYPE(dev->id->type), dev->id->num);
- ubi_dev.mtd_info = get_mtd_device_nm(mtd_dev);
- if (IS_ERR(ubi_dev.mtd_info)) {
- printf("Partition %s not found on device %s!\n", part_name,
- mtd_dev);
- return 1;
- }
+ put_mtd_device(mtd);
- ubi_dev.selected = 1;
-
- strcpy(ubi_dev.part_name, part_name);
- err = ubi_dev_scan(ubi_dev.mtd_info, ubi_dev.part_name,
- vid_header_offset);
+ err = ubi_dev_scan(mtd, vid_header_offset);
if (err) {
printf("UBI init error %d\n", err);
printf("Please check, if the correct MTD partition is used (size big enough?)\n");
- ubi_dev.selected = 0;
return err;
}
@@ -539,13 +493,13 @@ static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
/* Print current partition */
if (argc == 2) {
- if (!ubi_dev.selected) {
- printf("Error, no UBI device/partition selected!\n");
+ if (!ubi) {
+ printf("Error, no UBI device selected!\n");
return 1;
}
- printf("Device %d: %s, partition %s\n",
- ubi_dev.nr, ubi_dev.mtd_info->name, ubi_dev.part_name);
+ printf("Device %d: %s, MTD partition %s\n",
+ ubi->ubi_num, ubi->ubi_name, ubi->mtd->name);
return 0;
}
@@ -558,8 +512,8 @@ static int do_ubi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
return ubi_part(argv[2], vid_header_offset);
}
- if ((strcmp(argv[1], "part") != 0) && (!ubi_dev.selected)) {
- printf("Error, no UBI device/partition selected!\n");
+ if ((strcmp(argv[1], "part") != 0) && !ubi) {
+ printf("Error, no UBI device selected!\n");
return 1;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 7/7] cmd: mtdparts: describe as legacy
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
` (5 preceding siblings ...)
2018-10-01 13:43 ` [U-Boot] [PATCH v13 6/7] cmd: ubi: clean the partition handling Miquel Raynal
@ 2018-10-01 13:43 ` Miquel Raynal
6 siblings, 0 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 13:43 UTC (permalink / raw)
To: u-boot
The 'mtdparts' command is not needed anymore. While the environment
variable is still valid (and useful, along with the 'mtdids' one), the
command has been replaced by 'mtd' which is much more close to the MTD
stack and do not add its own specific glue.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Stefan Roese <sr@denx.de>
Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
cmd/Kconfig | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index b571e84cc1..12d19fd675 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1699,7 +1699,11 @@ config CMD_MTDPARTS
bool "MTD partition support"
select MTD_DEVICE if (CMD_NAND || NAND)
help
- MTD partition support
+ MTD partitioning tool support.
+ It is strongly encouraged to avoid using this command
+ anymore along with 'sf', 'nand', 'onenand'. One can still
+ declare the partitions in the mtdparts environment variable
+ but better use the MTD stack and the 'mtd' command instead.
config MTDIDS_DEFAULT
string "Default MTD IDs"
--
2.17.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-01 13:43 ` [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command Miquel Raynal
@ 2018-10-01 16:19 ` Jagan Teki
2018-10-01 20:39 ` Miquel Raynal
2018-10-03 12:35 ` Adam Ford
2018-10-08 17:46 ` Boris Brezillon
2 siblings, 1 reply; 25+ messages in thread
From: Jagan Teki @ 2018-10-01 16:19 UTC (permalink / raw)
To: u-boot
On Monday 01 October 2018 07:13 PM, Miquel Raynal wrote:
> There should not be a 'nand' command, a 'sf' command and certainly not
> a new 'spi-nand' command. Write a 'mtd' command instead to manage all
> MTD devices/partitions at once. This should be the preferred way to
> access any MTD device.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Acked-by: Jagan Teki <jagan@openedev.com>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
[snip]
>
> static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
> diff --git a/include/mtd.h b/include/mtd.h
> index 6e6da3002f..011f26b3e1 100644
> --- a/include/mtd.h
> +++ b/include/mtd.h
> @@ -8,6 +8,9 @@
>
> #include <linux/mtd/mtd.h>
>
> +struct udevice;
> +
> +#if defined(CONFIG_DM)
it should be CONFIG_MTD.
> /*
> * Get mtd_info structure of the dev, which is stored as uclass private.
> *
> @@ -20,5 +23,18 @@ static inline struct mtd_info *mtd_get_info(struct udevice *dev)
> }
>
> int mtd_probe(struct udevice *dev);
> +#else
> +static inline struct mtd_info *mtd_get_info(struct udevice *dev)
> +{
> + return NULL;
> +}
> +
> +static inline int mtd_probe(struct udevice *dev)
> +{
> + return 0;
> +}
there is not caller for this in non-dm, better this block empty is it?
Let me know so-that I can do that and apply.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-01 16:19 ` Jagan Teki
@ 2018-10-01 20:39 ` Miquel Raynal
0 siblings, 0 replies; 25+ messages in thread
From: Miquel Raynal @ 2018-10-01 20:39 UTC (permalink / raw)
To: u-boot
Hi Jagan,
Jagan Teki <jagan@openedev.com> wrote on Mon, 1 Oct 2018 21:49:32 +0530:
> On Monday 01 October 2018 07:13 PM, Miquel Raynal wrote:
> > There should not be a 'nand' command, a 'sf' command and certainly not
> > a new 'spi-nand' command. Write a 'mtd' command instead to manage all
> > MTD devices/partitions at once. This should be the preferred way to
> > access any MTD device.
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Acked-by: Jagan Teki <jagan@openedev.com>
> > Reviewed-by: Stefan Roese <sr@denx.de>
> > Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> > ---
>
> [snip]
>
> > > static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
> > diff --git a/include/mtd.h b/include/mtd.h
> > index 6e6da3002f..011f26b3e1 100644
> > --- a/include/mtd.h
> > +++ b/include/mtd.h
> > @@ -8,6 +8,9 @@
> > > #include <linux/mtd/mtd.h>
> > > +struct udevice;
> > +
> > +#if defined(CONFIG_DM)
>
> it should be CONFIG_MTD.
Same, I don't mind.
>
> > /*
> > * Get mtd_info structure of the dev, which is stored as uclass private.
> > *
> > @@ -20,5 +23,18 @@ static inline struct mtd_info *mtd_get_info(struct udevice *dev)
> > }
> > > int mtd_probe(struct udevice *dev);
> [...]
>
> there is not caller for this in non-dm, better this block empty is it?
So I can't remove mtd_get_info() which has no callers at all but you
want to remove the dummy helper which is not used when DM is not
enabled? I have no problem with that, but the logic is not very
coherent.
>
> Let me know so-that I can do that and apply.
Yes please, you can amend this patch and apply it.
BTW, the build is successful with all configurations
-> https://travis-ci.org/miquelraynal/u-boot/builds/435575048
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-01 13:43 ` [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command Miquel Raynal
2018-10-01 16:19 ` Jagan Teki
@ 2018-10-03 12:35 ` Adam Ford
2018-10-03 12:42 ` Miquel Raynal
2018-10-08 17:46 ` Boris Brezillon
2 siblings, 1 reply; 25+ messages in thread
From: Adam Ford @ 2018-10-03 12:35 UTC (permalink / raw)
To: u-boot
On Mon, Oct 1, 2018 at 8:48 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> There should not be a 'nand' command, a 'sf' command and certainly not
> a new 'spi-nand' command. Write a 'mtd' command instead to manage all
> MTD devices/partitions at once. This should be the preferred way to
> access any MTD device.
What is the expected behavior when I type 'mtd list' on my omap37
board, it just hangs.
I can use the nand read/write functions and mtdparts lists the
partitions, so I know nand works. My defconfig
lists the partitions, so if we're not supposed to use mtdparts, where
I do store the partition information?
I intentionally removed it from the device tree a while ago, because
U-Boot was passing the partition info to Linux.
adam
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Acked-by: Jagan Teki <jagan@openedev.com>
> Reviewed-by: Stefan Roese <sr@denx.de>
> Reviewed-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> cmd/Kconfig | 10 +-
> cmd/Makefile | 1 +
> cmd/mtd.c | 473 ++++++++++++++++++++++++++++++++++++++++
> drivers/mtd/Makefile | 2 +-
> drivers/mtd/mtd_uboot.c | 161 +++++++++++++-
> include/mtd.h | 16 ++
> 6 files changed, 659 insertions(+), 4 deletions(-)
> create mode 100644 cmd/mtd.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 13d4c991bf..c9be85989c 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -864,6 +864,12 @@ config CMD_MMC_SWRITE
> Enable support for the "mmc swrite" command to write Android sparse
> images to eMMC.
>
> +config CMD_MTD
> + bool "mtd"
> + select MTD_PARTITIONS
> + help
> + MTD commands support.
> +
> config CMD_NAND
> bool "nand"
> default y if NAND_SUNXI
> @@ -1697,14 +1703,14 @@ config CMD_MTDPARTS
>
> config MTDIDS_DEFAULT
> string "Default MTD IDs"
> - depends on CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> + depends on CMD_MTD || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> help
> Defines a default MTD IDs list for use with MTD partitions in the
> Linux MTD command line partitions format.
>
> config MTDPARTS_DEFAULT
> string "Default MTD partition scheme"
> - depends on CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> + depends on CMD_MTD || CMD_MTDPARTS || CMD_NAND || CMD_FLASH
> help
> Defines a default MTD partitioning scheme in the Linux MTD command
> line partitions format
> diff --git a/cmd/Makefile b/cmd/Makefile
> index a61fab6583..3ba65d4d93 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -92,6 +92,7 @@ obj-$(CONFIG_CMD_MISC) += misc.o
> obj-$(CONFIG_CMD_MMC) += mmc.o
> obj-$(CONFIG_CMD_MMC_SPI) += mmc_spi.o
> obj-$(CONFIG_MP) += mp.o
> +obj-$(CONFIG_CMD_MTD) += mtd.o
> obj-$(CONFIG_CMD_MTDPARTS) += mtdparts.o
> obj-$(CONFIG_CMD_NAND) += nand.o
> obj-$(CONFIG_CMD_NET) += net.o
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> new file mode 100644
> index 0000000000..6142223984
> --- /dev/null
> +++ b/cmd/mtd.c
> @@ -0,0 +1,473 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * mtd.c
> + *
> + * Generic command to handle basic operations on any memory device.
> + *
> + * Copyright: Bootlin, 2018
> + * Author: Miquèl Raynal <miquel.raynal@bootlin.com>
> + */
> +
> +#include <command.h>
> +#include <common.h>
> +#include <console.h>
> +#include <malloc.h>
> +#include <mapmem.h>
> +#include <mtd.h>
> +
> +static uint mtd_len_to_pages(struct mtd_info *mtd, u64 len)
> +{
> + do_div(len, mtd->writesize);
> +
> + return len;
> +}
> +
> +static bool mtd_is_aligned_with_min_io_size(struct mtd_info *mtd, u64 size)
> +{
> + return !do_div(size, mtd->writesize);
> +}
> +
> +static bool mtd_is_aligned_with_block_size(struct mtd_info *mtd, u64 size)
> +{
> + return !do_div(size, mtd->erasesize);
> +}
> +
> +static void mtd_dump_buf(const u8 *buf, uint len, uint offset)
> +{
> + int i, j;
> +
> + for (i = 0; i < len; ) {
> + printf("0x%08x:\t", offset + i);
> + for (j = 0; j < 8; j++)
> + printf("%02x ", buf[i + j]);
> + printf(" ");
> + i += 8;
> + for (j = 0; j < 8; j++)
> + printf("%02x ", buf[i + j]);
> + printf("\n");
> + i += 8;
> + }
> +}
> +
> +static void mtd_dump_device_buf(struct mtd_info *mtd, u64 start_off,
> + const u8 *buf, u64 len, bool woob)
> +{
> + bool has_pages = mtd->type == MTD_NANDFLASH ||
> + mtd->type == MTD_MLCNANDFLASH;
> + int npages = mtd_len_to_pages(mtd, len);
> + uint page;
> +
> + if (has_pages) {
> + for (page = 0; page < npages; page++) {
> + u64 data_off = page * mtd->writesize;
> +
> + printf("\nDump %d data bytes from 0x%08llx:\n",
> + mtd->writesize, start_off + data_off);
> + mtd_dump_buf(&buf[data_off],
> + mtd->writesize, start_off + data_off);
> +
> + if (woob) {
> + u64 oob_off = page * mtd->oobsize;
> +
> + printf("Dump %d OOB bytes from page at 0x%08llx:\n",
> + mtd->oobsize, start_off + data_off);
> + mtd_dump_buf(&buf[len + oob_off],
> + mtd->oobsize, 0);
> + }
> + }
> + } else {
> + printf("\nDump %lld data bytes from 0x%llx:\n",
> + len, start_off);
> + mtd_dump_buf(buf, len, start_off);
> + }
> +}
> +
> +static void mtd_show_parts(struct mtd_info *mtd, int level)
> +{
> + struct mtd_info *part;
> + int i;
> +
> + list_for_each_entry(part, &mtd->partitions, node) {
> + for (i = 0; i < level; i++)
> + printf("\t");
> + printf(" - 0x%012llx-0x%012llx : \"%s\"\n",
> + part->offset, part->offset + part->size, part->name);
> +
> + mtd_show_parts(part, level + 1);
> + }
> +}
> +
> +static void mtd_show_device(struct mtd_info *mtd)
> +{
> + /* Device */
> + printf("* %s\n", mtd->name);
> +#if defined(CONFIG_DM)
> + if (mtd->dev) {
> + printf(" - device: %s\n", mtd->dev->name);
> + printf(" - parent: %s\n", mtd->dev->parent->name);
> + printf(" - driver: %s\n", mtd->dev->driver->name);
> + }
> +#endif
> +
> + /* MTD device information */
> + printf(" - type: ");
> + switch (mtd->type) {
> + case MTD_RAM:
> + printf("RAM\n");
> + break;
> + case MTD_ROM:
> + printf("ROM\n");
> + break;
> + case MTD_NORFLASH:
> + printf("NOR flash\n");
> + break;
> + case MTD_NANDFLASH:
> + printf("NAND flash\n");
> + break;
> + case MTD_DATAFLASH:
> + printf("Data flash\n");
> + break;
> + case MTD_UBIVOLUME:
> + printf("UBI volume\n");
> + break;
> + case MTD_MLCNANDFLASH:
> + printf("MLC NAND flash\n");
> + break;
> + case MTD_ABSENT:
> + default:
> + printf("Unknown\n");
> + break;
> + }
> +
> + printf(" - block size: 0x%x bytes\n", mtd->erasesize);
> + printf(" - min I/O: 0x%x bytes\n", mtd->writesize);
> +
> + if (mtd->oobsize) {
> + printf(" - OOB size: %u bytes\n", mtd->oobsize);
> + printf(" - OOB available: %u bytes\n", mtd->oobavail);
> + }
> +
> + if (mtd->ecc_strength) {
> + printf(" - ECC strength: %u bits\n", mtd->ecc_strength);
> + printf(" - ECC step size: %u bytes\n", mtd->ecc_step_size);
> + printf(" - bitflip threshold: %u bits\n",
> + mtd->bitflip_threshold);
> + }
> +
> + printf(" - 0x%012llx-0x%012llx : \"%s\"\n",
> + mtd->offset, mtd->offset + mtd->size, mtd->name);
> +
> + /* MTD partitions, if any */
> + mtd_show_parts(mtd, 1);
> +}
> +
> +/* Logic taken from fs/ubifs/recovery.c:is_empty() */
> +static bool mtd_oob_write_is_empty(struct mtd_oob_ops *op)
> +{
> + int i;
> +
> + for (i = 0; i < op->len; i++)
> + if (op->datbuf[i] != 0xff)
> + return false;
> +
> + for (i = 0; i < op->ooblen; i++)
> + if (op->oobbuf[i] != 0xff)
> + return false;
> +
> + return true;
> +}
> +
> +static int do_mtd_list(void)
> +{
> + struct mtd_info *mtd;
> + int dev_nb = 0;
> +
> + /* Ensure all devices (and their partitions) are probed */
> + mtd_probe_devices();
> +
> + printf("List of MTD devices:\n");
> + mtd_for_each_device(mtd) {
> + if (!mtd_is_partition(mtd))
> + mtd_show_device(mtd);
> +
> + dev_nb++;
> + }
> +
> + if (!dev_nb) {
> + printf("No MTD device found\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static int mtd_special_write_oob(struct mtd_info *mtd, u64 off,
> + struct mtd_oob_ops *io_op,
> + bool write_empty_pages, bool woob)
> +{
> + int ret = 0;
> +
> + /*
> + * By default, do not write an empty page.
> + * Skip it by simulating a successful write.
> + */
> + if (!write_empty_pages && mtd_oob_write_is_empty(io_op)) {
> + io_op->retlen = mtd->writesize;
> + io_op->oobretlen = woob ? mtd->oobsize : 0;
> + } else {
> + ret = mtd_write_oob(mtd, off, io_op);
> + }
> +
> + return ret;
> +}
> +
> +static int do_mtd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + struct mtd_info *mtd;
> + const char *cmd;
> + char *mtd_name;
> +
> + /* All MTD commands need at least two arguments */
> + if (argc < 2)
> + return CMD_RET_USAGE;
> +
> + /* Parse the command name and its optional suffixes */
> + cmd = argv[1];
> +
> + /* List the MTD devices if that is what the user wants */
> + if (strcmp(cmd, "list") == 0)
> + return do_mtd_list();
> +
> + /*
> + * The remaining commands require also at least a device ID.
> + * Check the selected device is valid. Ensure it is probed.
> + */
> + if (argc < 3)
> + return CMD_RET_USAGE;
> +
> + mtd_name = argv[2];
> + mtd_probe_devices();
> + mtd = get_mtd_device_nm(mtd_name);
> + if (IS_ERR_OR_NULL(mtd)) {
> + printf("MTD device %s not found, ret %ld\n",
> + mtd_name, PTR_ERR(mtd));
> + return CMD_RET_FAILURE;
> + }
> + put_mtd_device(mtd);
> +
> + argc -= 3;
> + argv += 3;
> +
> + /* Do the parsing */
> + if (!strncmp(cmd, "read", 4) || !strncmp(cmd, "dump", 4) ||
> + !strncmp(cmd, "write", 5)) {
> + bool has_pages = mtd->type == MTD_NANDFLASH ||
> + mtd->type == MTD_MLCNANDFLASH;
> + bool dump, read, raw, woob, write_empty_pages;
> + struct mtd_oob_ops io_op = {};
> + uint user_addr = 0, npages;
> + u64 start_off, off, len, remaining, default_len;
> + u32 oob_len;
> + u8 *buf;
> + int ret;
> +
> + dump = !strncmp(cmd, "dump", 4);
> + read = dump || !strncmp(cmd, "read", 4);
> + raw = strstr(cmd, ".raw");
> + woob = strstr(cmd, ".oob");
> + write_empty_pages = !has_pages || strstr(cmd, ".dontskipff");
> +
> + if (!dump) {
> + if (!argc)
> + return CMD_RET_USAGE;
> +
> + user_addr = simple_strtoul(argv[0], NULL, 16);
> + argc--;
> + argv++;
> + }
> +
> + start_off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
> + if (!mtd_is_aligned_with_min_io_size(mtd, start_off)) {
> + printf("Offset not aligned with a page (0x%x)\n",
> + mtd->writesize);
> + return CMD_RET_FAILURE;
> + }
> +
> + default_len = dump ? mtd->writesize : mtd->size;
> + len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) :
> + default_len;
> + if (!mtd_is_aligned_with_min_io_size(mtd, len)) {
> + len = round_up(len, mtd->writesize);
> + printf("Size not on a page boundary (0x%x), rounding to 0x%llx\n",
> + mtd->writesize, len);
> + }
> +
> + remaining = len;
> + npages = mtd_len_to_pages(mtd, len);
> + oob_len = woob ? npages * mtd->oobsize : 0;
> +
> + if (dump)
> + buf = kmalloc(len + oob_len, GFP_KERNEL);
> + else
> + buf = map_sysmem(user_addr, 0);
> +
> + if (!buf) {
> + printf("Could not map/allocate the user buffer\n");
> + return CMD_RET_FAILURE;
> + }
> +
> + if (has_pages)
> + printf("%s %lld byte(s) (%d page(s)) at offset 0x%08llx%s%s%s\n",
> + read ? "Reading" : "Writing", len, npages, start_off,
> + raw ? " [raw]" : "", woob ? " [oob]" : "",
> + !read && write_empty_pages ? " [dontskipff]" : "");
> + else
> + printf("%s %lld byte(s) at offset 0x%08llx\n",
> + read ? "Reading" : "Writing", len, start_off);
> +
> + io_op.mode = raw ? MTD_OPS_RAW : MTD_OPS_AUTO_OOB;
> + io_op.len = has_pages ? mtd->writesize : len;
> + io_op.ooblen = woob ? mtd->oobsize : 0;
> + io_op.datbuf = buf;
> + io_op.oobbuf = woob ? &buf[len] : NULL;
> +
> + /* Search for the first good block after the given offset */
> + off = start_off;
> + while (mtd_block_isbad(mtd, off))
> + off += mtd->erasesize;
> +
> + /* Loop over the pages to do the actual read/write */
> + while (remaining) {
> + /* Skip the block if it is bad */
> + if (mtd_is_aligned_with_block_size(mtd, off) &&
> + mtd_block_isbad(mtd, off)) {
> + off += mtd->erasesize;
> + continue;
> + }
> +
> + if (read)
> + ret = mtd_read_oob(mtd, off, &io_op);
> + else
> + ret = mtd_special_write_oob(mtd, off, &io_op,
> + write_empty_pages,
> + woob);
> +
> + if (ret) {
> + printf("Failure while %s at offset 0x%llx\n",
> + read ? "reading" : "writing", off);
> + return CMD_RET_FAILURE;
> + }
> +
> + off += io_op.retlen;
> + remaining -= io_op.retlen;
> + io_op.datbuf += io_op.retlen;
> + io_op.oobbuf += io_op.oobretlen;
> + }
> +
> + if (!ret && dump)
> + mtd_dump_device_buf(mtd, start_off, buf, len, woob);
> +
> + if (dump)
> + kfree(buf);
> + else
> + unmap_sysmem(buf);
> +
> + if (ret) {
> + printf("%s on %s failed with error %d\n",
> + read ? "Read" : "Write", mtd->name, ret);
> + return CMD_RET_FAILURE;
> + }
> +
> + } else if (!strcmp(cmd, "erase")) {
> + bool scrub = strstr(cmd, ".dontskipbad");
> + struct erase_info erase_op = {};
> + u64 off, len;
> + int ret;
> +
> + off = argc > 0 ? simple_strtoul(argv[0], NULL, 16) : 0;
> + len = argc > 1 ? simple_strtoul(argv[1], NULL, 16) : mtd->size;
> +
> + if (!mtd_is_aligned_with_block_size(mtd, off)) {
> + printf("Offset not aligned with a block (0x%x)\n",
> + mtd->erasesize);
> + return CMD_RET_FAILURE;
> + }
> +
> + if (!mtd_is_aligned_with_block_size(mtd, len)) {
> + printf("Size not a multiple of a block (0x%x)\n",
> + mtd->erasesize);
> + return CMD_RET_FAILURE;
> + }
> +
> + printf("Erasing 0x%08llx ... 0x%08llx (%d eraseblock(s))\n",
> + off, off + len - 1, mtd_div_by_eb(len, mtd));
> +
> + erase_op.mtd = mtd;
> + erase_op.addr = off;
> + erase_op.len = len;
> + erase_op.scrub = scrub;
> +
> + while (erase_op.len) {
> + ret = mtd_erase(mtd, &erase_op);
> +
> + /* Abort if its not a bad block error */
> + if (ret != -EIO)
> + break;
> +
> + printf("Skipping bad block at 0x%08llx\n",
> + erase_op.fail_addr);
> +
> + /* Skip bad block and continue behind it */
> + erase_op.len -= erase_op.fail_addr - erase_op.addr;
> + erase_op.len -= mtd->erasesize;
> + erase_op.addr = erase_op.fail_addr + mtd->erasesize;
> + }
> +
> + if (ret && ret != -EIO)
> + return CMD_RET_FAILURE;
> + } else if (!strcmp(cmd, "bad")) {
> + loff_t off;
> +
> + if (!mtd_can_have_bb(mtd)) {
> + printf("Only NAND-based devices can have bad blocks\n");
> + return CMD_RET_SUCCESS;
> + }
> +
> + printf("MTD device %s bad blocks list:\n", mtd->name);
> + for (off = 0; off < mtd->size; off += mtd->erasesize)
> + if (mtd_block_isbad(mtd, off))
> + printf("\t0x%08llx\n", off);
> + } else {
> + return CMD_RET_USAGE;
> + }
> +
> + return CMD_RET_SUCCESS;
> +}
> +
> +static char mtd_help_text[] =
> +#ifdef CONFIG_SYS_LONGHELP
> + "- generic operations on memory technology devices\n\n"
> + "mtd list\n"
> + "mtd read[.raw][.oob] <name> <addr> [<off> [<size>]]\n"
> + "mtd dump[.raw][.oob] <name> [<off> [<size>]]\n"
> + "mtd write[.raw][.oob][.dontskipff] <name> <addr> [<off> [<size>]]\n"
> + "mtd erase[.dontskipbad] <name> [<off> [<size>]]\n"
> + "\n"
> + "Specific functions:\n"
> + "mtd bad <name>\n"
> + "\n"
> + "With:\n"
> + "\t<name>: NAND partition/chip name\n"
> + "\t<addr>: user address from/to which data will be retrieved/stored\n"
> + "\t<off>: offset in <name> in bytes (default: start of the part)\n"
> + "\t\t* must be block-aligned for erase\n"
> + "\t\t* must be page-aligned otherwise\n"
> + "\t<size>: length of the operation in bytes (default: the entire device)\n"
> + "\t\t* must be a multiple of a block for erase\n"
> + "\t\t* must be a multiple of a page otherwise (special case: default is a page with dump)\n"
> + "\n"
> + "The .dontskipff option forces writing empty pages, don't use it if unsure.\n"
> +#endif
> + "";
> +
> +U_BOOT_CMD(mtd, 10, 1, do_mtd, "MTD utils", mtd_help_text);
> diff --git a/drivers/mtd/Makefile b/drivers/mtd/Makefile
> index cdf515256a..22ceda93c0 100644
> --- a/drivers/mtd/Makefile
> +++ b/drivers/mtd/Makefile
> @@ -3,7 +3,7 @@
> # (C) Copyright 2000-2007
> # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
>
> -ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)))
> +ifneq (,$(findstring y,$(CONFIG_MTD_DEVICE)$(CONFIG_CMD_NAND)$(CONFIG_CMD_ONENAND)$(CONFIG_CMD_SF)$(CONFIG_CMD_MTD)))
> obj-y += mtdcore.o mtd_uboot.o
> endif
> obj-$(CONFIG_MTD) += mtd-uclass.o
> diff --git a/drivers/mtd/mtd_uboot.c b/drivers/mtd/mtd_uboot.c
> index 8d7e7890b7..6a211d52ff 100644
> --- a/drivers/mtd/mtd_uboot.c
> +++ b/drivers/mtd/mtd_uboot.c
> @@ -4,8 +4,15 @@
> * Heiko Schocher, DENX Software Engineering, hs at denx.de.
> */
> #include <common.h>
> +#include <dm/device.h>
> +#include <dm/uclass-internal.h>
> +#include <jffs2/jffs2.h> /* LEGACY */
> #include <linux/mtd/mtd.h>
> -#include <jffs2/jffs2.h> /* Legacy */
> +#include <linux/mtd/partitions.h>
> +#include <mtd.h>
> +
> +#define MTD_NAME_MAX_LEN 20
> +
>
> /**
> * mtd_search_alternate_name - Search an alternate name for @mtdname thanks to
> @@ -68,6 +75,158 @@ int mtd_search_alternate_name(const char *mtdname, char *altname,
> return -EINVAL;
> }
>
> +#if IS_ENABLED(CONFIG_MTD)
> +static void mtd_probe_uclass_mtd_devs(void)
> +{
> + struct udevice *dev;
> + int idx = 0;
> +
> + /* Probe devices with DM compliant drivers */
> + while (!uclass_find_device(UCLASS_MTD, idx, &dev) && dev) {
> + mtd_probe(dev);
> + idx++;
> + }
> +}
> +#else
> +static void mtd_probe_uclass_mtd_devs(void) { }
> +#endif
> +
> +#if defined(CONFIG_MTD_PARTITIONS)
> +int mtd_probe_devices(void)
> +{
> + static char *old_mtdparts;
> + static char *old_mtdids;
> + const char *mtdparts = env_get("mtdparts");
> + const char *mtdids = env_get("mtdids");
> + bool remaining_partitions = true;
> + struct mtd_info *mtd;
> +
> + mtd_probe_uclass_mtd_devs();
> +
> + /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> + if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
> + return 0;
> +
> + /* Update the local copy of mtdparts */
> + free(old_mtdparts);
> + free(old_mtdids);
> + old_mtdparts = strdup(mtdparts);
> + old_mtdids = strdup(mtdids);
> +
> + /* If at least one partition is still in use, do not delete anything */
> + mtd_for_each_device(mtd) {
> + if (mtd->usecount) {
> + printf("Partition \"%s\" already in use, aborting\n",
> + mtd->name);
> + return -EACCES;
> + }
> + }
> +
> + /*
> + * Everything looks clear, remove all partitions. It is not safe to
> + * remove entries from the mtd_for_each_device loop as it uses idr
> + * indexes and the partitions removal is done in bulk (all partitions of
> + * one device at the same time), so break and iterate from start each
> + * time a new partition is found and deleted.
> + */
> + while (remaining_partitions) {
> + remaining_partitions = false;
> + mtd_for_each_device(mtd) {
> + if (!mtd_is_partition(mtd) && mtd_has_partitions(mtd)) {
> + del_mtd_partitions(mtd);
> + remaining_partitions = true;
> + break;
> + }
> + }
> + }
> +
> + /* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
> + if (strstr(mtdparts, "mtdparts="))
> + mtdparts += 9;
> +
> + /* For each MTD device in mtdparts */
> + while (mtdparts[0] != '\0') {
> + char mtd_name[MTD_NAME_MAX_LEN], *colon;
> + struct mtd_partition *parts;
> + int mtd_name_len, nparts;
> + int ret;
> +
> + colon = strchr(mtdparts, ':');
> + if (!colon) {
> + printf("Wrong mtdparts: %s\n", mtdparts);
> + return -EINVAL;
> + }
> +
> + mtd_name_len = colon - mtdparts;
> + strncpy(mtd_name, mtdparts, mtd_name_len);
> + mtd_name[mtd_name_len] = '\0';
> + /* Move the pointer forward (including the ':') */
> + mtdparts += mtd_name_len + 1;
> + mtd = get_mtd_device_nm(mtd_name);
> + if (IS_ERR_OR_NULL(mtd)) {
> + char linux_name[MTD_NAME_MAX_LEN];
> +
> + /*
> + * The MTD device named "mtd_name" does not exist. Try
> + * to find a correspondance with an MTD device having
> + * the same type and number as defined in the mtdids.
> + */
> + debug("No device named %s\n", mtd_name);
> + ret = mtd_search_alternate_name(mtd_name, linux_name,
> + MTD_NAME_MAX_LEN);
> + if (!ret)
> + mtd = get_mtd_device_nm(linux_name);
> +
> + /*
> + * If no device could be found, move the mtdparts
> + * pointer forward until the next set of partitions.
> + */
> + if (ret || IS_ERR_OR_NULL(mtd)) {
> + printf("Could not find a valid device for %s\n",
> + mtd_name);
> + mtdparts = strchr(mtdparts, ';');
> + if (mtdparts)
> + mtdparts++;
> +
> + continue;
> + }
> + }
> +
> + /*
> + * Parse the MTD device partitions. It will update the mtdparts
> + * pointer, create an array of parts (that must be freed), and
> + * return the number of partition structures in the array.
> + */
> + ret = mtd_parse_partitions(mtd, &mtdparts, &parts, &nparts);
> + if (ret) {
> + printf("Could not parse device %s\n", mtd->name);
> + put_mtd_device(mtd);
> + return -EINVAL;
> + }
> +
> + if (!nparts)
> + continue;
> +
> + /* Create the new MTD partitions */
> + add_mtd_partitions(mtd, parts, nparts);
> +
> + /* Free the structures allocated during the parsing */
> + mtd_free_parsed_partitions(parts, nparts);
> +
> + put_mtd_device(mtd);
> + }
> +
> + return 0;
> +}
> +#else
> +int mtd_probe_devices(void)
> +{
> + mtd_probe_uclass_mtd_devs();
> +
> + return 0;
> +}
> +#endif /* defined(CONFIG_MTD_PARTITIONS) */
> +
> /* Legacy */
>
> static int get_part(const char *partname, int *idx, loff_t *off, loff_t *size,
> diff --git a/include/mtd.h b/include/mtd.h
> index 6e6da3002f..011f26b3e1 100644
> --- a/include/mtd.h
> +++ b/include/mtd.h
> @@ -8,6 +8,9 @@
>
> #include <linux/mtd/mtd.h>
>
> +struct udevice;
> +
> +#if defined(CONFIG_DM)
> /*
> * Get mtd_info structure of the dev, which is stored as uclass private.
> *
> @@ -20,5 +23,18 @@ static inline struct mtd_info *mtd_get_info(struct udevice *dev)
> }
>
> int mtd_probe(struct udevice *dev);
> +#else
> +static inline struct mtd_info *mtd_get_info(struct udevice *dev)
> +{
> + return NULL;
> +}
> +
> +static inline int mtd_probe(struct udevice *dev)
> +{
> + return 0;
> +}
> +#endif
> +
> +int mtd_probe_devices(void);
>
> #endif /* _MTD_H_ */
> --
> 2.17.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-03 12:35 ` Adam Ford
@ 2018-10-03 12:42 ` Miquel Raynal
2018-10-03 12:47 ` Adam Ford
0 siblings, 1 reply; 25+ messages in thread
From: Miquel Raynal @ 2018-10-03 12:42 UTC (permalink / raw)
To: u-boot
Hi Adam,
Adam Ford <aford173@gmail.com> wrote on Wed, 3 Oct 2018 07:35:15 -0500:
> On Mon, Oct 1, 2018 at 8:48 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > There should not be a 'nand' command, a 'sf' command and certainly not
> > a new 'spi-nand' command. Write a 'mtd' command instead to manage all
> > MTD devices/partitions at once. This should be the preferred way to
> > access any MTD device.
>
> What is the expected behavior when I type 'mtd list' on my omap37
> board, it just hangs.
What do you mean "hangs", does U-Boot crashes? Or is it really hanging
with no more on the console? Can you Ctrl-C to cancel the command or is
it really stuck?
>
> I can use the nand read/write functions and mtdparts lists the
> partitions, so I know nand works. My defconfig
> lists the partitions, so if we're not supposed to use mtdparts, where
> I do store the partition information?
You are not supposed to use the mtdpart _command_, but the mtdparts
_variable_ must be used in order to declare the partitions.
>
> I intentionally removed it from the device tree a while ago, because
> U-Boot was passing the partition info to Linux.
Indeed, that's his primary role.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-03 12:42 ` Miquel Raynal
@ 2018-10-03 12:47 ` Adam Ford
2018-10-03 12:57 ` Miquel Raynal
0 siblings, 1 reply; 25+ messages in thread
From: Adam Ford @ 2018-10-03 12:47 UTC (permalink / raw)
To: u-boot
On Wed, Oct 3, 2018 at 7:43 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Adam,
>
> Adam Ford <aford173@gmail.com> wrote on Wed, 3 Oct 2018 07:35:15 -0500:
>
> > On Mon, Oct 1, 2018 at 8:48 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > There should not be a 'nand' command, a 'sf' command and certainly not
> > > a new 'spi-nand' command. Write a 'mtd' command instead to manage all
> > > MTD devices/partitions at once. This should be the preferred way to
> > > access any MTD device.
> >
> > What is the expected behavior when I type 'mtd list' on my omap37
> > board, it just hangs.
>
> What do you mean "hangs", does U-Boot crashes? Or is it really hanging
> with no more on the console? Can you Ctrl-C to cancel the command or is
> it really stuck?
It's really stuck
U-Boot 2018.11-rc1-00636-g592cd5defd (Oct 03 2018 - 07:28:27 -0500)
OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 800 MHz
Model: LogicPD Zoom OMAP3 Development Kit
Logic DM37x/OMAP35x reference board + LPDDR/NAND
DRAM: 256 MiB
NAND: 512 MiB
MMC: OMAP SD/MMC: 0
Loading Environment from NAND... OK
OMAP die ID: 155000029ff800000168301018021018
Board: DM37xx Torpedo
Net: smc911x-0
Hit any key to stop autoboot: 0
OMAP Logic # mtd list
Control-C does nothing.
>
> >
> > I can use the nand read/write functions and mtdparts lists the
> > partitions, so I know nand works. My defconfig
> > lists the partitions, so if we're not supposed to use mtdparts, where
> > I do store the partition information?
>
> You are not supposed to use the mtdpart _command_, but the mtdparts
> _variable_ must be used in order to declare the partitions.
OK. If I can get MTD working, I'll work to remove the other commands
like NAND and MTDPARTS
>
> >
> > I intentionally removed it from the device tree a while ago, because
> > U-Boot was passing the partition info to Linux.
>
> Indeed, that's his primary role.
OK, I just want to make sure I'm understanding it correctly.
thanks,
adam
>
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-03 12:47 ` Adam Ford
@ 2018-10-03 12:57 ` Miquel Raynal
2018-10-03 13:35 ` Miquel Raynal
0 siblings, 1 reply; 25+ messages in thread
From: Miquel Raynal @ 2018-10-03 12:57 UTC (permalink / raw)
To: u-boot
Hi Adam,
Adam Ford <aford173@gmail.com> wrote on Wed, 3 Oct 2018 07:47:25 -0500:
> On Wed, Oct 3, 2018 at 7:43 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Adam,
> >
> > Adam Ford <aford173@gmail.com> wrote on Wed, 3 Oct 2018 07:35:15 -0500:
> >
> > > On Mon, Oct 1, 2018 at 8:48 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > There should not be a 'nand' command, a 'sf' command and certainly not
> > > > a new 'spi-nand' command. Write a 'mtd' command instead to manage all
> > > > MTD devices/partitions at once. This should be the preferred way to
> > > > access any MTD device.
> > >
> > > What is the expected behavior when I type 'mtd list' on my omap37
> > > board, it just hangs.
> >
> > What do you mean "hangs", does U-Boot crashes? Or is it really hanging
> > with no more on the console? Can you Ctrl-C to cancel the command or is
> > it really stuck?
>
> It's really stuck
>
> U-Boot 2018.11-rc1-00636-g592cd5defd (Oct 03 2018 - 07:28:27 -0500)
>
> OMAP3630/3730-GP ES1.2, CPU-OPP2, L3-200MHz, Max CPU Clock 800 MHz
> Model: LogicPD Zoom OMAP3 Development Kit
> Logic DM37x/OMAP35x reference board + LPDDR/NAND
> DRAM: 256 MiB
> NAND: 512 MiB
> MMC: OMAP SD/MMC: 0
> Loading Environment from NAND... OK
> OMAP die ID: 155000029ff800000168301018021018
> Board: DM37xx Torpedo
> Net: smc911x-0
> Hit any key to stop autoboot: 0
> OMAP Logic # mtd list
>
> Control-C does nothing.
>
>
> >
> > >
> > > I can use the nand read/write functions and mtdparts lists the
> > > partitions, so I know nand works. My defconfig
> > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > I do store the partition information?
> >
> > You are not supposed to use the mtdpart _command_, but the mtdparts
> > _variable_ must be used in order to declare the partitions.
>
> OK. If I can get MTD working, I'll work to remove the other commands
> like NAND and MTDPARTS
As of today, the process of migration is not entirely finished to DM
and you might still need to issue *first* a "nand probe" to register
the device operations.
For the hang, could you check the while (remaining_partitions) loop in
drivers/mtd/mtd_uboot.c:mtd_probe_devices()? Otherwise if you have some
time you may add more traces to track down where it hangs?
>
> >
> > >
> > > I intentionally removed it from the device tree a while ago, because
> > > U-Boot was passing the partition info to Linux.
> >
> > Indeed, that's his primary role.
>
> OK, I just want to make sure I'm understanding it correctly.
Sure, no pb!
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-03 12:57 ` Miquel Raynal
@ 2018-10-03 13:35 ` Miquel Raynal
2018-10-03 13:41 ` Adam Ford
0 siblings, 1 reply; 25+ messages in thread
From: Miquel Raynal @ 2018-10-03 13:35 UTC (permalink / raw)
To: u-boot
Hi Adam,
> >
> > >
> > > >
> > > > I can use the nand read/write functions and mtdparts lists the
> > > > partitions, so I know nand works. My defconfig
> > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > I do store the partition information?
> > >
> > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > _variable_ must be used in order to declare the partitions.
> >
> > OK. If I can get MTD working, I'll work to remove the other commands
> > like NAND and MTDPARTS
>
> As of today, the process of migration is not entirely finished to DM
> and you might still need to issue *first* a "nand probe" to register
> the device operations.
Mmmh there is no nand probe actually, for raw nands like the one you
have it should work out of the box.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-03 13:35 ` Miquel Raynal
@ 2018-10-03 13:41 ` Adam Ford
2018-10-08 16:13 ` Adam Ford
0 siblings, 1 reply; 25+ messages in thread
From: Adam Ford @ 2018-10-03 13:41 UTC (permalink / raw)
To: u-boot
On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> Hi Adam,
>
> > >
> > > >
> > > > >
> > > > > I can use the nand read/write functions and mtdparts lists the
> > > > > partitions, so I know nand works. My defconfig
> > > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > > I do store the partition information?
> > > >
> > > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > > _variable_ must be used in order to declare the partitions.
> > >
> > > OK. If I can get MTD working, I'll work to remove the other commands
> > > like NAND and MTDPARTS
> >
> > As of today, the process of migration is not entirely finished to DM
> > and you might still need to issue *first* a "nand probe" to register
> > the device operations.
>
> Mmmh there is no nand probe actually, for raw nands like the one you
> have it should work out of the box.
I have a few tasks to do today for work, but I'll try to do some
testing as you suggested this week and possibly later tonight.
adam
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-03 13:41 ` Adam Ford
@ 2018-10-08 16:13 ` Adam Ford
2018-10-08 16:28 ` Boris Brezillon
0 siblings, 1 reply; 25+ messages in thread
From: Adam Ford @ 2018-10-08 16:13 UTC (permalink / raw)
To: u-boot
On Wed, Oct 3, 2018 at 8:41 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Adam,
> >
> > > >
> > > > >
> > > > > >
> > > > > > I can use the nand read/write functions and mtdparts lists the
> > > > > > partitions, so I know nand works. My defconfig
> > > > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > > > I do store the partition information?
> > > > >
> > > > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > > > _variable_ must be used in order to declare the partitions.
> > > >
> > > > OK. If I can get MTD working, I'll work to remove the other commands
> > > > like NAND and MTDPARTS
> > >
> > > As of today, the process of migration is not entirely finished to DM
> > > and you might still need to issue *first* a "nand probe" to register
> > > the device operations.
> >
> > Mmmh there is no nand probe actually, for raw nands like the one you
> > have it should work out of the box.
i haven't actually insterted debug code yet, but I started a quick
code review. There is a function called 'mtd_probe_uclass_mtd_devs'
which states it will probe with DM compliant drivers.
I am thinking the nand and/or GPMC drivers are not yet DM compliant
yet. The DM tree doesn't list any of the MTD parts.
Is it save to assume it just wont' work until the drives are DM
compliant, or is this driver designed to play with the lower-level
drivers as-is?
adam
>
> I have a few tasks to do today for work, but I'll try to do some
> testing as you suggested this week and possibly later tonight.
>
> adam
> >
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-08 16:13 ` Adam Ford
@ 2018-10-08 16:28 ` Boris Brezillon
2018-10-08 16:52 ` Adam Ford
0 siblings, 1 reply; 25+ messages in thread
From: Boris Brezillon @ 2018-10-08 16:28 UTC (permalink / raw)
To: u-boot
Hi Adam,
On Mon, 8 Oct 2018 11:13:40 -0500
Adam Ford <aford173@gmail.com> wrote:
> On Wed, Oct 3, 2018 at 8:41 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Adam,
> > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > I can use the nand read/write functions and mtdparts lists the
> > > > > > > partitions, so I know nand works. My defconfig
> > > > > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > > > > I do store the partition information?
> > > > > >
> > > > > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > > > > _variable_ must be used in order to declare the partitions.
> > > > >
> > > > > OK. If I can get MTD working, I'll work to remove the other commands
> > > > > like NAND and MTDPARTS
> > > >
> > > > As of today, the process of migration is not entirely finished to DM
> > > > and you might still need to issue *first* a "nand probe" to register
> > > > the device operations.
> > >
> > > Mmmh there is no nand probe actually, for raw nands like the one you
> > > have it should work out of the box.
>
> i haven't actually insterted debug code yet, but I started a quick
> code review. There is a function called 'mtd_probe_uclass_mtd_devs'
> which states it will probe with DM compliant drivers.
> I am thinking the nand and/or GPMC drivers are not yet DM compliant
> yet. The DM tree doesn't list any of the MTD parts.
>
> Is it save to assume it just wont' work until the drives are DM
> compliant, or is this driver designed to play with the lower-level
> drivers as-is?
The whole point of this rework was to allow both old (non-DM compliant)
and new (DM compliant) drivers to be exposed the same way to the upper
layers. If it doesn't work for regular NAND drivers it's a bug, and we
should fix it.
Regards,
Boris
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-08 16:28 ` Boris Brezillon
@ 2018-10-08 16:52 ` Adam Ford
2018-10-08 16:58 ` Adam Ford
0 siblings, 1 reply; 25+ messages in thread
From: Adam Ford @ 2018-10-08 16:52 UTC (permalink / raw)
To: u-boot
On Mon, Oct 8, 2018 at 11:28 AM Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
>
> Hi Adam,
>
> On Mon, 8 Oct 2018 11:13:40 -0500
> Adam Ford <aford173@gmail.com> wrote:
>
> > On Wed, Oct 3, 2018 at 8:41 AM Adam Ford <aford173@gmail.com> wrote:
> > >
> > > On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >
> > > > Hi Adam,
> > > >
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > I can use the nand read/write functions and mtdparts lists the
> > > > > > > > partitions, so I know nand works. My defconfig
> > > > > > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > > > > > I do store the partition information?
> > > > > > >
> > > > > > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > > > > > _variable_ must be used in order to declare the partitions.
> > > > > >
> > > > > > OK. If I can get MTD working, I'll work to remove the other commands
> > > > > > like NAND and MTDPARTS
> > > > >
> > > > > As of today, the process of migration is not entirely finished to DM
> > > > > and you might still need to issue *first* a "nand probe" to register
> > > > > the device operations.
> > > >
> > > > Mmmh there is no nand probe actually, for raw nands like the one you
> > > > have it should work out of the box.
> >
> > i haven't actually insterted debug code yet, but I started a quick
> > code review. There is a function called 'mtd_probe_uclass_mtd_devs'
> > which states it will probe with DM compliant drivers.
> > I am thinking the nand and/or GPMC drivers are not yet DM compliant
> > yet. The DM tree doesn't list any of the MTD parts.
> >
> > Is it save to assume it just wont' work until the drives are DM
> > compliant, or is this driver designed to play with the lower-level
> > drivers as-is?
>
> The whole point of this rework was to allow both old (non-DM compliant)
> and new (DM compliant) drivers to be exposed the same way to the upper
> layers. If it doesn't work for regular NAND drivers it's a bug, and we
> should fix it.
That's what I assummed, so I kept trying to debug, but I wanted to
make sure I asked the question, because I've been burned by
assumptions before.
I narrowed the hang down to the following in driver/mtd/mtd_uboot,
mtd_probe_devices
int mtd_probe_devices(void)
{
...
/* Check if mtdparts/mtdids changed since last call, otherwise: exit */
if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
return 0;
}
The above function neither returns nor exits. I added a brace and a
printf just before the return, and the printf never appears.
Only when I put #if 0 around the 'if' statement above, and 'mtd list'
returned and did a dump.
I tried initializing
static char *old_mtdparts = '\0';
static char *old_mtdids = '\0';
and
static char *old_mtdparts = NULL;
static char *old_mtdids = NULL;
Neither fixed the hang.
adam
The functioning dump (with the ifdef) is as follows:
List of MTD devices:
* nand0
- type: NAND flash
- block size: 0x20000 bytes
- min I/O: 0x800 bytes
- OOB size: 64 bytes
- OOB available: 10 bytes
- ECC strength: 8 bits
- ECC step size: 512 bytes
- bitflip threshold: 6 bits
- 0x000000000000-0x000020000000 : "nand0"
- 0x000000000000-0x000000080000 : "MLO"
- 0x000000080000-0x000000240000 : "u-boot"
- 0x000000240000-0x000000260000 : "spl-os"
- 0x000000260000-0x000000280000 : "u-boot-env"
- 0x000000280000-0x000000880000 : "kernel"
- 0x000000880000-0x000020000000 : "fs"
>
> Regards,
>
> Boris
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-08 16:52 ` Adam Ford
@ 2018-10-08 16:58 ` Adam Ford
2018-10-08 17:27 ` Boris Brezillon
0 siblings, 1 reply; 25+ messages in thread
From: Adam Ford @ 2018-10-08 16:58 UTC (permalink / raw)
To: u-boot
On Mon, Oct 8, 2018 at 11:52 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Oct 8, 2018 at 11:28 AM Boris Brezillon
> <boris.brezillon@bootlin.com> wrote:
> >
> > Hi Adam,
> >
> > On Mon, 8 Oct 2018 11:13:40 -0500
> > Adam Ford <aford173@gmail.com> wrote:
> >
> > > On Wed, Oct 3, 2018 at 8:41 AM Adam Ford <aford173@gmail.com> wrote:
> > > >
> > > > On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > >
> > > > > Hi Adam,
> > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > I can use the nand read/write functions and mtdparts lists the
> > > > > > > > > partitions, so I know nand works. My defconfig
> > > > > > > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > > > > > > I do store the partition information?
> > > > > > > >
> > > > > > > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > > > > > > _variable_ must be used in order to declare the partitions.
> > > > > > >
> > > > > > > OK. If I can get MTD working, I'll work to remove the other commands
> > > > > > > like NAND and MTDPARTS
> > > > > >
> > > > > > As of today, the process of migration is not entirely finished to DM
> > > > > > and you might still need to issue *first* a "nand probe" to register
> > > > > > the device operations.
> > > > >
> > > > > Mmmh there is no nand probe actually, for raw nands like the one you
> > > > > have it should work out of the box.
> > >
> > > i haven't actually insterted debug code yet, but I started a quick
> > > code review. There is a function called 'mtd_probe_uclass_mtd_devs'
> > > which states it will probe with DM compliant drivers.
> > > I am thinking the nand and/or GPMC drivers are not yet DM compliant
> > > yet. The DM tree doesn't list any of the MTD parts.
> > >
> > > Is it save to assume it just wont' work until the drives are DM
> > > compliant, or is this driver designed to play with the lower-level
> > > drivers as-is?
> >
> > The whole point of this rework was to allow both old (non-DM compliant)
> > and new (DM compliant) drivers to be exposed the same way to the upper
> > layers. If it doesn't work for regular NAND drivers it's a bug, and we
> > should fix it.
>
> That's what I assummed, so I kept trying to debug, but I wanted to
> make sure I asked the question, because I've been burned by
> assumptions before.
>
> I narrowed the hang down to the following in driver/mtd/mtd_uboot,
> mtd_probe_devices
>
> int mtd_probe_devices(void)
> {
> ...
> /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
> return 0;
> }
>
> The above function neither returns nor exits. I added a brace and a
> printf just before the return, and the printf never appears.
> Only when I put #if 0 around the 'if' statement above, and 'mtd list'
> returned and did a dump.
>
> I tried initializing
> static char *old_mtdparts = '\0';
> static char *old_mtdids = '\0';
>
> and
>
> static char *old_mtdparts = NULL;
> static char *old_mtdids = NULL;
>
> Neither fixed the hang.
>
> adam
>
> The functioning dump (with the ifdef) is as follows:
>
> List of MTD devices:
> * nand0
> - type: NAND flash
> - block size: 0x20000 bytes
> - min I/O: 0x800 bytes
> - OOB size: 64 bytes
> - OOB available: 10 bytes
> - ECC strength: 8 bits
> - ECC step size: 512 bytes
> - bitflip threshold: 6 bits
> - 0x000000000000-0x000020000000 : "nand0"
> - 0x000000000000-0x000000080000 : "MLO"
> - 0x000000080000-0x000000240000 : "u-boot"
> - 0x000000240000-0x000000260000 : "spl-os"
> - 0x000000260000-0x000000280000 : "u-boot-env"
> - 0x000000280000-0x000000880000 : "kernel"
> - 0x000000880000-0x000020000000 : "fs"
>
>
>
> >
Lastly, I noticed that I am not able to de-select CMD_MTDPARTS from my
config because CMD_UBI (which I use) automatically selects
CMD_MTDPARTS. I only mention it because I know you're trying to
deprecate CMD_MTDPARTS.
adam
> > Regards,
> >
> > Boris
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-08 16:58 ` Adam Ford
@ 2018-10-08 17:27 ` Boris Brezillon
0 siblings, 0 replies; 25+ messages in thread
From: Boris Brezillon @ 2018-10-08 17:27 UTC (permalink / raw)
To: u-boot
On Mon, 8 Oct 2018 11:58:40 -0500
Adam Ford <aford173@gmail.com> wrote:
> On Mon, Oct 8, 2018 at 11:52 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Mon, Oct 8, 2018 at 11:28 AM Boris Brezillon
> > <boris.brezillon@bootlin.com> wrote:
> > >
> > > Hi Adam,
> > >
> > > On Mon, 8 Oct 2018 11:13:40 -0500
> > > Adam Ford <aford173@gmail.com> wrote:
> > >
> > > > On Wed, Oct 3, 2018 at 8:41 AM Adam Ford <aford173@gmail.com> wrote:
> > > > >
> > > > > On Wed, Oct 3, 2018 at 8:35 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > > > >
> > > > > > Hi Adam,
> > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I can use the nand read/write functions and mtdparts lists the
> > > > > > > > > > partitions, so I know nand works. My defconfig
> > > > > > > > > > lists the partitions, so if we're not supposed to use mtdparts, where
> > > > > > > > > > I do store the partition information?
> > > > > > > > >
> > > > > > > > > You are not supposed to use the mtdpart _command_, but the mtdparts
> > > > > > > > > _variable_ must be used in order to declare the partitions.
> > > > > > > >
> > > > > > > > OK. If I can get MTD working, I'll work to remove the other commands
> > > > > > > > like NAND and MTDPARTS
> > > > > > >
> > > > > > > As of today, the process of migration is not entirely finished to DM
> > > > > > > and you might still need to issue *first* a "nand probe" to register
> > > > > > > the device operations.
> > > > > >
> > > > > > Mmmh there is no nand probe actually, for raw nands like the one you
> > > > > > have it should work out of the box.
> > > >
> > > > i haven't actually insterted debug code yet, but I started a quick
> > > > code review. There is a function called 'mtd_probe_uclass_mtd_devs'
> > > > which states it will probe with DM compliant drivers.
> > > > I am thinking the nand and/or GPMC drivers are not yet DM compliant
> > > > yet. The DM tree doesn't list any of the MTD parts.
> > > >
> > > > Is it save to assume it just wont' work until the drives are DM
> > > > compliant, or is this driver designed to play with the lower-level
> > > > drivers as-is?
> > >
> > > The whole point of this rework was to allow both old (non-DM compliant)
> > > and new (DM compliant) drivers to be exposed the same way to the upper
> > > layers. If it doesn't work for regular NAND drivers it's a bug, and we
> > > should fix it.
> >
> > That's what I assummed, so I kept trying to debug, but I wanted to
> > make sure I asked the question, because I've been burned by
> > assumptions before.
> >
> > I narrowed the hang down to the following in driver/mtd/mtd_uboot,
> > mtd_probe_devices
> >
> > int mtd_probe_devices(void)
> > {
> > ...
> > /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> > if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
Can you try with
if (old_mtdparts && old_mtdids &&
!strcmp(mtdparts, old_mtdparts) &&
!strcmp(mtdids, old_mtdids))
> > return 0;
> > }
> >
> > The above function neither returns nor exits. I added a brace and a
> > printf just before the return, and the printf never appears.
> > Only when I put #if 0 around the 'if' statement above, and 'mtd list'
> > returned and did a dump.
> >
> > I tried initializing
> > static char *old_mtdparts = '\0';
> > static char *old_mtdids = '\0';
> >
> > and
> >
> > static char *old_mtdparts = NULL;
> > static char *old_mtdids = NULL;
> >
> > Neither fixed the hang.
> >
> > adam
> >
> > The functioning dump (with the ifdef) is as follows:
> >
> > List of MTD devices:
> > * nand0
> > - type: NAND flash
> > - block size: 0x20000 bytes
> > - min I/O: 0x800 bytes
> > - OOB size: 64 bytes
> > - OOB available: 10 bytes
> > - ECC strength: 8 bits
> > - ECC step size: 512 bytes
> > - bitflip threshold: 6 bits
> > - 0x000000000000-0x000020000000 : "nand0"
> > - 0x000000000000-0x000000080000 : "MLO"
> > - 0x000000080000-0x000000240000 : "u-boot"
> > - 0x000000240000-0x000000260000 : "spl-os"
> > - 0x000000260000-0x000000280000 : "u-boot-env"
> > - 0x000000280000-0x000000880000 : "kernel"
> > - 0x000000880000-0x000020000000 : "fs"
> >
> >
> >
> > >
>
> Lastly, I noticed that I am not able to de-select CMD_MTDPARTS from my
> config because CMD_UBI (which I use) automatically selects
> CMD_MTDPARTS. I only mention it because I know you're trying to
> deprecate CMD_MTDPARTS.
>
> adam
>
> > > Regards,
> > >
> > > Boris
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-01 13:43 ` [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command Miquel Raynal
2018-10-01 16:19 ` Jagan Teki
2018-10-03 12:35 ` Adam Ford
@ 2018-10-08 17:46 ` Boris Brezillon
2018-10-08 18:26 ` Adam Ford
2018-10-08 19:07 ` Thomas Petazzoni
2 siblings, 2 replies; 25+ messages in thread
From: Boris Brezillon @ 2018-10-08 17:46 UTC (permalink / raw)
To: u-boot
On Mon, 1 Oct 2018 15:43:29 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> +#if defined(CONFIG_MTD_PARTITIONS)
> +int mtd_probe_devices(void)
> +{
> + static char *old_mtdparts;
> + static char *old_mtdids;
> + const char *mtdparts = env_get("mtdparts");
> + const char *mtdids = env_get("mtdids");
> + bool remaining_partitions = true;
> + struct mtd_info *mtd;
> +
> + mtd_probe_uclass_mtd_devs();
> +
> + /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> + if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
> + return 0;
Should be:
if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
(mtdparts && old_mtdparts && mtdids && old_mtdids &&
!strcmp(mtdparts, old_mtdparts) &&
!strcmp(mtdids, old_mtdids)))
return 0;
> +
> + /* Update the local copy of mtdparts */
> + free(old_mtdparts);
> + free(old_mtdids);
> + old_mtdparts = strdup(mtdparts);
> + old_mtdids = strdup(mtdids);
> +
> + /* If at least one partition is still in use, do not delete anything */
> + mtd_for_each_device(mtd) {
> + if (mtd->usecount) {
> + printf("Partition \"%s\" already in use, aborting\n",
> + mtd->name);
> + return -EACCES;
> + }
> + }
> +
> + /*
> + * Everything looks clear, remove all partitions. It is not safe to
> + * remove entries from the mtd_for_each_device loop as it uses idr
> + * indexes and the partitions removal is done in bulk (all partitions of
> + * one device at the same time), so break and iterate from start each
> + * time a new partition is found and deleted.
> + */
> + while (remaining_partitions) {
> + remaining_partitions = false;
> + mtd_for_each_device(mtd) {
> + if (!mtd_is_partition(mtd) && mtd_has_partitions(mtd)) {
> + del_mtd_partitions(mtd);
> + remaining_partitions = true;
> + break;
> + }
> + }
> + }
> +
We should bail out if either mtdparts or mtdids is NULL:
if (!mtdparts || !mtdids)
return 0;
> + /* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
> + if (strstr(mtdparts, "mtdparts="))
> + mtdparts += 9;
> +
> + /* For each MTD device in mtdparts */
> + while (mtdparts[0] != '\0') {
> + char mtd_name[MTD_NAME_MAX_LEN], *colon;
> + struct mtd_partition *parts;
> + int mtd_name_len, nparts;
> + int ret;
> +
> + colon = strchr(mtdparts, ':');
> + if (!colon) {
> + printf("Wrong mtdparts: %s\n", mtdparts);
> + return -EINVAL;
> + }
> +
> + mtd_name_len = colon - mtdparts;
> + strncpy(mtd_name, mtdparts, mtd_name_len);
> + mtd_name[mtd_name_len] = '\0';
> + /* Move the pointer forward (including the ':') */
> + mtdparts += mtd_name_len + 1;
> + mtd = get_mtd_device_nm(mtd_name);
> + if (IS_ERR_OR_NULL(mtd)) {
> + char linux_name[MTD_NAME_MAX_LEN];
> +
> + /*
> + * The MTD device named "mtd_name" does not exist. Try
> + * to find a correspondance with an MTD device having
> + * the same type and number as defined in the mtdids.
> + */
> + debug("No device named %s\n", mtd_name);
> + ret = mtd_search_alternate_name(mtd_name, linux_name,
> + MTD_NAME_MAX_LEN);
> + if (!ret)
> + mtd = get_mtd_device_nm(linux_name);
> +
> + /*
> + * If no device could be found, move the mtdparts
> + * pointer forward until the next set of partitions.
> + */
> + if (ret || IS_ERR_OR_NULL(mtd)) {
> + printf("Could not find a valid device for %s\n",
> + mtd_name);
> + mtdparts = strchr(mtdparts, ';');
> + if (mtdparts)
> + mtdparts++;
> +
> + continue;
> + }
> + }
> +
> + /*
> + * Parse the MTD device partitions. It will update the mtdparts
> + * pointer, create an array of parts (that must be freed), and
> + * return the number of partition structures in the array.
> + */
> + ret = mtd_parse_partitions(mtd, &mtdparts, &parts, &nparts);
> + if (ret) {
> + printf("Could not parse device %s\n", mtd->name);
> + put_mtd_device(mtd);
> + return -EINVAL;
> + }
> +
> + if (!nparts)
> + continue;
> +
> + /* Create the new MTD partitions */
> + add_mtd_partitions(mtd, parts, nparts);
> +
> + /* Free the structures allocated during the parsing */
> + mtd_free_parsed_partitions(parts, nparts);
> +
> + put_mtd_device(mtd);
> + }
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-08 17:46 ` Boris Brezillon
@ 2018-10-08 18:26 ` Adam Ford
2018-10-08 19:07 ` Thomas Petazzoni
1 sibling, 0 replies; 25+ messages in thread
From: Adam Ford @ 2018-10-08 18:26 UTC (permalink / raw)
To: u-boot
On Mon, Oct 8, 2018 at 12:46 PM Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
>
> On Mon, 1 Oct 2018 15:43:29 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > +#if defined(CONFIG_MTD_PARTITIONS)
> > +int mtd_probe_devices(void)
> > +{
> > + static char *old_mtdparts;
> > + static char *old_mtdids;
> > + const char *mtdparts = env_get("mtdparts");
> > + const char *mtdids = env_get("mtdids");
> > + bool remaining_partitions = true;
> > + struct mtd_info *mtd;
> > +
> > + mtd_probe_uclass_mtd_devs();
> > +
> > + /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> > + if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
> > + return 0;
>
> Should be:
>
> if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
> (mtdparts && old_mtdparts && mtdids && old_mtdids &&
> !strcmp(mtdparts, old_mtdparts) &&
> !strcmp(mtdids, old_mtdids)))
> return 0;
>
Thanks for the suggestion.
I created a patch to change this. I forgot to add the Suggested-by
note. If whomever the maintainer wants to edit the patch, comments,
etc. go ahead. Just know it now works on omap3_logic.
> > +
> > + /* Update the local copy of mtdparts */
> > + free(old_mtdparts);
> > + free(old_mtdids);
> > + old_mtdparts = strdup(mtdparts);
> > + old_mtdids = strdup(mtdids);
> > +
> > + /* If at least one partition is still in use, do not delete anything */
> > + mtd_for_each_device(mtd) {
> > + if (mtd->usecount) {
> > + printf("Partition \"%s\" already in use, aborting\n",
> > + mtd->name);
> > + return -EACCES;
> > + }
> > + }
> > +
> > + /*
> > + * Everything looks clear, remove all partitions. It is not safe to
> > + * remove entries from the mtd_for_each_device loop as it uses idr
> > + * indexes and the partitions removal is done in bulk (all partitions of
> > + * one device at the same time), so break and iterate from start each
> > + * time a new partition is found and deleted.
> > + */
> > + while (remaining_partitions) {
> > + remaining_partitions = false;
> > + mtd_for_each_device(mtd) {
> > + if (!mtd_is_partition(mtd) && mtd_has_partitions(mtd)) {
> > + del_mtd_partitions(mtd);
> > + remaining_partitions = true;
> > + break;
> > + }
> > + }
> > + }
> > +
>
> We should bail out if either mtdparts or mtdids is NULL:
>
> if (!mtdparts || !mtdids)
> return 0;
And add this in the patch as well.
>
> > + /* Start the parsing by ignoring the extra 'mtdparts=' prefix, if any */
> > + if (strstr(mtdparts, "mtdparts="))
> > + mtdparts += 9;
> > +
> > + /* For each MTD device in mtdparts */
> > + while (mtdparts[0] != '\0') {
> > + char mtd_name[MTD_NAME_MAX_LEN], *colon;
> > + struct mtd_partition *parts;
> > + int mtd_name_len, nparts;
> > + int ret;
> > +
> > + colon = strchr(mtdparts, ':');
> > + if (!colon) {
> > + printf("Wrong mtdparts: %s\n", mtdparts);
> > + return -EINVAL;
> > + }
> > +
> > + mtd_name_len = colon - mtdparts;
> > + strncpy(mtd_name, mtdparts, mtd_name_len);
> > + mtd_name[mtd_name_len] = '\0';
> > + /* Move the pointer forward (including the ':') */
> > + mtdparts += mtd_name_len + 1;
> > + mtd = get_mtd_device_nm(mtd_name);
> > + if (IS_ERR_OR_NULL(mtd)) {
> > + char linux_name[MTD_NAME_MAX_LEN];
> > +
> > + /*
> > + * The MTD device named "mtd_name" does not exist. Try
> > + * to find a correspondance with an MTD device having
> > + * the same type and number as defined in the mtdids.
> > + */
> > + debug("No device named %s\n", mtd_name);
> > + ret = mtd_search_alternate_name(mtd_name, linux_name,
> > + MTD_NAME_MAX_LEN);
> > + if (!ret)
> > + mtd = get_mtd_device_nm(linux_name);
> > +
> > + /*
> > + * If no device could be found, move the mtdparts
> > + * pointer forward until the next set of partitions.
> > + */
> > + if (ret || IS_ERR_OR_NULL(mtd)) {
> > + printf("Could not find a valid device for %s\n",
> > + mtd_name);
> > + mtdparts = strchr(mtdparts, ';');
> > + if (mtdparts)
> > + mtdparts++;
> > +
> > + continue;
> > + }
> > + }
> > +
> > + /*
> > + * Parse the MTD device partitions. It will update the mtdparts
> > + * pointer, create an array of parts (that must be freed), and
> > + * return the number of partition structures in the array.
> > + */
> > + ret = mtd_parse_partitions(mtd, &mtdparts, &parts, &nparts);
> > + if (ret) {
> > + printf("Could not parse device %s\n", mtd->name);
> > + put_mtd_device(mtd);
> > + return -EINVAL;
> > + }
> > +
> > + if (!nparts)
> > + continue;
> > +
> > + /* Create the new MTD partitions */
> > + add_mtd_partitions(mtd, parts, nparts);
> > +
> > + /* Free the structures allocated during the parsing */
> > + mtd_free_parsed_partitions(parts, nparts);
> > +
> > + put_mtd_device(mtd);
> > + }
> > +
> > + return 0;
> > +}
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-08 17:46 ` Boris Brezillon
2018-10-08 18:26 ` Adam Ford
@ 2018-10-08 19:07 ` Thomas Petazzoni
2018-10-08 19:14 ` Adam Ford
1 sibling, 1 reply; 25+ messages in thread
From: Thomas Petazzoni @ 2018-10-08 19:07 UTC (permalink / raw)
To: u-boot
Hello,
On Mon, 8 Oct 2018 19:46:27 +0200, Boris Brezillon wrote:
> > + /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> > + if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
> > + return 0;
>
> Should be:
>
> if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
If there's a || here, it means you can split the conditions into to:
if (!mtdparts && !old_mtdparts && !mtdids && !old_mtdids)
return 0;
if (mtdparts && old_mtdparts && mtdids && old_mtdids &&
!strcmp(mtdparts, old_mtdparts) &&
!strcmp(mtdids, old_mtdids)))
return 0;
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command
2018-10-08 19:07 ` Thomas Petazzoni
@ 2018-10-08 19:14 ` Adam Ford
0 siblings, 0 replies; 25+ messages in thread
From: Adam Ford @ 2018-10-08 19:14 UTC (permalink / raw)
To: u-boot
On Mon, Oct 8, 2018 at 2:10 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Mon, 8 Oct 2018 19:46:27 +0200, Boris Brezillon wrote:
>
> > > + /* Check if mtdparts/mtdids changed since last call, otherwise: exit */
> > > + if (!strcmp(mtdparts, old_mtdparts) && !strcmp(mtdids, old_mtdids))
> > > + return 0;
> >
> > Should be:
> >
> > if ((!mtdparts && !old_mtdparts && !mtdids && !old_mtdids) ||
>
> If there's a || here, it means you can split the conditions into to:
>
> if (!mtdparts && !old_mtdparts && !mtdids && !old_mtdids)
> return 0;
>
> if (mtdparts && old_mtdparts && mtdids && old_mtdids &&
> !strcmp(mtdparts, old_mtdparts) &&
> !strcmp(mtdids, old_mtdids)))
> return 0;
>
> Best regards,
Is there a reason, it can't be left the way it was?
adam
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2018-10-08 19:14 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-01 13:43 [U-Boot] [PATCH v13 0/7] SPI-NAND support (third batch) Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 1/7] mtd: uclass: add probe function Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 2/7] mtd: mtdpart: add a generic mtdparts-like parser Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 3/7] mtd: uboot: search for an equivalent MTD name with the mtdids Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 4/7] mtd: mtdpart: implement proper partition handling Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 5/7] cmd: mtd: add 'mtd' command Miquel Raynal
2018-10-01 16:19 ` Jagan Teki
2018-10-01 20:39 ` Miquel Raynal
2018-10-03 12:35 ` Adam Ford
2018-10-03 12:42 ` Miquel Raynal
2018-10-03 12:47 ` Adam Ford
2018-10-03 12:57 ` Miquel Raynal
2018-10-03 13:35 ` Miquel Raynal
2018-10-03 13:41 ` Adam Ford
2018-10-08 16:13 ` Adam Ford
2018-10-08 16:28 ` Boris Brezillon
2018-10-08 16:52 ` Adam Ford
2018-10-08 16:58 ` Adam Ford
2018-10-08 17:27 ` Boris Brezillon
2018-10-08 17:46 ` Boris Brezillon
2018-10-08 18:26 ` Adam Ford
2018-10-08 19:07 ` Thomas Petazzoni
2018-10-08 19:14 ` Adam Ford
2018-10-01 13:43 ` [U-Boot] [PATCH v13 6/7] cmd: ubi: clean the partition handling Miquel Raynal
2018-10-01 13:43 ` [U-Boot] [PATCH v13 7/7] cmd: mtdparts: describe as legacy Miquel Raynal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.