linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
@ 2022-09-23  9:34 Mika Westerberg
  2022-09-23  9:37 ` Michael Walle
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Mika Westerberg @ 2022-09-23  9:34 UTC (permalink / raw)
  To: Tudor Ambarus, Pratyush Yadav
  Cc: Michael Walle, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Takahiro Kuwano, Hongyu Ning,
	Mika Westerberg, linux-mtd

The Intel SPI-NOR controller does not support the 4-byte address opcode
so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
chip probe fail like this:

  [ 12.291082] spi-nor: probe of spi0.0 failed with error -524

Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
Return error code from set_4byte_addr_mode()") it worked just fine.

Fix this by ignoring -ENOTSUPP in spi_nor_init().

Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
Cc: stable@vger.kernel.org
Reported-by: Hongyu Ning <hongyu.ning@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
The previous version of the patch (the revert) can be found here:

  https://lore.kernel.org/linux-mtd/20220922134824.46758-1-mika.westerberg@linux.intel.com/

In this version we ignore -ENOTSUPP but the other error codes will be
passed to the caller.

 drivers/mtd/spi-nor/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index f2c64006f8d7..bee8fc4c9f07 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2724,7 +2724,9 @@ static int spi_nor_init(struct spi_nor *nor)
 		 */
 		WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
 			  "enabling reset hack; may not recover from unexpected reboots\n");
-		return nor->params->set_4byte_addr_mode(nor, true);
+		err = nor->params->set_4byte_addr_mode(nor, true);
+		if (err && err != -ENOTSUPP)
+			return err;
 	}
 
 	return 0;
-- 
2.35.1


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-09-23  9:34 [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init() Mika Westerberg
@ 2022-09-23  9:37 ` Michael Walle
  2022-10-03  5:04 ` Tudor.Ambarus
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Michael Walle @ 2022-09-23  9:37 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
	Vignesh Raghavendra, Takahiro Kuwano, Hongyu Ning, linux-mtd

Am 2022-09-23 11:34, schrieb Mika Westerberg:
> The Intel SPI-NOR controller does not support the 4-byte address opcode
> so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI 
> flash
> chip probe fail like this:
> 
>   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
> 
> Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
> Return error code from set_4byte_addr_mode()") it worked just fine.
> 
> Fix this by ignoring -ENOTSUPP in spi_nor_init().
> 
> Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from
> set_4byte_addr_mode()")
> Cc: stable@vger.kernel.org
> Reported-by: Hongyu Ning <hongyu.ning@intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Michael Walle <michael@walle.cc>

-michael

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-09-23  9:34 [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init() Mika Westerberg
  2022-09-23  9:37 ` Michael Walle
@ 2022-10-03  5:04 ` Tudor.Ambarus
  2022-10-03  5:21   ` Mika Westerberg
  2022-10-06  4:41 ` Tudor.Ambarus
  2022-10-18  8:17 ` Miquel Raynal
  3 siblings, 1 reply; 9+ messages in thread
From: Tudor.Ambarus @ 2022-10-03  5:04 UTC (permalink / raw)
  To: mika.westerberg, pratyush
  Cc: michael, miquel.raynal, richard, vigneshr, Takahiro.Kuwano,
	hongyu.ning, linux-mtd

On 9/23/22 12:34, Mika Westerberg wrote:

Hi, Mika,

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The Intel SPI-NOR controller does not support the 4-byte address opcode
> so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
> chip probe fail like this:
> 
>   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
> 
> Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
> Return error code from set_4byte_addr_mode()") it worked just fine.
> 
> Fix this by ignoring -ENOTSUPP in spi_nor_init().
> 
> Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
> Cc: stable@vger.kernel.org
> Reported-by: Hongyu Ning <hongyu.ning@intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
> The previous version of the patch (the revert) can be found here:
> 
>   https://lore.kernel.org/linux-mtd/20220922134824.46758-1-mika.westerberg@linux.intel.com/
> 
> In this version we ignore -ENOTSUPP but the other error codes will be
> passed to the caller.
> 
>  drivers/mtd/spi-nor/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f2c64006f8d7..bee8fc4c9f07 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2724,7 +2724,9 @@ static int spi_nor_init(struct spi_nor *nor)
>                  */
>                 WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
>                           "enabling reset hack; may not recover from unexpected reboots\n");
> -               return nor->params->set_4byte_addr_mode(nor, true);
> +               err = nor->params->set_4byte_addr_mode(nor, true);
> +               if (err && err != -ENOTSUPP)
> +                       return err;
>         }

