[BUG]: spi-nor: spi_nor_init() fails when SR locked

[BUG]: spi-nor: spi_nor_init() fails when SR locked

[David Clear]

Hi Tudor,

I'm seeing a change of behavior in spi-nor/next vs. my older 4.14
kernel w.r.t. how SR lock is working.

In my system with a Micron mt25qu02g, I'm using the hardware WP# to
lock the SR.  When hardware write-protect is on, at the next reboot
the kernel rejects the flash as it cannot unlock the whole device.

I traced the problem to spi_nor_write_sr1_and_check(), which is
writing to the SR and reading back a different value (as the locked SR
bits are immutable).

Call trace:
 spi_nor_write_sr1_and_check+0x68/0x70
 spi_nor_write_sr_and_check+0x34/0xd0
 spi_nor_sr_unlock+0x108/0x230
 spi_nor_unlock+0x54/0x80  (via spi_nor_unlock_all())
 spi_nor_init+0x94/0x100

spi_nor_write_sr1_and_check() is returning -EIO here:
        if (nor->bouncebuf[0] != sr1) {
                dev_dbg(nor->dev, "SR1: read back test failed\n");
                BUG();
                return -EIO;
        }

In my specific case Linux doesn't have (and cannot be given, for
product security reasons) control over the WP#.  In any case, I don't
think a write-protected flash should be rejected here.

Can you suggest how we might proceed?  The way WP# is used is
board-specific so perhaps a device-tree property of some sort in the
flash device node can inform the code to do or not do the
spi_nor_unlock_all()?

I can take a look at putting a patch together if you suggest an
acceptable mechanism.

Regards,
David.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [BUG]: spi-nor: spi_nor_init() fails when SR locked

[Tudor.Ambarus]

Hi, David,

On 7/20/20 9:13 PM, David Clear wrote:
> 
> Hi Tudor,
> 
> I'm seeing a change of behavior in spi-nor/next vs. my older 4.14
> kernel w.r.t. how SR lock is working.
> 
> In my system with a Micron mt25qu02g, I'm using the hardware WP# to
> lock the SR.  When hardware write-protect is on, at the next reboot
> the kernel rejects the flash as it cannot unlock the whole device.
> 
> I traced the problem to spi_nor_write_sr1_and_check(), which is
> writing to the SR and reading back a different value (as the locked SR
> bits are immutable).
> 
> Call trace:
>  spi_nor_write_sr1_and_check+0x68/0x70
>  spi_nor_write_sr_and_check+0x34/0xd0
>  spi_nor_sr_unlock+0x108/0x230
>  spi_nor_unlock+0x54/0x80  (via spi_nor_unlock_all())
>  spi_nor_init+0x94/0x100
> 
> spi_nor_write_sr1_and_check() is returning -EIO here:
>         if (nor->bouncebuf[0] != sr1) {
>                 dev_dbg(nor->dev, "SR1: read back test failed\n");
>                 BUG();
>                 return -EIO;
>         }
> 
> In my specific case Linux doesn't have (and cannot be given, for
> product security reasons) control over the WP#.  In any case, I don't
> think a write-protected flash should be rejected here.
> 
> Can you suggest how we might proceed?  The way WP# is used is
> board-specific so perhaps a device-tree property of some sort in the
> flash device node can inform the code to do or not do the
> spi_nor_unlock_all()?
> 
> I can take a look at putting a patch together if you suggest an
> acceptable mechanism.
> 

Thanks for the detailed report. I'll have to study this a bit, I'll get
back to you in few days/one week. We tackled the unlock_all stuff in
the past, Michael did a patch, I'll check how that fits here.

https://patchwork.ozlabs.org/project/linux-mtd/patch/20200327155939.13153-1-michael@walle.cc/

Cheers,
ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [BUG]: spi-nor: spi_nor_init() fails when SR locked

[David Clear]

On Tue, Jul 21, 2020 at 2:37 AM <Tudor.Ambarus@microchip.com> wrote:
> We tackled the unlock_all stuff in
> the past, Michael did a patch, I'll check how that fits here.

Thanks Tudor.  The new Macronix mx662ug45g flash in my patch also
supports similar block protections, with WP#, but it looks like the driver
doesn't support BP in any of the Macronix flashes.

The functionality is similar to Micron's (power-of-2 sectors, top/bottom,
WP# lock), with two differences:
1. The BP bits in the SR are in different positions
2. The Top/Bottom bit is OTP, and is in the Configuration Register

As Macronix flashes don't declare BP support it looks like they won't suffer
this specific issue (I'll verify in the next few days), but I'd like to
evolve the driver to support BP on these devices.

Thanks for the patch pointer.  I'll take a look at it.

Regards,
David.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/