On Thu, Mar 28, 2024 at 03:44:15PM +0100, Rafał Miłecki wrote:
On 2024-03-28 15:19, Christian Marangi wrote:
On Wed, Mar 27, 2024 at 11:15:02PM +0100, Rafał Miłecki wrote:
On 22.03.2024 05:09, Christian Marangi wrote:
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 5887feb347a4..0de87bc63840 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -900,7 +900,7 @@ static struct nvmem_device *mtd_otp_nvmem_register(struct mtd_info *mtd, config.name = compatible; config.id = NVMEM_DEVID_AUTO; config.owner = THIS_MODULE;
- config.add_legacy_fixed_of_cells = true;
- config.add_legacy_fixed_of_cells = !mtd_type_is_nand(mtd); config.type = NVMEM_TYPE_OTP; config.root_only = true; config.ignore_wp = true;
I think there may be even more unwanted behaviour here. If mtd_otp_nvmem_register() fails to find node with "user-otp" / "factory-otp" compatible then it sets "config.of_node" to NULL but that means NVMEM core still looks for NVMEM cells in device's "of_node".
I believe we should not look for OTP NVMEM cells out of the "user-otp" / "factory-otp" compatible nodes.
So maybe what we need in the first place is just: config.add_legacy_fixed_of_cells = !!np; ?
Any extra limitation of .add_legacy_fixed_of_cells should probably be used only if we want to prevent new users of the legacy syntax. The problem is that mtd.yaml binding allowed "user-otp" and "factory-otp" with old syntax cells. It means every MTD device was allowed to have them.
No in-kernel DTS even used "user-otp" or "factory-otp" with NVMEM legacy cells but I'm not sure about downstream DTS files. Ideally we would do config.add_legacy_fixed_of_cells = false; but that could break compatibility with some downstream DTS files.
Yes the main problem is prevent regression in downstream. I feel for the nand usage, this is 100% of the times broken. For SPI and other corner case MTD devices it's not?
Anyway did you by chance have a suggestion for a better fixes tag?
My personal idea for that would be to put two Fixes with two commits and describe in commit body that one just exposed existing bug.
You may check my OpenWrt quick patch for an idea how I'd handle that: https://git.openwrt.org/?p=openwrt/openwrt.git%3Ba=blob%3Bf=target/linux/gen...
My concern is that using !!np might pose some regression problem. Also I feel adding the macronix commit in fixes tag might be confusing?
Think I will just use the nand check just to be extra safe and add a kernel dependency for when the add_legacy_fixed_of_cells was introduced since before that a different patch is needed. What do you think?