So as of now if you use a flash larger than 16 MBytes, you can't access above 16 MBytes,
right? What happens at the controller side when it receives a nor->addr_nbytes of value 4?

Shouldn't spi_mem_supports_op() trim the 4-byte ops?

The better fix to me would be to extend the SPI NOR core to support the Extended Address
Register which consists of the 4th byte of memory address when the flash is operated
in 3-byte address mode.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-10-03  5:04 ` Tudor.Ambarus
@ 2022-10-03  5:21   ` Mika Westerberg
  2022-10-03  5:52     ` Tudor.Ambarus
  0 siblings, 1 reply; 9+ messages in thread
From: Mika Westerberg @ 2022-10-03  5:21 UTC (permalink / raw)
  To: Tudor.Ambarus
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Takahiro.Kuwano, hongyu.ning, linux-mtd

Hi,

On Mon, Oct 03, 2022 at 05:04:26AM +0000, Tudor.Ambarus@microchip.com wrote:
> On 9/23/22 12:34, Mika Westerberg wrote:
> 
> Hi, Mika,
> 
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > The Intel SPI-NOR controller does not support the 4-byte address opcode
> > so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
> > chip probe fail like this:
> > 
> >   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
> > 
> > Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
> > Return error code from set_4byte_addr_mode()") it worked just fine.
> > 
> > Fix this by ignoring -ENOTSUPP in spi_nor_init().
> > 
> > Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
> > Cc: stable@vger.kernel.org
> > Reported-by: Hongyu Ning <hongyu.ning@intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
> > The previous version of the patch (the revert) can be found here:
> > 
> >   https://lore.kernel.org/linux-mtd/20220922134824.46758-1-mika.westerberg@linux.intel.com/
> > 
> > In this version we ignore -ENOTSUPP but the other error codes will be
> > passed to the caller.
> > 
> >  drivers/mtd/spi-nor/core.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index f2c64006f8d7..bee8fc4c9f07 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2724,7 +2724,9 @@ static int spi_nor_init(struct spi_nor *nor)
> >                  */
> >                 WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
> >                           "enabling reset hack; may not recover from unexpected reboots\n");
> > -               return nor->params->set_4byte_addr_mode(nor, true);
> > +               err = nor->params->set_4byte_addr_mode(nor, true);
> > +               if (err && err != -ENOTSUPP)
> > +                       return err;
> >         }
> 
> So as of now if you use a flash larger than 16 MBytes, you can't
> access above 16 MBytes, right? What happens at the controller side
> when it receives a nor->addr_nbytes of value 4?

The Intel controller does not really expose any of these operations to
the CPU so the only thing the driver can do is to tell the SPI-NOR core
that this is not supported.

> 
> Shouldn't spi_mem_supports_op() trim the 4-byte ops?
> 
> The better fix to me would be to extend the SPI NOR core to support the Extended Address
> Register which consists of the 4th byte of memory address when the flash is operated
> in 3-byte address mode.

This is also something the Intel controller does not expose to the CPU
so I'm not sure if I have any means to test this approach.

My point is that currently all the Intel SPI users are basically broken
in v6.0 because of the commit 08412e72afba so shouldn't that be dealt
first and then look at any possible improments?

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-10-03  5:21   ` Mika Westerberg
@ 2022-10-03  5:52     ` Tudor.Ambarus
  2022-10-03  6:11       ` Mika Westerberg
  0 siblings, 1 reply; 9+ messages in thread
