[OpenWrt-Devel] Rebooting boards with > 16M SPI flash (was: Re: help)

Hi,

ldy647 <ldy647@163.com> writes:
> recently, when we install our wireless router, we found when we run
> the reboot command, the board couldn't restart. We hope you could lend
> us a hand to solve this problem. We'll be quite grateful for what you
> do for us.

Apparently MT7620 can't handle 4 byte addressing mode of the flash
memory on hardware (probably ROM bootloader) level so it needs to be
reset somehow prior to resetting the SoC. Some SoCs have a special
output from the watchdog to reset all the on-board peripherals, it
should be connected to the !RESET pin of the flash as well. And the
kernel should then use the watchdog driver to cause a reboot.

As a workaround when hardware modification is not possible and the
flash IC supports a reset command, you can use a patch like this (in
some cases when the kernel is completely stuck it won't help to reboot
of course, as the watchdog will fire on hardware level automatically,
if configured):

Hi Paul,

looks good to me but i will run this past linux-mtd and ask them for an
opinion before merging it.

	John

On 08/07/2015 14:00, Paul Fertser wrote:
> Hi,
> 
> ldy647 <ldy647@163.com> writes:
>> recently, when we install our wireless router, we found when we run
>> the reboot command, the board couldn't restart. We hope you could lend
>> us a hand to solve this problem. We'll be quite grateful for what you
>> do for us.
> 
> Apparently MT7620 can't handle 4 byte addressing mode of the flash
> memory on hardware (probably ROM bootloader) level so it needs to be
> reset somehow prior to resetting the SoC. Some SoCs have a special
> output from the watchdog to reset all the on-board peripherals, it
> should be connected to the !RESET pin of the flash as well. And the
> kernel should then use the watchdog driver to cause a reboot.
> 
> As a workaround when hardware modification is not possible and the
> flash IC supports a reset command, you can use a patch like this (in
> some cases when the kernel is completely stuck it won't help to reboot
> of course, as the watchdog will fire on hardware level automatically,
> if configured):
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index b298466..932f307 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -21,6 +21,7 @@
>  #include <linux/module.h>
>  #include <linux/device.h>
>  #include <linux/interrupt.h>
> +#include <linux/reboot.h>
>  #include <linux/mutex.h>
>  #include <linux/math64.h>
>  #include <linux/slab.h>
> @@ -61,6 +62,9 @@
>  /* Used for Spansion flashes only. */
>  #define	OPCODE_BRWR		0x17	/* Bank register write */
>  
> +#define OPCODE_RESET_ENABLE	0x66
> +#define OPCODE_RESET		0x99
> +
>  /* Status Register bits. */
>  #define	SR_WIP			1	/* Write in progress */
>  #define	SR_WEL			2	/* Write enable latch */
> @@ -907,6 +911,21 @@ static const struct spi_device_id *jedec_probe(struct spi_device *spi)
>  	return ERR_PTR(-ENODEV);
>  }
>  
> +static int m25p80_reboot(struct notifier_block *nb, unsigned long val,
> +			       void *v)
> +{
> +	struct mtd_info *mtd;
> +
> +	mtd = container_of(nb, struct mtd_info, reboot_notifier);
> +	struct m25p *flash = mtd_to_m25p(mtd);
> +
> +	flash->command[0] = OPCODE_RESET_ENABLE;
> +	spi_write(flash->spi, flash->command, 1);
> +	flash->command[0] = OPCODE_RESET;
> +	spi_write(flash->spi, flash->command, 1);
> +
> +	return NOTIFY_DONE;
> +}
>  
>  /*
>   * board specific setup should have ensured the SPI clock used here
> @@ -1010,6 +1029,7 @@ static int m25p_probe(struct spi_device *spi)
>  	flash->mtd.size = info->sector_size * info->n_sectors;
>  	flash->mtd._erase = m25p80_erase;
>  	flash->mtd._read = m25p80_read;
> +	flash->mtd.reboot_notifier.notifier_call = m25p80_reboot;
>  
>  	/* flash protection support for STmicro chips */
>  	if (JEDEC_MFR(info->jedec_id) == CFI_MFR_ST) {
> @@ -1085,6 +1105,7 @@ static int m25p_probe(struct spi_device *spi)
>  				flash->mtd.eraseregions[i].numblocks);
>  
>  
> +	register_reboot_notifier(&flash->mtd.reboot_notifier);
>  	/* partitions should match sector boundaries; and it may be good to
>  	 * use readonly partitions for writeprotected sectors (BP2..BP0).
>  	 */
> @@ -1099,6 +1120,8 @@ static int m25p_remove(struct spi_device *spi)
>  	struct m25p	*flash = dev_get_drvdata(&spi->dev);
>  	int		status;
>  
> +	unregister_reboot_notifier(&flash->mtd.reboot_notifier);
> +
>  	/* Clean up MTD stuff. */
>  	status = mtd_device_unregister(&flash->mtd);
>  	if (status == 0) {
>

Hi John,

John Crispin <blogic@openwrt.org> writes:
> looks good to me but i will run this past linux-mtd and ask them for an
> opinion before merging it.

This patch was meant as a quick "hack" rather than a complete solution,
that's why I didn't bother adding proper commit message, S-o-b etc. But
since I'm the author, I can do that promptly, whenever that's needed.

The main limitation is that I checked only a single datasheet (likely
Winbond) so other models might require different treatment. Another is
that such a patch is mostly a bad substitute for proper hardware design.

Citeren Paul Fertser <fercerpav@gmail.com>:

> Hi John,
>
> John Crispin <blogic@openwrt.org> writes:
>> looks good to me but i will run this past linux-mtd and ask them for an
>> opinion before merging it.
>
> This patch was meant as a quick "hack" rather than a complete solution,
> that's why I didn't bother adding proper commit message, S-o-b etc. But
> since I'm the author, I can do that promptly, whenever that's needed.
>
> The main limitation is that I checked only a single datasheet (likely
> Winbond) so other models might require different treatment.

The way to exit 4-byte addressing mode, can be determined by looking  
at the SFDP (JEDEC Standard JESD216B). As you already suspected, there  
are many possibilities. The method required, can be derived from the  
16th DWORD from the JEDEC Basic Flash Parameter Table (page 28).

> Another is
> that such a patch is mostly a bad substitute for proper hardware design.

Not really. There may not be a hardware reset line if the flash is  
running in x4 data mode.

Regards, Arjen

Arjen de Korte <arjen+openwrt@de-korte.org> writes:
> Citeren Paul Fertser <fercerpav@gmail.com>:
>> Another is
>> that such a patch is mostly a bad substitute for proper hardware design.
>
> Not really. There may not be a hardware reset line if the flash is
> running in x4 data mode.

Imagine there's a bug in the kernel that made it hang for real. Then
you're counting on the watchdog (usually SoCs come with one integrated)
to cleanly reboot the whole system. If the flash is used in QSPI or
whatever other mode that doesn't allow a dedicated reset line then it
needs to be powercycled on hardware level to ensure reliable operation,
I think.

Citeren Paul Fertser <fercerpav@gmail.com>:

> Arjen de Korte <arjen+openwrt@de-korte.org> writes:
>> Citeren Paul Fertser <fercerpav@gmail.com>:
>>> Another is
>>> that such a patch is mostly a bad substitute for proper hardware design.
>>
>> Not really. There may not be a hardware reset line if the flash is
>> running in x4 data mode.
>
> Imagine there's a bug in the kernel that made it hang for real. Then
> you're counting on the watchdog (usually SoCs come with one integrated)
> to cleanly reboot the whole system. If the flash is used in QSPI or
> whatever other mode that doesn't allow a dedicated reset line then it
> needs to be powercycled on hardware level to ensure reliable operation,
> I think.

Exactly. If you're designing for rock bottom low cost (I'm a hardware  
engineer), the SPI flash used will not have a dedicated reset line.  
That is why helpdesks will always tell in case of problems to power  
off a device for 30-60 seconds and then switch it back on again. In  
many cases, a watchdog reset is not equivalent to a power on reset.

Best regards, Arjen

On Wed, Jul 8, 2015 at 2:00 PM, Paul Fertser <fercerpav@gmail.com> wrote:
> Hi,
>
> ldy647 <ldy647@163.com> writes:
>> recently, when we install our wireless router, we found when we run
>> the reboot command, the board couldn't restart. We hope you could lend
>> us a hand to solve this problem. We'll be quite grateful for what you
>> do for us.
>
> Apparently MT7620 can't handle 4 byte addressing mode of the flash
> memory on hardware (probably ROM bootloader) level so it needs to be
> reset somehow prior to resetting the SoC. Some SoCs have a special
> output from the watchdog to reset all the on-board peripherals, it
> should be connected to the !RESET pin of the flash as well. And the
> kernel should then use the watchdog driver to cause a reboot.

According to the datasheet you can set the default to 4addr through
boostrapping pins. This should fix your particular issue.


Regards
Jonas