Re: [PATCH v2] mtd: spi-nor: handle unsupported FSR opcodes properly

Mika Westerberg
2022-06-16T10:36:03Z

Hi,

On Thu, Jun 16, 2022 at 07:40:18AM +0000, Oleksandr Ocheretnyi -X (oocheret - GLOBALLOGIC INC at Cisco) wrote:
> Hi Mika,
>
> > Originally commit 094d3b9 ("mtd: spi-nor: Add USE_FSR flag for
> n25q*
> > entries") and following one 8f93826 ("mtd: spi-nor: micron-st:
> convert
> > USE_FSR to a manufacturer flag") enabled SPINOR_OP_RDFSR opcode
> handling
> > ability, however some controller drivers still cannot handle it
> properly
> > in the micron_st_nor_ready() call what breaks some mtd callbacks
> with
> > next error logs:
> >
> > mtdblock: erase of region [address1, size1] on "BIOS" failed
> > mtdblock: erase of region [address2, size2] on "BIOS" failed
> >
> > The Intel SPI controller does not support low level operations,
> like
> > reading the flag status register (FSR). It only exposes a set of
> high
> > level operations for software to use. For this reason check the
> return
> > value of micron_st_nor_read_fsr() and if the operation was not
> > supported, use the status register value only. This allows the
> chip to
> > work even when attached to Intel SPI controller (there are such
> systems
> > out there).
> >
>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> I don't think I signed this off.
>
> I thought if I take your case (-EOPNOTSUPP) and update it with
> (-ENOTSUPP) I need to keep
>
> your Sighed-off-by: note as well.

That's not how it typically works. People will give their tag explicitly
and then you can add those.

> > Signed-off-by: Oleksandr Ocheretnyi <oocheret@cisco.com>
> > Link: [1]https://lore.kernel.org/lkml/YmZUCIE%2FND82BlNh@lahna/
> > ---
>
> What changed between v1 and v2?
>
> ​I updated v1 patch taking into account your changes
> [2]https://lore.kernel.org/linux-mtd/20220506105158.43613-1-mika.wester
> berg@linux.intel.com to check -EOPNOTSUPP case as well. After I
> combined both patches I've got v2.

Please put that information after the '---' in the patch.

> And did you take into consideration the comments I gave?
>
> ​If you say about keeping -ENOTSUPP as intel driver errorcode - I took
> it however doubted to use it here because of note about nfs above.
> There is no problem to restore previous variant with -ENOTSUPP in intel
> driver errorcode.

Well we would need to get some feedback from SPI-NOR maintainers. I
would personally keep using ENOTSUPP to be consistent with the rest of
the code in SPI-NOR code (or convert it to use EOPNOTSUPP everywhere)
but it is not up to me ;-)

For Intel driver it is fine to use either (whetever the decision of
SPI-NOR maintainers' is).