From: Tudor.Ambarus @ 2022-10-03  5:52 UTC (permalink / raw)
  To: mika.westerberg
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Takahiro.Kuwano, hongyu.ning, linux-mtd

On 10/3/22 08:21, Mika Westerberg wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi,
> 
> On Mon, Oct 03, 2022 at 05:04:26AM +0000, Tudor.Ambarus@microchip.com wrote:
>> On 9/23/22 12:34, Mika Westerberg wrote:
>>
>> Hi, Mika,
>>
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> The Intel SPI-NOR controller does not support the 4-byte address opcode
>>> so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
>>> chip probe fail like this:
>>>
>>>   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
>>>
>>> Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
>>> Return error code from set_4byte_addr_mode()") it worked just fine.
>>>
>>> Fix this by ignoring -ENOTSUPP in spi_nor_init().
>>>
>>> Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
>>> Cc: stable@vger.kernel.org
>>> Reported-by: Hongyu Ning <hongyu.ning@intel.com>
>>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>> ---
>>> The previous version of the patch (the revert) can be found here:
>>>
>>>   https://lore.kernel.org/linux-mtd/20220922134824.46758-1-mika.westerberg@linux.intel.com/
>>>
>>> In this version we ignore -ENOTSUPP but the other error codes will be
>>> passed to the caller.
>>>
>>>  drivers/mtd/spi-nor/core.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>>> index f2c64006f8d7..bee8fc4c9f07 100644
>>> --- a/drivers/mtd/spi-nor/core.c
>>> +++ b/drivers/mtd/spi-nor/core.c
>>> @@ -2724,7 +2724,9 @@ static int spi_nor_init(struct spi_nor *nor)
>>>                  */
>>>                 WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
>>>                           "enabling reset hack; may not recover from unexpected reboots\n");
>>> -               return nor->params->set_4byte_addr_mode(nor, true);
>>> +               err = nor->params->set_4byte_addr_mode(nor, true);
>>> +               if (err && err != -ENOTSUPP)
>>> +                       return err;
>>>         }
>>
>> So as of now if you use a flash larger than 16 MBytes, you can't
>> access above 16 MBytes, right? What happens at the controller side
>> when it receives a nor->addr_nbytes of value 4?
> 
> The Intel controller does not really expose any of these operations to
> the CPU so the only thing the driver can do is to tell the SPI-NOR core
> that this is not supported.

That would be good. And you'll get a non-working flash anyway. But I was
asking something else. There should be an address register in the controller.
If you write 4 address bytes, the forth is ignored, or why does it work? You'll
at least get inconsistent results and always write in the first 16 Mbytes regardless
of the value of the 4th byte of address.
> 
>>
>> Shouldn't spi_mem_supports_op() trim the 4-byte ops?
>>
>> The better fix to me would be to extend the SPI NOR core to support the Extended Address
>> Register which consists of the 4th byte of memory address when the flash is operated
>> in 3-byte address mode.
> 
> This is also something the Intel controller does not expose to the CPU
> so I'm not sure if I have any means to test this approach.

would you please remind me what you mean with "expose to the CPU"? I thought about
that the intel-spi controller can inform the SPI NOR core that it doesn't support
4-byte addresses and then the core to take that information and use the EAR register.
Would that be possible?

> 
> My point is that currently all the Intel SPI users are basically broken
> in v6.0 because of the commit 08412e72afba so shouldn't that be dealt
> first and then look at any possible improments?

