We may cause a memory leak when the @types has more then one parser.
Take the `default_mtd_part_types` for example. The default_mtd_part_types has
two parsers now: `cmdlinepart` and `ofpart`.
Assume the following case:
The kernel command line sets the partitions like:
#gpmi-nand:20m(boot),20m(kernel),1g(rootfs),-(user)
But the devicetree file(such as arch/arm/boot/dts/imx28-evk.dts) also sets
the same partitions as the kernel command line does.
In the current code, the partitions parsed out by the `ofpart` will
overwrite the @pparts which has already set by the `cmdlinepart` parser,
and the the partitions parsed out by the `cmdlinepart` is missed.
A memory leak occurs.
So we should break the code as soon as we parse out the partitions,
In actually, this patch makes a priority order between the parsers.
If one parser has already parsed out the partitions successfully,
it's no need to use another parser anymore.
Signed-off-by: Huang Shijie <shijie8@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
'mtd_get_device_size()' returns the size of the whole MTD device, that is the
mtd_info master size. This will be used by UBI to calculate the maximum number
of bad blocks (MBB) on a MTD device.
Artem: amended the patch a bit.
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
'struct mtd_info' is not modified by 'mtd_is_partition()' so it can be marked
as "const".
Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
The drivers' _read() method, absent an error, returns a non-negative integer
indicating the maximum number of bit errors that were corrected in any one
region comprising an ecc step. MTD returns -EUCLEAN if this is >=
bitflip_threshold, 0 otherwise. If bitflip_threshold is zero, the comparison is
not made since these devices lack ECC and always return zero in the non-error
case (thanks Brian)¹. Note that this is a subtle change to the driver
interface.
This and the preceding patches in this set were tested with ubi on top of the
nandsim and docg4 devices, running the ubi test io_basic from mtd-utils.
¹ http://lists.infradead.org/pipermail/linux-mtd/2012-March/040468.html
Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Acked-by: Brian Norris <computersforpeace@gmail.com>
Ivan Djelic <ivan.djelic@parrot.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
An element 'bitflip_threshold' is added to struct mtd_info, and also exposed as
a read/write variable in sysfs. This will be used to determine whether or not
mtd_read() returns -EUCLEAN or 0 (absent a hard error). If the driver leaves it
as zero, mtd will set it to a default value of ecc_strength.
This v2 adds the line that propagates bitflip_threshold from the master to the
partitions - thanks Ivan¹.
¹ http://lists.infradead.org/pipermail/linux-mtd/2012-April/040900.html
Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Flash device drivers initialize 'ecc_strength' in struct mtd_info, which is the
maximum number of bit errors that can be corrected in one writesize region.
Drivers using the nand interface intitialize 'strength' in struct nand_ecc_ctrl,
which is the maximum number of bit errors that can be corrected in one ecc step.
Nand infrastructure code translates this to 'ecc_strength'.
Also for nand drivers, the nand infrastructure code sets ecc.strength for ecc
modes NAND_ECC_SOFT, NAND_ECC_SOFT_BCH, and NAND_ECC_NONE. It is set in the
driver for all other modes.
Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
This patch reverts a change that may have been mistakenly included with the set
of patches that introduced the new mtd api entry functions. Or perhaps I am
mistaken :)
The problem is in the partition wrapper functions, where the calls to the driver
methods were replaced with calls to the new mtd api functions. This causes the
api function to be called a second time, further down the call stack. This is
not only unnecessary and redundant - because the sanity checking code and (more
restrictive) bounds checks for the partition were done in the first call - but
is potentially problematic and confusing.
For example, the call stack for a call to mtd_read() on a partitioned device
currently looks like this:
mtd_read() gets struct mtd_info for the partition
|
+-> part_read() via the pointer assigned when the partition was created
|
+->mtd_read() this time gets struct mtd_info for the master
|
+->xyz_driver_read() via the pointer asigned by the driver
It seems that this can cause a variety of problems. For example, if you want to
add code to the api function that tests a value in mtd_info that is relevant
only to the partition. Or (in my case) you want the driver to return a value
that may be different from that returned by the mtd api function.
This patch eliminates the second call to the mtd api function. It was tested on
the docg4 nand driver with a subset of the api functions, but I inspected the
rest and don't see any problems.
Signed-off-by: Mike Dunn <mikedunn@newsguy.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Many drivers check whether the partition is R/O and return -EROFS if yes.
Let's stop having duplicated checks and move them to the API functions
instead.
And again a bit of noise - deleted few too sparse newlines, sorry.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
We already verify that offset and length are within the MTD device size
in the MTD API functions. Let's remove the duplicated checks in drivers.
This patch only affects the following API's:
'mtd_erase()'
'mtd_point()'
'mtd_unpoint()'
'mtd_get_unmapped_area()'
'mtd_read()'
'mtd_write()'
'mtd_panic_write()'
'mtd_lock()'
'mtd_unlock()'
'mtd_is_locked()'
'mtd_block_isbad()'
'mtd_block_markbad()'
This patch adds a bit of noise by removing too sparse empty lines, but this is
not too bad.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
The 'mtd_unpoint()' API function should be able to return an error code because
it may fail if you specify incorrect offset. This patch changes this MTD API
function and amends all the drivers correspondingly.
Also return '-EOPNOTSUPP' from 'mtd_unpoint()' when the '->unpoint()' method is
undefined. We do not really need this currently, but this just makes
sense to be consistent with 'mtd_point()'.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
This patch renames all MTD functions by adding a "_" prefix:
mtd->erase -> mtd->_erase
mtd->read_oob -> mtd->_read_oob
...
The reason is that we are re-working the MTD API and from now on it is
an error to use MTD function pointers directly - we have a corresponding
API call for every pointer. By adding a leading "_" we achieve the following:
1. Make sure we convert every direct pointer users
2. A leading "_" suggests that this interface is internal and it becomes
less likely that people will use them directly
3. Make sure all the out-of-tree modules stop compiling and the owners
spot the big API change and amend them.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
This patch is part of a patch-set which changes the MTD interface
from 'mtd->func()' form to 'mtd_func()' form. We need this because
we want to add common code to to all drivers in the mtd core level,
which is impossible with the current interface when MTD clients
call driver functions like 'read()' or 'write()' directly.
At this point we just introduce a new inline wrapper function, but
later some of them are expected to gain more code. E.g., the input
parameters check should be moved to the wrappers rather than be
duplicated at many drivers.
This particular patch introduced the 'mtd_erase()' interface. The
following patches add all the other interfaces one by one.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
These modes are not necessarily for OOB only. Particularly, MTD_OOB_RAW
affected operations on in-band page data as well. To clarify these
options and to emphasize that their effect is applied per-operation, we
change the primary prefix to MTD_OPS_.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@intel.com>
There is no need to pollute public header with a definition private
to mtdpart.c. Move it from mtd/partitions.h to mtdpart.c
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
There is no need to export parse_mtd_partitions() now , as it's fully handled
by registration functions. So move the definition to private header and
remove respective EXPORT_SYMBOL_GPL.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Prepare to convert of_mtd_parse_partitions() to usual partitions parser:
1) Register ofpart parser
2) Internally don't use passed device for error printing
3) Add device_node to mtd_part_parser_data struct
4) Move of_mtd_parse_partitions from __devinit to common text section
5) add ofpart to the default list of partition parsers
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Signed-off-by: Artem Bityutskiy <dedekind1@gmail.com>
Encapsulate last MTD partition parser argument into a separate
structure. Currently it holds only 'origin' field for RedBoot parser,
but will be extended in future to contain at least device_node for OF
devices.
Amended commentary to make kerneldoc happy
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Signed-off-by: Artem Bityutskiy <dedekind1@gmail.com>
Add a kerneldoc comment for the 'parse_mtd_partitions()' function - its
behavior has changed recently so it is good idea to have it documented.
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Add support for MTDPART_OFS_RETAIN: such partitions start at the current
offset, take as much space as possible, but rain part->size bytes after
the end of the partitions for other parts. Primarily this is intended
for ts72xx arm platforms cleanup.
Artem: tweaked the patch a bit
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Lots of MTD devices default to cmdlinepart, NULL as partition parsing
order. Make it a default.
Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
These symbols are replaced with mtd_device_register() (and removal with
mtd_device_unregister()) for public registration.
Signed-off-by: Jamie Iles <jamie@jamieiles.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Currently when we register partitions in 'parse_mtd_partitions()' we accept the
list of parsers we should try. And if one of the parsers was not found we print
a message. Well, first of all this whole idea is bad - look at how many
'part_probes' and 'part_probe_types' variables we have - nearly every driver
defines one. Instead, we should just go through all registered parsers all the
time. But this needs to be worked on separately.
This patch makes life of MTD partitions' users a bit simpler and allows them to
safely request parsers which have not been registered -
'parse_mtd_partitions()' will not print a "not available" message in this
case.
The point is that drivers do not have to do things like this any longer:
static const char *part_probe_types[] = { "cmdlinepart", "RedBoot",
"afs",
NULL };
but can simply do like this:
static const char *part_probe_types[] = { "cmdlinepart", "RedBoot", "afs", NULL };
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
This patch fixes the mtdpart bug which allows users reading OOB past the
end of the partition. This happens because 'part_read_oob()' allows reading
multiple OOB areas in one go, and mtdparts does not validate the OOB
length in the request.
Although there is such check in 'nand_do_read_oob()' in nand_base.c, but
it checks that we do not read past the flash chip, not the partition,
because in nand_base.c we work with the whole chip (e.g., mtd->size
in nand_base.c is the size of the whole chip). So this check cannot
be done correctly in nand_base.c and should be instead done in mtdparts.c.
This problem was reported by Jason Liu <r64343@freescale.com> and reproduced
with nandsim:
$ modprobe nandsim first_id_byte=0x20 second_id_byte=0xaa third_id_byte=0x00 \
fourth_id_byte=0x15 parts=0x400,0x400
$ modprobe nandsim mtd_oobtest.ko dev=0
$ dmesg
= snip =
mtd_oobtest: attempting to read past end of device
mtd_oobtest: an error is expected...
mtd_oobtest: error: read past end of device
= snip =
mtd_oobtest: finished with 2 errors
Reported-by: Jason Liu <liu.h.jason@gmail.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Propagate the writebufsize to the partition's MTD object so
that UBI can set correct value for it's minimal I/O size
using the writebufsize field of MTD object of the partition.
By previous patches we added proper writebufsize field
initialization. Next patch can now change UBI to use
this field for setting the minimal I/O size.
Signed-off-by: Anatolij Gustschin <agust@denx.de>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
David Woodhouse