SPI NOR changes for 7.2
Notable changes:
- Big set of cleanups and improvements to the locking support. This
series contains some cleanups and bug fixes for code and documentation
around write protection. Then support is added for complement locking,
which allows finer grained configuration of what is considered locked
and unlocked. Then complement locking is enabled on a bunch of Winbond
W25 flashes.
- Fix die erase support on Spansion flashes. Die erase is only supported
on multi-die flashes, but the die erase opcode was set for all. When
the opcode is set, it overrides the default chip erase opcode which
should be used for single-die flashes. Only set the opcode on
multi-die flashes. Also, the opcode was not set on multi-die s28hx-t
flashes. Set it so they can use die-erase correctly.
Use additional labels so that a bit of exception handling can be better
reused at the end of two function implementations.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Use local variables for the list entry, mtd_info, and private data while
initializing a new device. This keeps the initialization path easier to
read and publishes the new list entry only after mtd_device_register()
has succeeded.
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
register_device() links a new slram_mtdlist entry before allocating all
of the state needed by the entry. If a later allocation, memremap(), or
mtd_device_register() fails, the partially initialized entry remains on
the global list. A later cleanup can then dereference or free invalid
state from that failed entry.
Unwind the partially initialized entry and clear the list tail on each
failure path after the entry has been linked.
Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Unfortunately, this chip also comes with an incorrect BFPT table,
indicating the Control Register cannot be read back. This is wrong,
reading back the register works and has no (observed) side effect. The
datasheet clearly indicates supporting the 35h command and all bits from
the CR are marked readable. QE and CMP bits are inside, and can be
properly read back.
Add a fixup for this, otherwise it would defeat the use of the CMP
feature.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Unfortunately, this chip also comes with an incorrect BFPT table,
indicating the Control Register cannot be read back. This is wrong,
reading back the register works and has no (observed) side effect. The
datasheet clearly indicates supporting the 35h command and all bits from
the CR are marked readable. QE and CMP bits are inside, and can be
properly read back.
Add a fixup for this, otherwise it would defeat the use of the CMP
feature.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Unfortunately, this chip also comes with an incorrect BFPT table,
indicating the Control Register cannot be read back. This is wrong,
reading back the register works and has no (observed) side effect. The
datasheet clearly indicates supporting the 35h command and all bits from
the CR are marked readable. QE and CMP bits are inside, and can be
properly read back.
Add a fixup for this, otherwise it would defeat the use of the CMP
feature.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
This chip has support for the locking complement (CMP) feature. Add
the relevant bit to enable it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Extend the test coverage by giving guidelines to verify the CMP bit acts
according to our expectations.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
The current locking implementation allows to select a power of two
number of blocks, which is going to be the protected amount, as well as
telling whether this is the data at the top (end of the device) or the
bottom (beginning of the device). This means at most we can cover half
of the device or the entire device, but nothing in between.
The complement feature allows a much finer grain of configuration, by
allowing to invert what is considered locked and unlocked.
Add support for this feature. The only known position for the CMP bit is
bit 6 of the configuration register.
The locking and unlocking logics are kept unchanged if the CMP bit is
unavailable. Otherwise, once the regular logic has been applied, we
check if we already found an optimal configuration. If not, we try with
the CMP bit set. If the coverage is closer to the request, we use it.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
As recently raised on the mailing list, it may be useful to propose a
list of steps to go through in order to prove the devices have been
described correctly, especially since all the block protection
information is not stored in any kind of table and is instead filled
manually by developers.
Use the debugfs output to ease the comparison between expectations and
reality.
[rdunlap@infradead.org: fix build warning]
Link: https://lore.kernel.org/linux-mtd/20260526172341.773398-1-rdunlap@infradead.org/
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
This driver was originally merged in 2002 for a board using
the Cirrus Logic CL/PS711x platform, but the actual board
file never made it upstream.
The SoC platform is still supported but uses devicetree
based probing, so if anyone ever wanted to upstream board
support, they would just use the regular physmap driver.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Rather than using platform data or DT properties, the configuration
for this mtd map driver used to be passed through the global
uclinux_ram_map structure, but the last instance was removed in
commit 4ba66a9760 ("arch: remove blackfin port") in 2018.
After commit 251f26c9e8 ("mtd: maps: Make uclinux_ram_map static"),
it became impossible to configure it at all, even with out-of-tree
platform code.
Clearly nobody is using it, so remove it.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
There were four MTD maps drivers that were used with AMD Élan SoCs.
Since 486-class CPU support was removed in commit 8b793a92d8 ("x86/cpu:
Remove M486/M486SX/ELAN support"), it is no longer possible to actually
use these. Three of them have already been removed, so remove the
remaining driver entirely.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
[Miquel Raynal: Only NETtel was still in the tree, adapt the patch and the commit log]
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Replace all printk(KERN_WARNING ...) calls with pr_warn() to follow
the kernel's recommended logging style and resolve checkpatch warnings.
Also convert one bare printk() that was missing a log level.
No functional change.
Signed-off-by: Li Xinyu <xinyuili@126.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Initializing a pci_device_id using plain list expressions is hard to
understand for a human who isn't into the Linux PCI subsystem. So for
each initializer use one of the PCI_DEVICE macros for .vendor, .device,
.subvendor and .subdevice and named initializers for the remaining
assignments. This makes it easier to parse and also more robust against
chagnes to the struct definition.
Also drop useless zeros and commas.
The mentioned robustness is relevant for a planned change to struct
pci_device_id that replaces .driver_data by an anonymous union.
This change doesn't introduce changes to the compiled pci_device_id
array. Tested on x86 and arm64.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
The mtd_info contains a struct device, which must be linked to its
parent. Without this, the initialization of the MTD fails with a NULL
pointer dereference.
Fixes: 47a72688fa ("mtd: flash mapping support for Dreamcast VMU.")
Cc: stable@vger.kernel.org
Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Use kzalloc_obj() / kzalloc_objs() to allocate the memcard structs,
instead of kmalloc_obj() / kmalloc_objs() to prevent access to
uninitialized data.
Fixes runtime error: Fault in unaligned fixup: 0000 [#1] at
mtd_get_fact_prot_info.
Fixes: 47a72688fa ("mtd: flash mapping support for Dreamcast VMU.")
Cc: stable@vger.kernel.org
Signed-off-by: Florian Fuchs <fuchsfl@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Since commit 8b793a92d8 ("x86/cpu: Remove M486/M486SX/ELAN support"),
this board is no longer supported. Remove the mtd map too.
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Since commit 8b793a92d8 ("x86/cpu: Remove M486/M486SX/ELAN support"),
this board is no longer supported. Remove the mtd map too.
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Since commit 8b793a92d8 ("x86/cpu: Remove M486/M486SX/ELAN support"),
this board is no longer supported. Remove the mtd map too.
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
In order to get a very clear view of the sectors being locked, besides
the `params` output giving the ranges, we may want to see a proper map
of the sectors and for each of them, their status. Depending on the use
case, this map may be easier to parse by humans and gives a more acurate
feeling of the situation. At least myself, for the few locking-related
developments I recently went through, I found it very useful to get a
clearer mental model of what was locked/unlocked.
Here is an example of output:
$ cat /sys/kernel/debug/spi-nor/spi0.0/locked-sectors-map
Locked sectors map (x: locked, .: unlocked, unit: 64kiB)
0x00000000 (# 0): ................ ................ ................ ................
0x00400000 (# 64): ................ ................ ................ ................
0x00800000 (# 128): ................ ................ ................ ................
0x00c00000 (# 192): ................ ................ ................ ................
0x01000000 (# 256): ................ ................ ................ ................
0x01400000 (# 320): ................ ................ ................ ................
0x01800000 (# 384): ................ ................ ................ ................
0x01c00000 (# 448): ................ ................ ................ ................
0x02000000 (# 512): ................ ................ ................ ................
0x02400000 (# 576): ................ ................ ................ ................
0x02800000 (# 640): ................ ................ ................ ................
0x02c00000 (# 704): ................ ................ ................ ................
0x03000000 (# 768): ................ ................ ................ ................
0x03400000 (# 832): ................ ................ ................ ................
0x03800000 (# 896): ................ ................ ................ ................
0x03c00000 (# 960): ................ ................ ................ ..............xx
The output is wrapped at 64 sectors, spaces every 16 sectors are
improving the readability, every line starts by the first sector
offset (hex) and number (decimal).
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
[pratyush@kernel.org: split the debugfs_create_file() into two lines]
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
The ioctl output may be counter intuitive in some cases. Asking for a
"locked status" over a region that is only partially locked will return
"unlocked" whereas in practice maybe the biggest part is actually
locked.
Knowing what is the real software locking state through debugfs would be
very convenient for development/debugging purposes, hence this proposal
for adding an extra block at the end of the file: a "locked sectors"
array which lists every section, if it is locked or not, showing both
the address ranges and the sizes in numbers of "lock sectors" (which on
small density devices is typically different than erase blocks).
Here is an example of output, what is after the "sector map" is new.
$ cat /sys/kernel/debug/spi-nor/spi0.0/params
name (null)
id ef a0 20 00 00 00
size 64.0 MiB
write size 1
page size 256
address nbytes 4
flags HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | HAS_16BIT_SR | HAS_SR_TB_BIT6 | HAS_4BIT_BP | SOFT_RESET | NO_WP
opcodes
read 0xec
dummy cycles 6
erase 0xdc
program 0x34
8D extension none
protocols
read 1S-4S-4S
write 1S-1S-4S
register 1S-1S-1S
erase commands
21 (4.00 KiB) [1]
dc (64.0 KiB) [3]
c7 (64.0 MiB)
sector map
region (in hex) | erase mask | overlaid
------------------+------------+---------
00000000-03ffffff | [ 3] | no
locked sectors
region (in hex) | status | #sectors
------------------+----------+---------
00000000-03ffffff | unlocked | 1024
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
In order to be able to generate debugfs output without having to
actually reach the flash, create a SPI NOR local cache of the status
registers. What matters in our case are all the bits related to sector
locking. As such, in order to make it clear that this cache is not
intended to be used anywhere else, we zero the irrelevant bits.
The cache is initialized once during the early init, and then maintained
every time the write protection scheme is updated.
Suggested-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
As a final preparation step for the introduction of CMP support, make
a few more cosmetic changes to simplify the reading of the diff when
adding the CMP feature. In particular, define "min_prot_len" earlier as
it will be reused and move the definition of the "ret" variable at the
end of the stack just because it looks better.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
In both the locking/unlocking steps, at the end we verify whether we do
not lock/unlock more than requested (in which case an error must be
returned).
While being possible to do that with very simple mask comparisons, it
does not scale when adding extra locking features such as the CMP
possibility. In order to make these checks slightly easier to read and
more future proof, use existing helpers to read the (future) status
register, extract the covered range, and compare it with very usual
algebric comparisons.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
The status register contains 3 or 4 BP (Block Protect) bits, 0 or 1
TB (Top/Bottom) bit, soon 0 or 1 CMP (Complement) bit. The last BP bit
and the TB bit locations change between vendors. The whole logic of
buildling the content of the status register based on some input
conditions is used two times and soon will be used 4 times.
Create dedicated helpers for these steps.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Ease the future reuse of the tb (Top/Bottom) boolean by creating an
intermediate variable.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
"mask" is not very descriptive when we already manipulate two masks, and
soon will manipulate three. Rename it "bp_mask" to align with the
existing "tb_mask" and soon "cmp_mask".
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
There are many helpers already to either read and/or write SR and/or CR,
as well as sometimes check the returned values. In order to be able to
switch from a 1 byte status register to a 2 bytes status register while
keeping the same level of verification, let's introduce a new helper
that writes them both (atomically) and then reads them back (separated)
to compare the values.
In case 2 bytes registers are not supported, we still have the usual
fallback available in the helper being exported to the rest of the core.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
At this stage, the Status Register is most often seen as a single
byte. This is subject to change when we will need to read the CMP bit
which is located in the Control Register (kind of secondary status
register). Both will need to be carried.
Change a few prototypes to carry a u8 pointer. This way it also makes it
very clear where we access the first register, and where we will access
the second.
There is no functional change.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
The comment states that some power of two sizes are not supported. This
is very device dependent (based on the size), so modulate a bit the
sentence to make it more accurate.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Add more details about how these requests are actually handled in the
SPI NOR core. Their behaviour was not entirely clear to me at first, and
explaining them in plain English sounds the way to go.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Align the number of dashes to the bigger column width (the title in this
case) to make the output more pleasant and aligned with what is done
in the "params" file output.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
The "params" variable is used everywhere else, align this particular
line of the file to use "params" directly rather than the "nor" pointer.
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
There are two status registers, named 1 and 2. The current wording is
misleading as "1" may refer to the status register ID as well as the
number of bytes required (which, in this case can be 1 or 2).
Clarify the comments by aligning them on the same pattern:
"{read,write} status {1,2} register"
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Not all chips implement the 4BAIT table which typically indicates the
program capability, while many of them do implement the relevant SFDP
parts indicating the read capabilities. In such a situation, programs
can happen in single mode (1-1-1) and reads in quad mode (1-1-4 or
1-4-4). For the reads to work in such condition, the QE bit must be set.
In case we later use the spi_nor_write_16bit_sr_and_check() helper with
a chip with such configuration, the QE bit would get incorrectly
cleared.
Make sure this doesn't happen by keeping the QE bit under a simpler
condition:
- the quad enable hook is there (no change)
- and at least one of the two protocols is based on quad I/O cycles
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Pratyush Yadav <pratyush@kernel.org>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
I do not think the MTD dependency is needed twice. This is likely a
duplicate coming from a former rebase when the spi-nor core got cleaned
up a while ago. Remove the extra line.
Fixes: b35b9a1036 ("mtd: spi-nor: Move m25p80 code in spi-nor.c")
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
In the case of the first block being locked (or the few first blocks),
if the user want to fully unlock the device it has two possibilities:
- either it asks to unlock the entire device, and this works;
- or it asks to unlock just the block(s) that are currently locked,
which fails.
It fails because the conditions "can_be_top" and "can_be_bottom" are
true. Indeed, in this case, we unlock everything, so the TB bit does not
matter. However in the current implementation, use_top would be true (as
this is the favourite option) and lock_len, which in practice should be
reduced down to 0, is set to "nor->params->size - (ofs + len)" which is
a positive number. This is wrong.
An easy way is to simply add an extra condition. In the unlock() path,
if we can achieve the same result from both sides, it means we unlock
everything and lock_len must simply be 0. A comment is added to clarify
that logic.
Fixes: 3dd8012a8e ("mtd: spi-nor: add TB (Top/Bottom) protect support")
Cc: stable@kernel.org
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
As mentioned above the spi_nor_option_flags enumeration in core.h, this
list should be kept in sync with the one in the core.
Add the missing flag.
Fixes: 6a42bc97cc ("mtd: spi-nor: core: Allow specifying the byte order in Octal DTR mode")
Reviewed-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Signed-off-by: Pratyush Yadav <pratyush@kernel.org>
Use a flexible array member and kzalloc_flex() to do so. Simplifies
memory allocation slightly.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Modernize string-to-number conversion in mtdsuper.c by replacing
simple_strtoul with kstrtouint. This change provides proper type safety
for MTD device numbers which are non-negative integers.
Using kstrtouint avoids unsigned long to int conversion and is more
appropriate for device indices. The debug output format specifier is
updated to %u for unsigned int.
Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Replace deprecated simple_strtoul with kstrtouint for better error
handling and type safety. The kstrtouint function provides stricter
validation, automatically rejecting inputs like "123abc" that
simple_strtoul would partially accept.
Using kstrtouint avoids unsigned long to int conversion and is more
appropriate for MTD device indices which are non-negative integers.
Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Various names for Qualcomm as a company are used in user-visible config
options: QCOM, Qualcomm and Qualcomm Technologies. Switch to unified
"Qualcomm" so it will be easier for users to identify the options when
for example running menuconfig.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Pull clk fix from Stephen Boyd:
"One more fix for the merge window to avoid a boot hang on
Raspberry Pi 3B by marking the VEC clk critical so that it
doesn't get turned off and hang the bus"
* tag 'clk-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux:
clk: bcm: rpi: Mark VEC clock as CLK_IGNORE_UNUSED