Right, I got you from the first time, I just wanted to highlight that the intel-spi
controller and the SPI NOR core behave badly, and they should be fixed.
Your patch fixes a regression of a somewhat working flash, so probably it should be
applied for the time being. But I'd really love a proper fix.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-10-03  5:52     ` Tudor.Ambarus
@ 2022-10-03  6:11       ` Mika Westerberg
  0 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2022-10-03  6:11 UTC (permalink / raw)
  To: Tudor.Ambarus
  Cc: pratyush, michael, miquel.raynal, richard, vigneshr,
	Takahiro.Kuwano, hongyu.ning, linux-mtd

Hi,

On Mon, Oct 03, 2022 at 05:52:42AM +0000, Tudor.Ambarus@microchip.com wrote:
> On 10/3/22 08:21, Mika Westerberg wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hi,
> > 
> > On Mon, Oct 03, 2022 at 05:04:26AM +0000, Tudor.Ambarus@microchip.com wrote:
> >> On 9/23/22 12:34, Mika Westerberg wrote:
> >>
> >> Hi, Mika,
> >>
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>>
> >>> The Intel SPI-NOR controller does not support the 4-byte address opcode
> >>> so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
> >>> chip probe fail like this:
> >>>
> >>>   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
> >>>
> >>> Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
> >>> Return error code from set_4byte_addr_mode()") it worked just fine.
> >>>
> >>> Fix this by ignoring -ENOTSUPP in spi_nor_init().
> >>>
> >>> Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
> >>> Cc: stable@vger.kernel.org
> >>> Reported-by: Hongyu Ning <hongyu.ning@intel.com>
> >>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >>> ---
> >>> The previous version of the patch (the revert) can be found here:
> >>>
> >>>   https://lore.kernel.org/linux-mtd/20220922134824.46758-1-mika.westerberg@linux.intel.com/
> >>>
> >>> In this version we ignore -ENOTSUPP but the other error codes will be
> >>> passed to the caller.
> >>>
> >>>  drivers/mtd/spi-nor/core.c | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >>> index f2c64006f8d7..bee8fc4c9f07 100644
> >>> --- a/drivers/mtd/spi-nor/core.c
> >>> +++ b/drivers/mtd/spi-nor/core.c
> >>> @@ -2724,7 +2724,9 @@ static int spi_nor_init(struct spi_nor *nor)
> >>>                  */
> >>>                 WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
> >>>                           "enabling reset hack; may not recover from unexpected reboots\n");
> >>> -               return nor->params->set_4byte_addr_mode(nor, true);
> >>> +               err = nor->params->set_4byte_addr_mode(nor, true);
> >>> +               if (err && err != -ENOTSUPP)
> >>> +                       return err;
> >>>         }
> >>
> >> So as of now if you use a flash larger than 16 MBytes, you can't
> >> access above 16 MBytes, right? What happens at the controller side
> >> when it receives a nor->addr_nbytes of value 4?
> > 
> > The Intel controller does not really expose any of these operations to
> > the CPU so the only thing the driver can do is to tell the SPI-NOR core
> > that this is not supported.
> 
> That would be good. And you'll get a non-working flash anyway. But I was
> asking something else. There should be an address register in the controller.
> If you write 4 address bytes, the forth is ignored, or why does it work? You'll
> at least get inconsistent results and always write in the first 16 Mbytes regardless
> of the value of the 4th byte of address.

It works because the Intel SPI controller is not a real "SPI" controller
so instead of all the low lever operations it simply exposes bunch of
high-level commands such as read and write. All the rest is handled
internally by the controller so it knows (I think based on the SFDP
information and possibly something else part of the BIOS image) the
supported address length.

> >>
> >> Shouldn't spi_mem_supports_op() trim the 4-byte ops?
> >>
> >> The better fix to me would be to extend the SPI NOR core to support the Extended Address
> >> Register which consists of the 4th byte of memory address when the flash is operated
> >> in 3-byte address mode.
> > 
> > This is also something the Intel controller does not expose to the CPU
> > so I'm not sure if I have any means to test this approach.
> 
> would you please remind me what you mean with "expose to the CPU"? I thought about
> that the intel-spi controller can inform the SPI NOR core that it doesn't support
> 4-byte addresses and then the core to take that information and use the EAR register.
> Would that be possible?

The controller only exposes a couple of high-level operations that we
can perform in software through the register interface. There is no way
to access EAR register or anything like that. This high-level commands
to read and write status register but that's pretty much all.

For completeness here are all the possible commands it supports (in
hardware sequencig mode which is the one used with the modern chips):

0 Read (1 up to 64 bytes by setting FDBC)
1 Reserved
2 Write (1 up to 64 bytes by setting FDBC)
3 4k Block Erase
4 64k Sector erase
5 Read SFDP
6 Read JEDEC ID
7 write status
8 read status
9 RPMC Op1
A RPMC Op2

> > 
> > My point is that currently all the Intel SPI users are basically broken
> > in v6.0 because of the commit 08412e72afba so shouldn't that be dealt
> > first and then look at any possible improments?
> 
> Right, I got you from the first time, I just wanted to highlight that the intel-spi
> controller and the SPI NOR core behave badly, and they should be fixed.
> Your patch fixes a regression of a somewhat working flash, so probably it should be
> applied for the time being. But I'd really love a proper fix.

I'm fine to have a "proper" fix but just wanted to mention that this is
real regression that is currently visible to users.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-09-23  9:34 [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init() Mika Westerberg
  2022-09-23  9:37 ` Michael Walle
  2022-10-03  5:04 ` Tudor.Ambarus
@ 2022-10-06  4:41 ` Tudor.Ambarus
  2022-10-06  4:56   ` Tudor.Ambarus
  2022-10-18  8:17 ` Miquel Raynal
  3 siblings, 1 reply; 9+ messages in thread
