Commit 22d982 ramips: add support for switching between 3-byte and 4-byte addressing on w25q256 flash

Sorry I do not have your forum ID or github ID, and you leave the fake email on github commit, so I have to post here.
your commit https://git.lede-project.org/?p=source.git;a=commitdiff;h=22d982ea003360d86ff1fef5114fb41dfa09ce9b for solve 32MB SPI NOR FLASH soft reset hang on MT7620/MT7621 I do not think thats the right way, and it will reduce IO performance.
There is already a graceful solution at 2015-02-05 on china blog csdn.
http://blog.csdn.net/manfeel/article/details/43530817
while kernel get ready to make soft reset, switch SPI back to 24bits address mode, thats enought, not only W25Q256 but also MX25L256 or any other 32MB chips, will never hang while rebooting.

cat ~/git/lede/source/target/linux/ramips/patches-4.4/0911-spi_nor_32MB_soft_reset_hang_fix.patch
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -261,6 +261,12 @@
 {
        struct m25p     *flash = spi_get_drvdata(spi);

+       // manfeel note: add spi flash reset code
+       flash->command[0] = 0x66;
+       spi_write(flash->spi, flash->command, 1);
+       flash->command[0] = 0x99;
+       spi_write(flash->spi, flash->command, 1);
+
        /* Clean up MTD stuff. */
        return mtd_device_unregister(&flash->spi_nor.mtd);
 }
@@ -328,6 +334,7 @@
        .id_table       = m25p_ids,
        .probe  = m25p_probe,
        .remove = m25p_remove,
+       .shutdown = m25p_remove,

        /* REVISIT: many of these chips have deep power-down modes, which
         * should clearly be entered on suspend() to minimize power use.

What "fake email"? Just a personal domain...

Also the github ID nbd168 is pretty visible in the commit:
https://github.com/lede-project/source/commit/22d982ea003360d86ff1fef5114fb41dfa09ce9b

anyway,
cc to @nbd

This "problem" must be solved in hardware, not software. All software solutions, even the one from Felix, are just a workarounds and won't help in extreme situation (like reset caused by watchdog). I suggest you to contact with the manufacturer which made the hardware you are using and tell them about the mistake they made (and are still making in new designs).

And please, read carefully this comment: https://github.com/lede-project/source/pull/620#issuecomment-295401936

This patch will handle any soft reset while kernel still in control , If a case such like watch dog event that even kernel can not handle it, how can a driver do anymore? And I think many watch dog will trigger a HW reset instead of soft reset.

And I am using this patch with MXIC MX 25L25645G SOP8, testing reboot shell command, by shell script or cmdline, no crash rebooting back to live.

That's the reason why this problem is a hardware design mistake and cannot be fully fixed in software.

With this patch my router can up again after sysupgrade, without it, I need come to the router and power down&up, complain wont help, just do what we can do.

Your board has a different FLASH chip, so the workaround from Felix doesn't apply in your case.
Please file a bug on our tracker and next time buy hardware which doesn't have such design flaws.

I have both W25Q256FV SOP16 and MX25L25645G SOP8 flash chip, testing pass with this patch.
1113-3
I buy this hardware because It is low power device with 11ac and GbE, can have my router DC 12V 30Ah UPS keep it up 17 live hours together with a 5V webcam.
And file a bug means?

it means filing a bug at the LEDE bug tracker:
https://bugs.lede-project.org

This is just a discussion forum.

Ok, I will report this soft reset hang bug and with this patch on tracker, instead of submit a patch pull request to LEDE github.
Because I do not want to leave my real name with LEDE github 'Signed-off-by' digital finger print. I am living in border of China and the reason of GFW policy.

It was rejected already before:
https://github.com/lede-project/source/pull/718

Instead of sending the same patch again, please report the problem and give as much information about your hardware as you can.

I did consider the simpler patch of resetting the chip before a soft reboot, but as pepe2k already pointed out, this does not work if the watchdog resets the board. My patch is designed to leave the chip in 3-address mode as long as possible in order to minimize the chance of a permanent hang after the watchdog fires.
Yes, we can contact the manufacturer to point out the hardware design flaw, but that won't change the fact that there is hardware with this flaw out there and we have to deal with that.
I guess we can also look into extending my workaround for other flash chips, e.g. MX25L25645G

As I said, most watchdogs will trigger a Hard Rest, just like power up reset, we do not need fix that. If kernel crash, we can do nothing, and we have no way fix anything.
But at lease we need make the normal flow working well, type: reboot, system need reset and let boot loader get control.

Yes I find that, but this patch only for kernel 4.4.x, and I write a patch for kernel 4.9.x.

cat target/linux/ramips/patches-4.9/0911-spi_nor_32MB_soft_reset_hang_fix.patch
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -257,6 +257,16 @@
        return mtd_device_unregister(&flash->spi_nor.mtd);
 }

+static void m25p_shutdown(struct spi_device *spi)
+{
+       struct m25p     *flash = spi_get_drvdata(spi);
+
+       flash->command[0] = 0x66;
+       spi_write(flash->spi, flash->command, 1);
+       flash->command[0] = 0x99;
+       spi_write(flash->spi, flash->command, 1);
+}
+
 /*
  * Do NOT add to this array without reading the following:
  *
@@ -327,6 +337,7 @@
        .id_table       = m25p_ids,
        .probe  = m25p_probe,
        .remove = m25p_remove,
+       .shutdown = m25p_shutdown,

        /* REVISIT: many of these chips have deep power-down modes, which
         * should clearly be entered on suspend() to minimize power use.

On the board that I developed this patch for (ZBT WG3526), a watchdog-issued hard reset does not reset the flash chip. They probably forgot to connect the flash chip reset line to the system reset line.

Just file a bug.
https://bugs.lede-project.org/index.php?do=details&task_id=1165

I understand know. And I think this patch will not conflict with your solution, it will let more devices fix this bug.

One potential issue with your patch is that it will make it harder to figure out which devices need fixing for watchdog resets as well. The way it is now, it is obvious by testing software reset, where additional workarounds are needed. In those cases it probably makes sense to reuse the code from my workaround which switches between 3-byte and 4-byte mode.

Well, first make it work, then make it better.

I find that the only way make spi flash chip reset is cut off power supply, because there is no pin to reset it.
And the soft rest just the patch: send cmd 0x66 0x99.
So watch dog reset issue for 32MB spi flash chip is no solution to mt7620/mt7621, of cause your patch only for W25Q256 can minimize the chance of hangs.
Lets forget about watch dog, fix the normal reboot soft reset first.