Hi, > I'm reporting this as an user of OpenWRT 23.05 on an Ubiquiti > Nanostation AC, who hopes to upgrade to 24.10. Currently, upgrading > results in a device where no configuration can be saved. Recovery > requires physical presence, on devices often installed outdoor. thanks for the bug report. > The issue has an explanation and a patch (for kernel 6.6) has been > available since late 2024 [1, attached below][2, which merges multiple > fixes]. It has been used by others [3] and myself. However, its author > believes that it doesn't have the correct approach to be upstreamed [4]. > OpenWRT maintainers are waiting for an ACK or better from upstream > before applying [5]. > > Is there a way to "unlock" this (pun intended) ? > > For reference, OpenWRT configures those devices with > CONFIG_MTD_SPI_NOR_SWP_DISABLE [6]. > > I'm available to test, but: > - the only device I can use is a production one > - OpenWRT is still working on upgrading ath79 from 6.6 to 6.12 [7], so > I'd be limited to testing on 6.6 > > Let me know how I can help. > > Thanks! > > Jean-Marc > > > [1] > https://github.com/openwrt/openwrt/pull/17287/commits/c98a55f95268f109911c5fddf5a153cfe3565b74 > [2] https://github.com/aredn/aredn/blob/main/patches/006-flash-fixes.patch > [3] https://github.com/openwrt/openwrt/issues/17285#issuecomment-2946978832 > [4] https://github.com/openwrt/openwrt/pull/17287#issuecomment-2558569582 > [5] https://github.com/openwrt/openwrt/pull/17287#issuecomment-2558502454 > [6] > https://github.com/openwrt/openwrt/blob/bb59922007043c0a0813d62b0d9f6e801caff9df/target/linux/ath79/generic/config-default > [7] https://github.com/openwrt/openwrt/issues/16569 > > > From c98a55f95268f109911c5fddf5a153cfe3565b74 Mon Sep 17 00:00:00 2001 > From: Tim Wilkinson > Date: Mon, 16 Dec 2024 09:37:34 -0800 > Subject: [PATCH] kernel: Fix setup of flash chips which must be unlocked > before use. > > Setup the mtd information for spi nor flash before calling running > the nor setup. > > The current code assumes the nor setup doesnt make reference to the mtd > information. There's even a comment to this effect. However, at least > when flash chips must be unlocked, this isn't true. Consequently, the > unlock code will fail because it makes reference to uninitialized mtd > information. This failure is silent because the bad mtd information > claims the flash is 0 bytes long, and so there is nothing to unlock. > > This patch moves the mtd initialization before the nor setup, so the > mtd info is available. > > This fix has been tested on two different Ubiquiti devices - the > Rocket AC Lite and the Rocket M5 XW. These use the mx25l12805d and > mx25l6405d Macronix flash chips respectively. Previously these devices > had failed to operate correctly as it was not possible for any > persistent changes to be made once the factory build had been installed. > With this change these devices behave correctly. > > Signed-off-by: Tim Wilkinson > --- > .../436-mtd-spi-earlier-mtd-setup.patch | 20 +++++++++++++++++++ > 1 file changed, 20 insertions(+) > create mode 100644 > target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch > > diff --git > a/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch > b/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch > new file mode 100644 > index 00000000000000..da75e9f7abfe96 > --- /dev/null > +++ b/target/linux/generic/pending-6.6/436-mtd-spi-earlier-mtd-setup.patch > @@ -0,0 +1,20 @@ > +--- a/drivers/mtd/spi-nor/core.c > ++++ b/drivers/mtd/spi-nor/core.c > +@@ -3540,14 +3540,14 @@ > + if (ret) > + return ret; > + > ++ /* No mtd_info fields should be used up to this point. */ > ++ spi_nor_set_mtd_info(nor); > ++ > + /* Send all the required SPI flash commands to initialize device */ > + ret = spi_nor_init(nor); > + if (ret) > + return ret; > + > +- /* No mtd_info fields should be used up to this point. */ > +- spi_nor_set_mtd_info(nor); > +- This seems to be due to the use of the uninitalized "mtd->size". Could you try the following patch which is based on the latest next kernel. It replaces mtd->size with nor->params->size, so you could backport it to 6.6, but maybe it will apply anyway. ---snip--- diff --git a/drivers/mtd/spi-nor/swp.c b/drivers/mtd/spi-nor/swp.c index 9c9328478d8a..9b07f83aeac7 100644 --- a/drivers/mtd/spi-nor/swp.c +++ b/drivers/mtd/spi-nor/swp.c @@ -56,7 +56,6 @@ static u64 spi_nor_get_min_prot_length_sr(struct spi_nor *nor) static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs, u64 *len) { - struct mtd_info *mtd = &nor->mtd; u64 min_prot_len; u8 mask = spi_nor_get_sr_bp_mask(nor); u8 tb_mask = spi_nor_get_sr_tb_mask(nor); @@ -77,13 +76,13 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs, min_prot_len = spi_nor_get_min_prot_length_sr(nor); *len = min_prot_len << (bp - 1); - if (*len > mtd->size) - *len = mtd->size; + if (*len > nor->params->size) + *len = nor->params->size; if (nor->flags & SNOR_F_HAS_SR_TB && sr & tb_mask) *ofs = 0; else - *ofs = mtd->size - *len; + *ofs = nor->params->size - *len; } /* @@ -158,7 +157,6 @@ static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, u64 len, */ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len) { - struct mtd_info *mtd = &nor->mtd; u64 min_prot_len; int ret, status_old, status_new; u8 mask = spi_nor_get_sr_bp_mask(nor); @@ -183,7 +181,7 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len) can_be_bottom = false; /* If anything above us is unlocked, we can't use 'top' protection */ - if (!spi_nor_is_locked_sr(nor, ofs + len, mtd->size - (ofs + len), + if (!spi_nor_is_locked_sr(nor, ofs + len, nor->params->size - (ofs + len), status_old)) can_be_top = false; @@ -195,11 +193,11 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len) /* lock_len: length of region that should end up locked */ if (use_top) - lock_len = mtd->size - ofs; + lock_len = nor->params->size - ofs; else lock_len = ofs + len; - if (lock_len == mtd->size) { + if (lock_len == nor->params->size) { val = mask; } else { min_prot_len = spi_nor_get_min_prot_length_sr(nor); @@ -248,7 +246,6 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, u64 len) */ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len) { - struct mtd_info *mtd = &nor->mtd; u64 min_prot_len; int ret, status_old, status_new; u8 mask = spi_nor_get_sr_bp_mask(nor); @@ -273,7 +270,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len) can_be_top = false; /* If anything above us is locked, we can't use 'bottom' protection */ - if (!spi_nor_is_unlocked_sr(nor, ofs + len, mtd->size - (ofs + len), + if (!spi_nor_is_unlocked_sr(nor, ofs + len, nor->params->size - (ofs + len), status_old)) can_be_bottom = false; @@ -285,7 +282,7 @@ static int spi_nor_sr_unlock(struct spi_nor *nor, loff_t ofs, u64 len) /* lock_len: length of region that should remain locked */ if (use_top) - lock_len = mtd->size - (ofs + len); + lock_len = nor->params->size - (ofs + len); else lock_len = ofs; ---snip--- -michael