From: Tudor.Ambarus @ 2022-10-06  4:41 UTC (permalink / raw)
  To: mika.westerberg, pratyush
  Cc: michael, miquel.raynal, richard, vigneshr, Takahiro.Kuwano,
	hongyu.ning, linux-mtd

Hi, Miquel,

It's too late for me to queue patches for next, but I'd like to have this patch
integrated as it solves a regression. Would you queue it to mtd/next?

Thanks,
ta

On 9/23/22 12:34, Mika Westerberg wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The Intel SPI-NOR controller does not support the 4-byte address opcode
> so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
> chip probe fail like this:
> 
>   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
> 
> Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
> Return error code from set_4byte_addr_mode()") it worked just fine.
> 
> Fix this by ignoring -ENOTSUPP in spi_nor_init().
> 
> Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
> Cc: stable@vger.kernel.org
> Reported-by: Hongyu Ning <hongyu.ning@intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> ---
> The previous version of the patch (the revert) can be found here:
> 
>   https://lore.kernel.org/linux-mtd/20220922134824.46758-1-mika.westerberg@linux.intel.com/
> 
> In this version we ignore -ENOTSUPP but the other error codes will be
> passed to the caller.
> 
>  drivers/mtd/spi-nor/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index f2c64006f8d7..bee8fc4c9f07 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2724,7 +2724,9 @@ static int spi_nor_init(struct spi_nor *nor)
>                  */
>                 WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
>                           "enabling reset hack; may not recover from unexpected reboots\n");
> -               return nor->params->set_4byte_addr_mode(nor, true);
> +               err = nor->params->set_4byte_addr_mode(nor, true);
> +               if (err && err != -ENOTSUPP)
> +                       return err;
>         }
> 
>         return 0;
> --
> 2.35.1
> 

-- 
Cheers,
ta

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-10-06  4:41 ` Tudor.Ambarus
@ 2022-10-06  4:56   ` Tudor.Ambarus
  0 siblings, 0 replies; 9+ messages in thread
From: Tudor.Ambarus @ 2022-10-06  4:56 UTC (permalink / raw)
  To: mika.westerberg, pratyush, miquel.raynal, richard
  Cc: michael, miquel.raynal, richard, vigneshr, Takahiro.Kuwano,
	hongyu.ning, linux-mtd

On 10/6/22 07:41, Tudor.Ambarus@microchip.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi, Miquel,
> 
> It's too late for me to queue patches for next, but I'd like to have this patch
> integrated as it solves a regression. Would you queue it to mtd/next?
> 

