arm32 allmodconfig fails to build with gcc:
drivers/mtd/chips/cfi_cmdset_0001.c:1883:1: error: the frame size of 1296
bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
With MTD_MAP_BANK_WIDTH_32 a map_word is 32 bytes, and with KASAN_STACK
every temporary gets its own redzoned slot. do_write_buffer() builds a
map_word for each of the twelve commands it issues, and those temporaries
dominate its frame.
Issue the commands through a small noinline helper that takes the
command as a plain value and builds the map_word in its own frame. The
sequence of reads and writes to the chip is unchanged.
The helper must not be inlined, and reusing one map_word local is not
enough: clang cannot assume the callee does not observe the local, so it
still returns each CMD() into a per-call-site temporary, and the frame
grows (1760 -> 1824 bytes in cfi_intelext_writev(), into which clang
inlines do_write_buffer()).
Frame sizes with MTD_MAP_BANK_WIDTH_32 and KASAN_STACK:
gcc 15.2 do_write_buffer() 1296 -> 520
clang 21 cfi_intelext_writev() 1760 -> 896
The helper itself takes 168 (gcc) / 160 (clang) bytes.
Reported-by: Andy Shevchenko <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Suggested-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/all/[email protected]/
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
cfi_intelext_reboot() puts each chip into FL_SHUTDOWN (array mode) via
the reboot notifier so the bootloader can read the flash on the next
boot. If pm_suspend then runs on the same chip during the shutdown
sequence, cfi_intelext_suspend() falls into the default branch and
prints:
Flash device refused suspend due to active operation (state 20)
The chip is not active; it is already shut down. Treat FL_SHUTDOWN the
same as FL_PM_SUSPENDED in the suspend switch so nothing is done and
no warning is emitted.
Reproduced on every boot of the QEMU 'virt' arm64 machine, which
exposes two Intel-Sharp pflash chips.
Signed-off-by: Breno Leitao <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
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 <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
This converts some of the visually simpler cases that have been split
over multiple lines. I only did the ones that are easy to verify the
resulting diff by having just that final GFP_KERNEL argument on the next
line.
Somebody should probably do a proper coccinelle script for this, but for
me the trivial script actually resulted in an assertion failure in the
middle of the script. I probably had made it a bit _too_ trivial.
So after fighting that far a while I decided to just do some of the
syntactically simpler cases with variations of the previous 'sed'
scripts.
The more syntactically complex multi-line cases would mostly really want
whitespace cleanup anyway.
Signed-off-by: Linus Torvalds <[email protected]>
This is the exact same thing as the 'alloc_obj()' version, only much
smaller because there are a lot fewer users of the *alloc_flex()
interface.
As with alloc_obj() version, this was done entirely with mindless brute
force, using the same script, except using 'flex' in the pattern rather
than 'objs*'.
Signed-off-by: Linus Torvalds <[email protected]>
This was done entirely with mindless brute force, using
git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' |
xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/'
to convert the new alloc_obj() users that had a simple GFP_KERNEL
argument to just drop that argument.
Note that due to the extreme simplicity of the scripting, any slightly
more complex cases spread over multiple lines would not be triggered:
they definitely exist, but this covers the vast bulk of the cases, and
the resulting diff is also then easier to check automatically.
For the same reason the 'flex' versions will be done as a separate
conversion.
Signed-off-by: Linus Torvalds <[email protected]>
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <[email protected]>
UBSAN reports shift-out-of-bounds in jedec_read_mfr() and jedec_read_id():
UBSAN: shift-out-of-bounds in drivers/mtd/chips/jedec_probe.c:1924:13
shift exponent 32 is too large for 32-bit type 'int'
UBSAN: shift-out-of-bounds in drivers/mtd/chips/jedec_probe.c:1940:12
shift exponent 32 is too large for 32-bit type 'int'
The JEDEC manufacturer/device ID masking uses:
(1 << (cfi->device_type * 8)) - 1
When cfi->device_type is 4, this evaluates to 1 << 32. Since the
literal '1' has type int, this is a 32-bit shift and is undefined behavior.
Fix it by using a 64-bit literal (1ULL) so the shift is performed in a 64-bit type.
Co-developed-by: Hui Peng <[email protected]>
Signed-off-by: Hui Peng <[email protected]>
Co-developed-by: Zhihao Yao (Zephyr) <[email protected]>
Signed-off-by: Zhihao Yao (Zephyr) <[email protected]>
Signed-off-by: Chenxi Hou <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Documentation/process/deprecated.rst recommends against performing
dynamic size calculations in the arguments of memory allocator
functions due to the risk of overflow. Such calculations can wrap
around and result in a smaller allocation than expected.
Replace the size calculation in cfiq allocation with struct_size()
helper to make the code clearer and handle overflows correctly.
Signed-off-by: Rahul Kumar <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Documentation/process/deprecated.rst recommends against performing
dynamic size calculations in the arguments of memory allocator
function due to the risk of overflow. Such calculations can
wrap around and result in a smaller allocation than what the caller
was expecting.
Replace the size calculation in cfiq allocation with struct_size()
helper to make the code clearer and handle the overflows correctly.
Signed-off-by: Gopi Krishna Menon <[email protected]>
Reviewed-by: Vignesh Raghavendra <[email protected]>
link: https://lore.kernel.org/linux-kernel-mentees/[email protected]/T/#u
Signed-off-by: Miquel Raynal <[email protected]>
There is a extraneous space before a newline in a pr_debug message.
Remove the space and remove a space after ( and before literal string
to clean up checkpatch warning.
Signed-off-by: Colin Ian King <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Variable ret is being assigned a value that is never read, the following
goto statement jumps to a statement that assigns ret a return from the
call to function do_write_oneword_once. The assignment is redundant
and can be removed.
Signed-off-by: Colin Ian King <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
With ARCH=x86, make allmodconfig && make W=1 C=1 reports:
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/parsers/brcm_u-boot.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/parsers/tplink_safeloader.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_util.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/chips/cfi_cmdset_0020.o
WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/mtd/maps/map_funcs.o
Add the missing invocations of the MODULE_DESCRIPTION() macro.
Signed-off-by: Jeff Johnson <[email protected]>
Acked-by: Vignesh Raghavendra <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
In two functions the variable timeo is being initialized with a value
that is never read, it is being re-assigned later on. The initializations
are redundant and can be removed.
Cleans up clang scan build warning:
warning: Value stored to 'timeo' during its initialization is never
read [deadcode.DeadStores]
Signed-off-by: Colin Ian King <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
Currently the offset into the device when looking for OTP
bits can go outside of the address of the MTD NOR devices,
and if that memory isn't readable, bad things happen
on the IXP4xx (added prints that illustrate the problem before
the crash):
cfi_intelext_otp_walk walk OTP on chip 0 start at reg_prot_offset 0x00000100
ixp4xx_copy_from copy from 0x00000100 to 0xc880dd78
cfi_intelext_otp_walk walk OTP on chip 0 start at reg_prot_offset 0x12000000
ixp4xx_copy_from copy from 0x12000000 to 0xc880dd78
8<--- cut here ---
Unable to handle kernel paging request at virtual address db000000
[db000000] *pgd=00000000
(...)
This happens in this case because the IXP4xx is big endian and
the 32- and 16-bit fields in the struct cfi_intelext_otpinfo are not
properly byteswapped. Compare to how the code in read_pri_intelext()
byteswaps the fields in struct cfi_pri_intelext.
Adding a small byte swapping loop for the OTP in read_pri_intelext()
and the crash goes away.
The problem went unnoticed for many years until I enabled
CONFIG_MTD_OTP on the IXP4xx as well, triggering the bug.
Cc: [email protected]
Reviewed-by: Nicolas Pitre <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
When the DT property no-unaligned-direct-access is set, map->phys is set
to NO_XIP. With this property set, the flash should not be exposed
directly to MTD users, since it cannot be mapped.
map_ram() exposes the flash direct access unconditionally which leads to
access errors (when the bus width does not match the RAM width).
Therefore do not set point and unpoint when NO_XIP is set.
Signed-off-by: Shivamurthy Shastri <[email protected]>
Reviewed-by: Benedikt Spranger <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
The DT of_device.h and of_platform.h date back to the separate
of_platform_bus_type before it as merged into the regular platform bus.
As part of that merge prepping Arm DT support 13 years ago, they
"temporarily" include each other. They also include platform_device.h
and of.h. As a result, there's a pretty much random mix of those include
files used throughout the tree. In order to detangle these headers and
replace the implicit includes with struct declarations, users need to
explicitly include the correct includes.
Signed-off-by: Rob Herring <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Lee Jones <[email protected]>
Some of the files in mtd/chips do not have a SPDX license
header, presumably because the text string "GPL'd" didn't
parse with Thomas rulesets for magic license tagging.
Fix this, the code is initially from RedHat which clearly
targeted the Linux kernel and intended it to be GPLv2.
In any case the original author appears to be David
Woodhouse who can then confirm this.
Cc: David Woodhouse <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
Since commit dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to
check correct value") buffered writes fail on S29GL064N. This is
because, on S29GL064N, reads return 0xFF at the end of DQ polling for
write completion, where as, chip_good() check expects actual data
written to the last location to be returned post DQ polling completion.
Fix is to revert to using chip_good() for S29GL064N which only checks
for DQ lines to settle down to determine write completion.
Link: https://lore.kernel.org/r/[email protected]/
Fixes: dfeae1073583("mtd: cfi_cmdset_0002: Change write buffer to check correct value")
Cc: [email protected]
Signed-off-by: Tokunori Ikegami <[email protected]>
Acked-by: Vignesh Raghavendra <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
drivers/mtd/maps/ixp4xx.c requires MTD_CFI_BE_BYTE_SWAP to be set
in order to compile.
drivers/mtd/maps/ixp4xx.c:57:4: error: #error CONFIG_MTD_CFI_BE_BYTE_SWAP required
This patch avoids the #error output by enforcing the policy in
Kconfig. Not sure if this is the right approach, but it helps doing
randconfig builds.
Signed-off-by: Arnd Bergmann <[email protected]>
Acked-by: Linus Walleij <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
The write buffer comes from user and should be const.
Constify write buffer in mtd core and across all _write_user_prot_reg()
users. cfi_cmdset_{0001, 0002} and onenand_base will pay the cost of an
explicit cast to discard the const qualifier since the beginning, since
they are using an otp_op_t function prototype that is used for both reads
and writes. mtd_dataflash and SPI NOR will benefit of the const buffer
because they are using different paths for writes and reads.
Signed-off-by: Tudor Ambarus <[email protected]>
Signed-off-by: Miquel Raynal <[email protected]>
Link: https://lore.kernel.org/linux-mtd/[email protected]
CFI changes
Just a couple of fixes for v5.13-rc1
* Disable broken buffered writes for CFI chips within ID 0x2201
* Address a Coverity report for unused value
The variable timeo is being initialized with a value that is never read
and it is being updated later with a new value. The initialization is
redundant and can be removed.
Signed-off-by: Colin Ian King <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Addresses-Coverity: ("Unused value")
Buffer writes do not work with AMD chip 0x2201. The chip in question
is a AMD/Spansion/Cypress Semiconductor S29GL256N and datasheet [1]
talks about writing buffers being possible. While waiting for a neater
solution resort to writing word-sized chunks only.
Without the patch kernel logs will be flooded with entries like below:
jffs2_scan_eraseblock(): End of filesystem marker found at 0x0
jffs2_build_filesystem(): unlocking the mtd device...
done.
jffs2_build_filesystem(): erasing all blocks after the end marker...
MTD do_write_buffer_wait(): software timeout, address:0x01ec000a.
jffs2: Write clean marker to block at 0x01920000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01e2000a.
jffs2: Write clean marker to block at 0x01880000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01e0000a.
jffs2: Write clean marker to block at 0x01860000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01dc000a.
jffs2: Write clean marker to block at 0x01820000 failed: -5
MTD do_write_buffer_wait(): software timeout, address:0x01da000a.
jffs2: Write clean marker to block at 0x01800000 failed: -5
...
Tested on a Buffalo wzr-hp-g300nh running kernel 5.10.16.
[1] https://www.cypress.com/file/219941/download
or https://datasheetspdf.com/pdf-file/565708/SPANSION/S29GL256N/1
Signed-off-by: Mauri Sandberg <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Rationale:
Reduces attack surface on kernel devs opening the links for MITM
as HTTPS traffic is much harder to manipulate.
Deterministic algorithm:
For each file:
If not .svg:
For each line:
If doesn't contain `\bxmlns\b`:
For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`:
If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`:
If both the HTTP and HTTPS versions
return 200 OK and serve the same content:
Replace HTTP with HTTPS.
Signed-off-by: Alexander A. Klimov <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
The flash controller implemented by the Arm Base platform behaves like
the Intel StrataFlash J3 device, but omits several features. In
particular it doesn't implement a protection register, so "Number of
Protection register fields" in the Primary Vendor-Specific Extended
Query, is 0.
The Intel StrataFlash J3 datasheet only lists 1 as a valid value for
NumProtectionFields. It describes the field as:
"Number of Protection register fields in JEDEC ID space.
“00h,” indicates that 256 protection bytes are available"
While a value of 0 may arguably not be architecturally valid, the
driver's current behavior is certainly wrong: if NumProtectionFields is
0, read_pri_intelext() adds a negative value to the unsigned extra_size,
and ends up in an infinite loop.
Fix it by ignoring a NumProtectionFields of 0.
Signed-off-by: Jean-Philippe Brucker <[email protected]>
Tested-by: Sudeep Holla <[email protected]>
Tested-by: Catalin Marinas <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
The commit 4844ef8030 ("mtd: cfi_cmdset_0002: Add support for polling
status register") added checking for the status register error bits into
chip_good() to only return 1 if these bits are 0s. Unfortunately, this
means that polling using chip_good() always reaches a timeout condition
when erase or program failure bits are set. Let's fully delegate the task
of determining the error conditions to cfi_check_err_status() and make
chip_good() only look for the Device Ready/Busy condition.
Fixes: 4844ef8030 ("mtd: cfi_cmdset_0002: Add support for polling status register")
Signed-off-by: Sergei Shtylyov <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
Cypress S26K{L|S}P{128|256|512}S datasheet says that the error bits in
the status register are only valid when the "device ready" bit 7 is set.
Add the check for the device ready bit in cfi_check_err_status() as that
function isn't always called with this bit set.
Fixes: 4844ef8030 ("mtd: cfi_cmdset_0002: Add support for polling status register")
Signed-off-by: Sergei Shtylyov <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
Else there may be a double-free problem, because cfi->cfiq will
be freed by mtd_do_chip_probe() if both the two invocations of
check_cmd_set() return failure.
Signed-off-by: Hou Tao <[email protected]>
Reviewed-by: Richard Weinberger <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
The 'ret' local variables are typically initialized to 0 but this value is
often unused, thus we can kill those initializers.
Signed-off-by: Sergei Shtylyov <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
Use DIV_ROUND_UP() in cfi_udelay() instead of open-coding it...
Doing this also helpfully gets rid of two complaints from
'scripts/checkpatch.pl --strict':
CHECK: spaces preferred around that '+' (ctx:VxV)
#29: FILE: drivers/mtd/chips/cfi_util.c:29:
+ msleep((us+999)/1000);
^
CHECK: spaces preferred around that '/' (ctx:VxV)
#29: FILE: drivers/mtd/chips/cfi_util.c:29:
+ msleep((us+999)/1000);
^
Signed-off-by: Sergei Shtylyov <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
The chip state is set to erasing by the function after getting chip.
So it should be to get chip as erasing mode at first.
But previously it was to get chip as writing mode then fix as erasing.
Signed-off-by: Tokunori Ikegami <[email protected]>
Cc: [email protected]
Signed-off-by: Richard Weinberger <[email protected]>
Some write buffer functions are not used when FORCE_WORD_WRITE is set to 1.
So the compile warning messages are output if FORCE_WORD_WRITE is 1. To
resolve this disable the write buffer functions if FORCE_WORD_WRITE is 1.
Cc: Fabio Bettoni <[email protected]>
Co: Hauke Mehrtens <[email protected]>
Cc: Chris Packham <[email protected]>
Cc: Joakim Tjernlund <[email protected]>
Cc: [email protected]
Signed-off-by: Tokunori Ikegami <[email protected]>
Signed-off-by: Vignesh Raghavendra <[email protected]>
Karl Mehltretter