A bit of context. You'll see the following checkpatch warning:
WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP

It comes from SPI. We should evaluate if we can s/ENOTSUPP/EOPNOTSUPP
in SPI, but we can do it later on. I prefer this patch instead of reverting
the offending commit, as it does not ignore other errors.

> 
> On 9/23/22 12:34, Mika Westerberg wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> The Intel SPI-NOR controller does not support the 4-byte address opcode
>> so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
>> chip probe fail like this:
>>
>>   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
>>
>> Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
>> Return error code from set_4byte_addr_mode()") it worked just fine.
>>
>> Fix this by ignoring -ENOTSUPP in spi_nor_init().
>>
>> Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
>> Cc: stable@vger.kernel.org
>> Reported-by: Hongyu Ning <hongyu.ning@intel.com>
>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
>> ---
>> The previous version of the patch (the revert) can be found here:
>>
>>   https://lore.kernel.org/linux-mtd/20220922134824.46758-1-mika.westerberg@linux.intel.com/
>>
>> In this version we ignore -ENOTSUPP but the other error codes will be
>> passed to the caller.
>>
>>  drivers/mtd/spi-nor/core.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index f2c64006f8d7..bee8fc4c9f07 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -2724,7 +2724,9 @@ static int spi_nor_init(struct spi_nor *nor)
>>                  */
>>                 WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
>>                           "enabling reset hack; may not recover from unexpected reboots\n");
>> -               return nor->params->set_4byte_addr_mode(nor, true);
>> +               err = nor->params->set_4byte_addr_mode(nor, true);
>> +               if (err && err != -ENOTSUPP)
>> +                       return err;
>>         }
>>
>>         return 0;
>> --
>> 2.35.1
>>
> 
> --
> Cheers,
> ta
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

-- 
Cheers,
ta

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init()
  2022-09-23  9:34 [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init() Mika Westerberg
                   ` (2 preceding siblings ...)
  2022-10-06  4:41 ` Tudor.Ambarus
@ 2022-10-18  8:17 ` Miquel Raynal
  3 siblings, 0 replies; 9+ messages in thread
From: Miquel Raynal @ 2022-10-18  8:17 UTC (permalink / raw)
  To: Mika Westerberg, Tudor Ambarus, Pratyush Yadav
  Cc: Miquel Raynal, Michael Walle, Richard Weinberger,
	Vignesh Raghavendra, Takahiro Kuwano, Hongyu Ning, linux-mtd

On Fri, 2022-09-23 at 09:34:41 UTC, Mika Westerberg wrote:
> The Intel SPI-NOR controller does not support the 4-byte address opcode
> so ->set_4byte_addr_mode() ends up returning -ENOTSUPP and the SPI flash
> chip probe fail like this:
> 
>   [ 12.291082] spi-nor: probe of spi0.0 failed with error -524
> 
> Whereas previously before commit 08412e72afba ("mtd: spi-nor: core:
> Return error code from set_4byte_addr_mode()") it worked just fine.
> 
> Fix this by ignoring -ENOTSUPP in spi_nor_init().
> 
> Fixes: 08412e72afba ("mtd: spi-nor: core: Return error code from set_4byte_addr_mode()")
> Cc: stable@vger.kernel.org
> Reported-by: Hongyu Ning <hongyu.ning@intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Michael Walle <michael@walle.cc>
> Acked-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes, thanks.

Miquel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-10-18  8:17 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23  9:34 [PATCH v2] mtd: spi-nor: core: Ignore -ENOTSUPP in spi_nor_init() Mika Westerberg
2022-09-23  9:37 ` Michael Walle
2022-10-03  5:04 ` Tudor.Ambarus
2022-10-03  5:21   ` Mika Westerberg
2022-10-03  5:52     ` Tudor.Ambarus
2022-10-03  6:11       ` Mika Westerberg
2022-10-06  4:41 ` Tudor.Ambarus
2022-10-06  4:56   ` Tudor.Ambarus
2022-10-18  8:17 ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).