spi-nor: maxronix MX25L12835F support

spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi all,

I faced an issue with a SPI flash on our board. We use a macronix
MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
the MX25L12805D [2].

The newer MX25L12835F has support for dual/quad read mode and RDSFDP
while the older doesn't.

I thought that I could do a fixup with a device specific
post_bfpt_fixups() call but by now this seems not possible. The older
MX25L12805D has no flags set that allows a call to
spi_nor_sfdp_init_params() and implements the fixup.

Has anyone an idea how to solve this?


[1] https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
[2] https://www.mxic.com.tw/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf

Thanks

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Pratyush Yadav]

On 15/02/21 10:53PM, Heiko Thiery wrote:
> Hi all,
> 
> I faced an issue with a SPI flash on our board. We use a macronix
> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> the MX25L12805D [2].
> 
> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> while the older doesn't.
> 
> I thought that I could do a fixup with a device specific
> post_bfpt_fixups() call but by now this seems not possible. The older
> MX25L12805D has no flags set that allows a call to
> spi_nor_sfdp_init_params() and implements the fixup.
> 
> Has anyone an idea how to solve this?

The post_sfdp fixup is always run regardless of whether the flash has 
SFDP or not. You can try putting your flash-specific fixups there.

> 
> 
> [1] https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
> [2] https://www.mxic.com.tw/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf

Both these links are broken.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi Pratyush et all,

Am Di., 16. Feb. 2021 um 10:27 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
>
> On 15/02/21 10:53PM, Heiko Thiery wrote:
> > Hi all,
> >
> > I faced an issue with a SPI flash on our board. We use a macronix
> > MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > the MX25L12805D [2].
> >
> > The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > while the older doesn't.
> >
> > I thought that I could do a fixup with a device specific
> > post_bfpt_fixups() call but by now this seems not possible. The older
> > MX25L12805D has no flags set that allows a call to
> > spi_nor_sfdp_init_params() and implements the fixup.
> >
> > Has anyone an idea how to solve this?
>
> The post_sfdp fixup is always run regardless of whether the flash has
> SFDP or not. You can try putting your flash-specific fixups there.

But as far as I can see, this function (spi_nor_post_sfdp_fixups) has
no sfdp information arguments to decide on what to do. Only
spi_nor_post_bfpt_fixups() has these parameters but this is only
called inside spi_nor_parse_bfpt().

>
> >
> >
> > [1] https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
> > [2] https://www.mxic.com.tw/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf
>
> Both these links are broken.

Works here for me. Nethertheless here are shorter ones:

[1] https://bit.ly/3jQFNLL
[2] https://bit.ly/2OKiGH9

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Michael Walle]

Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> On 15/02/21 10:53PM, Heiko Thiery wrote:
>> Hi all,
>> 
>> I faced an issue with a SPI flash on our board. We use a macronix
>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
>> the MX25L12805D [2].
>> 
>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
>> while the older doesn't.
>> 
>> I thought that I could do a fixup with a device specific
>> post_bfpt_fixups() call but by now this seems not possible. The older
>> MX25L12805D has no flags set that allows a call to
>> spi_nor_sfdp_init_params() and implements the fixup.
>> 
>> Has anyone an idea how to solve this?
> 
> The post_sfdp fixup is always run regardless of whether the flash has
> SFDP or not. You can try putting your flash-specific fixups there.

Well the problem here is, that the SFDP setup is skipped though the
flash would support SFDP. If the jedec id wasn't already in the table,
there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
parsed. But because there is already the legacy device (which likely
doesn't support SFDP) it really doesn't fit.

Its unclear to me, why the SFDP is only parsed if one of the
SPI_NOR_*_READ flags are set.

>> [1] 
>> https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
>> [2] 
>> https://www.mxic.com.tw/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf
> 
> Both these links are broken.

mh, they work for me.

-michael

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

Re: spi-nor: maxronix MX25L12835F support

[Pratyush Yadav]

On 16/02/21 10:48AM, Michael Walle wrote:
> Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> > On 15/02/21 10:53PM, Heiko Thiery wrote:
> > > Hi all,
> > > 
> > > I faced an issue with a SPI flash on our board. We use a macronix
> > > MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > > the MX25L12805D [2].
> > > 
> > > The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > > while the older doesn't.
> > > 
> > > I thought that I could do a fixup with a device specific
> > > post_bfpt_fixups() call but by now this seems not possible. The older
> > > MX25L12805D has no flags set that allows a call to
> > > spi_nor_sfdp_init_params() and implements the fixup.
> > > 
> > > Has anyone an idea how to solve this?
> > 
> > The post_sfdp fixup is always run regardless of whether the flash has
> > SFDP or not. You can try putting your flash-specific fixups there.
> 
> Well the problem here is, that the SFDP setup is skipped though the
> flash would support SFDP. If the jedec id wasn't already in the table,
> there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> parsed. But because there is already the legacy device (which likely
> doesn't support SFDP) it really doesn't fit.

Is it possible to differentiate between the two flashes in any way? If 
so you can use the init_params() fixup to check that add the flags for 
the new flash. Modifying nor->info feels kind of wrong but it is an 
acceptable compromise in this situation IMO.

> 
> Its unclear to me, why the SFDP is only parsed if one of the
> SPI_NOR_*_READ flags are set.

I don't know either. Probably to differentiate between legacy flashes 
that don't support SFDP at all.

> 
> > > [1] https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
> > > [2] https://www.mxic.com.tw/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf
> > 
> > Both these links are broken.
> 
> mh, they work for me.

Ah yes they do. My terminal emulator didn't parse them correctly and 
opened something different in the browser.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: spi-nor: maxronix MX25L12835F support

[Pratyush Yadav]

On 16/02/21 03:46PM, Pratyush Yadav wrote:
> On 16/02/21 10:48AM, Michael Walle wrote:
> > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> > > On 15/02/21 10:53PM, Heiko Thiery wrote:
> > > > Hi all,
> > > > 
> > > > I faced an issue with a SPI flash on our board. We use a macronix
> > > > MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > > > the MX25L12805D [2].
> > > > 
> > > > The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > > > while the older doesn't.
> > > > 
> > > > I thought that I could do a fixup with a device specific
> > > > post_bfpt_fixups() call but by now this seems not possible. The older
> > > > MX25L12805D has no flags set that allows a call to
> > > > spi_nor_sfdp_init_params() and implements the fixup.
> > > > 
> > > > Has anyone an idea how to solve this?
> > > 
> > > The post_sfdp fixup is always run regardless of whether the flash has
> > > SFDP or not. You can try putting your flash-specific fixups there.
> > 
> > Well the problem here is, that the SFDP setup is skipped though the
> > flash would support SFDP. If the jedec id wasn't already in the table,
> > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > parsed. But because there is already the legacy device (which likely
> > doesn't support SFDP) it really doesn't fit.
> 
> Is it possible to differentiate between the two flashes in any way? If 
> so you can use the init_params() fixup to check that add the flags for 
> the new flash. Modifying nor->info feels kind of wrong but it is an 
> acceptable compromise in this situation IMO.

I take that back. nor->info is declared as const and let's keep it that 
way. Maybe you can add something in nor->flags to indicate we want to 
parse SFDP? Or maybe there is some other way to indicate SFDP support? 
Dunno...

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi,

Am Di., 16. Feb. 2021 um 11:20 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
>
> On 16/02/21 03:46PM, Pratyush Yadav wrote:
> > On 16/02/21 10:48AM, Michael Walle wrote:
> > > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> > > > On 15/02/21 10:53PM, Heiko Thiery wrote:
> > > > > Hi all,
> > > > >
> > > > > I faced an issue with a SPI flash on our board. We use a macronix
> > > > > MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > > > > the MX25L12805D [2].
> > > > >
> > > > > The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > > > > while the older doesn't.
> > > > >
> > > > > I thought that I could do a fixup with a device specific
> > > > > post_bfpt_fixups() call but by now this seems not possible. The older
> > > > > MX25L12805D has no flags set that allows a call to
> > > > > spi_nor_sfdp_init_params() and implements the fixup.
> > > > >
> > > > > Has anyone an idea how to solve this?
> > > >
> > > > The post_sfdp fixup is always run regardless of whether the flash has
> > > > SFDP or not. You can try putting your flash-specific fixups there.
> > >
> > > Well the problem here is, that the SFDP setup is skipped though the
> > > flash would support SFDP. If the jedec id wasn't already in the table,
> > > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > > parsed. But because there is already the legacy device (which likely
> > > doesn't support SFDP) it really doesn't fit.
> >
> > Is it possible to differentiate between the two flashes in any way? If
> > so you can use the init_params() fixup to check that add the flags for
> > the new flash. Modifying nor->info feels kind of wrong but it is an
> > acceptable compromise in this situation IMO.
>
> I take that back. nor->info is declared as const and let's keep it that
> way. Maybe you can add something in nor->flags to indicate we want to
> parse SFDP? Or maybe there is some other way to indicate SFDP support?

That was also my intention. I thought about something like
SPI_NOR_FORCE_SFDP. But what will happen if we try to parse and the
legacy device does not support SFDP read?

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Pratyush Yadav]

On 16/02/21 11:41AM, Heiko Thiery wrote:
> Hi,
> 
> Am Di., 16. Feb. 2021 um 11:20 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
> >
> > On 16/02/21 03:46PM, Pratyush Yadav wrote:
> > > On 16/02/21 10:48AM, Michael Walle wrote:
> > > > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> > > > > On 15/02/21 10:53PM, Heiko Thiery wrote:
> > > > > > Hi all,
> > > > > >
> > > > > > I faced an issue with a SPI flash on our board. We use a macronix
> > > > > > MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > > > > > the MX25L12805D [2].
> > > > > >
> > > > > > The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > > > > > while the older doesn't.
> > > > > >
> > > > > > I thought that I could do a fixup with a device specific
> > > > > > post_bfpt_fixups() call but by now this seems not possible. The older
> > > > > > MX25L12805D has no flags set that allows a call to
> > > > > > spi_nor_sfdp_init_params() and implements the fixup.
> > > > > >
> > > > > > Has anyone an idea how to solve this?
> > > > >
> > > > > The post_sfdp fixup is always run regardless of whether the flash has
> > > > > SFDP or not. You can try putting your flash-specific fixups there.
> > > >
> > > > Well the problem here is, that the SFDP setup is skipped though the
> > > > flash would support SFDP. If the jedec id wasn't already in the table,
> > > > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > > > parsed. But because there is already the legacy device (which likely
> > > > doesn't support SFDP) it really doesn't fit.
> > >
> > > Is it possible to differentiate between the two flashes in any way? If
> > > so you can use the init_params() fixup to check that add the flags for
> > > the new flash. Modifying nor->info feels kind of wrong but it is an
> > > acceptable compromise in this situation IMO.
> >
> > I take that back. nor->info is declared as const and let's keep it that
> > way. Maybe you can add something in nor->flags to indicate we want to
> > parse SFDP? Or maybe there is some other way to indicate SFDP support?
> 
> That was also my intention. I thought about something like
> SPI_NOR_FORCE_SFDP. But what will happen if we try to parse and the
> legacy device does not support SFDP read?

Most likely it will not do anything and SFDP parsing will fail because 
it can't find the SFDP signature. But let's try to avoid that if 
possible. Is it possible to differentiate between the two flashes in any 
way? If it is possible, then just set the flag for the new device and 
leave the legacy device alone.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi,

Am Di., 16. Feb. 2021 um 11:48 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
>
> On 16/02/21 11:41AM, Heiko Thiery wrote:
> > Hi,
> >
> > Am Di., 16. Feb. 2021 um 11:20 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
> > >
> > > On 16/02/21 03:46PM, Pratyush Yadav wrote:
> > > > On 16/02/21 10:48AM, Michael Walle wrote:
> > > > > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> > > > > > On 15/02/21 10:53PM, Heiko Thiery wrote:
> > > > > > > Hi all,
> > > > > > >
> > > > > > > I faced an issue with a SPI flash on our board. We use a macronix
> > > > > > > MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > > > > > > the MX25L12805D [2].
> > > > > > >
> > > > > > > The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > > > > > > while the older doesn't.
> > > > > > >
> > > > > > > I thought that I could do a fixup with a device specific
> > > > > > > post_bfpt_fixups() call but by now this seems not possible. The older
> > > > > > > MX25L12805D has no flags set that allows a call to
> > > > > > > spi_nor_sfdp_init_params() and implements the fixup.
> > > > > > >
> > > > > > > Has anyone an idea how to solve this?
> > > > > >
> > > > > > The post_sfdp fixup is always run regardless of whether the flash has
> > > > > > SFDP or not. You can try putting your flash-specific fixups there.
> > > > >
> > > > > Well the problem here is, that the SFDP setup is skipped though the
> > > > > flash would support SFDP. If the jedec id wasn't already in the table,
> > > > > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > > > > parsed. But because there is already the legacy device (which likely
> > > > > doesn't support SFDP) it really doesn't fit.
> > > >
> > > > Is it possible to differentiate between the two flashes in any way? If
> > > > so you can use the init_params() fixup to check that add the flags for
> > > > the new flash. Modifying nor->info feels kind of wrong but it is an
> > > > acceptable compromise in this situation IMO.
> > >
> > > I take that back. nor->info is declared as const and let's keep it that
> > > way. Maybe you can add something in nor->flags to indicate we want to
> > > parse SFDP? Or maybe there is some other way to indicate SFDP support?
> >
> > That was also my intention. I thought about something like
> > SPI_NOR_FORCE_SFDP. But what will happen if we try to parse and the
> > legacy device does not support SFDP read?
>
> Most likely it will not do anything and SFDP parsing will fail because
> it can't find the SFDP signature. But let's try to avoid that if
> possible. Is it possible to differentiate between the two flashes in any
> way? If it is possible, then just set the flag for the new device and
> leave the legacy device alone.

Is there a good reason not to do the SFDP parsing in general? At the
moment I have no other idea how to differentiate the two flashes.

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Pratyush Yadav]

On 16/02/21 11:55AM, Heiko Thiery wrote:
> Hi,
> 
> Am Di., 16. Feb. 2021 um 11:48 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
> >
> > On 16/02/21 11:41AM, Heiko Thiery wrote:
> > > Hi,
> > >
> > > Am Di., 16. Feb. 2021 um 11:20 Uhr schrieb Pratyush Yadav <p.yadav@ti.com>:
> > > >
> > > > On 16/02/21 03:46PM, Pratyush Yadav wrote:
> > > > > On 16/02/21 10:48AM, Michael Walle wrote:
> > > > > > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> > > > > > > On 15/02/21 10:53PM, Heiko Thiery wrote:
> > > > > > > > Hi all,
> > > > > > > >
> > > > > > > > I faced an issue with a SPI flash on our board. We use a macronix
> > > > > > > > MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > > > > > > > the MX25L12805D [2].
> > > > > > > >
> > > > > > > > The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > > > > > > > while the older doesn't.
> > > > > > > >
> > > > > > > > I thought that I could do a fixup with a device specific
> > > > > > > > post_bfpt_fixups() call but by now this seems not possible. The older
> > > > > > > > MX25L12805D has no flags set that allows a call to
> > > > > > > > spi_nor_sfdp_init_params() and implements the fixup.
> > > > > > > >
> > > > > > > > Has anyone an idea how to solve this?
> > > > > > >
> > > > > > > The post_sfdp fixup is always run regardless of whether the flash has
> > > > > > > SFDP or not. You can try putting your flash-specific fixups there.
> > > > > >
> > > > > > Well the problem here is, that the SFDP setup is skipped though the
> > > > > > flash would support SFDP. If the jedec id wasn't already in the table,
> > > > > > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > > > > > parsed. But because there is already the legacy device (which likely
> > > > > > doesn't support SFDP) it really doesn't fit.
> > > > >
> > > > > Is it possible to differentiate between the two flashes in any way? If
> > > > > so you can use the init_params() fixup to check that add the flags for
> > > > > the new flash. Modifying nor->info feels kind of wrong but it is an
> > > > > acceptable compromise in this situation IMO.
> > > >
> > > > I take that back. nor->info is declared as const and let's keep it that
> > > > way. Maybe you can add something in nor->flags to indicate we want to
> > > > parse SFDP? Or maybe there is some other way to indicate SFDP support?
> > >
> > > That was also my intention. I thought about something like
> > > SPI_NOR_FORCE_SFDP. But what will happen if we try to parse and the
> > > legacy device does not support SFDP read?
> >
> > Most likely it will not do anything and SFDP parsing will fail because
> > it can't find the SFDP signature. But let's try to avoid that if
> > possible. Is it possible to differentiate between the two flashes in any
> > way? If it is possible, then just set the flag for the new device and
> > leave the legacy device alone.
> 
> Is there a good reason not to do the SFDP parsing in general? At the
> moment I have no other idea how to differentiate the two flashes.

The fact that the flash (legacy one) does not support the SFDP command 
at all is reason enough for me. Why issue commands that a flash doesn't 
support?

But if you don't manage to find anything better, I guess the SFDP 
command can be used to tell the flashes apart. But don't rely on 
spi_nor_parse_sfdp() to do so. Do it in the default_init() hook. Of 
course, details can be better fleshed out when there is an actual patch 
to read through :-)

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

Hi, all,

+zhengxunli, juliensu & ycllin

On 2/16/21 11:48 AM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2021-02-16 10:27, schrieb Pratyush Yadav:
>> On 15/02/21 10:53PM, Heiko Thiery wrote:
>>> Hi all,
>>>
>>> I faced an issue with a SPI flash on our board. We use a macronix
>>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
>>> the MX25L12805D [2].
>>>
>>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
>>> while the older doesn't.
>>>
>>> I thought that I could do a fixup with a device specific
>>> post_bfpt_fixups() call but by now this seems not possible. The older
>>> MX25L12805D has no flags set that allows a call to
>>> spi_nor_sfdp_init_params() and implements the fixup.
>>>
>>> Has anyone an idea how to solve this?

Maybe macronix can help with some suggestions on how to differentiate
between flashes at runtime.

My first thought is to introduce a SPI_NOR_HAS_SFDP flag. For the flash
that doesn't support SFDP tables, there should be no functional change,
for the one that support SFDP it should fill the properties from the
SFDP tables.

>>
>> The post_sfdp fixup is always run regardless of whether the flash has
>> SFDP or not. You can try putting your flash-specific fixups there.
> 
> Well the problem here is, that the SFDP setup is skipped though the
> flash would support SFDP. If the jedec id wasn't already in the table,
> there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> parsed. But because there is already the legacy device (which likely
> doesn't support SFDP) it really doesn't fit.
> 
> Its unclear to me, why the SFDP is only parsed if one of the
> SPI_NOR_*_READ flags are set.

My guess is that a new SFDP flag was not necessary. SFDP defines multiple
tables, but there is just one that is mandatory, BFPT. BFPT defines DUAL
and QUAD parameters. From the spi-nor code, a BFPT without DUAL or QUAD
support doesn't make sense, even though DUAL or QUAD are not mandatory
in BFPT as I see in the standard. So probably it was just a way to avoid
adding a extra flag. We have to check the git history for a more accurate
description, this was just a guess.

Thinking loud, now we do a static initialization of flash params, that
can be overwritten dynamically by SFDP. How about doing the params init
the other way around. Try first to dynamically discover the params via
SFDP, and if SFDP fails or if it is not defined, do the static init via
flags. That would spare some code. And new flash IDs will have less flags
declared, and we'll better track faulty SFDP flashes.

Cheers,
ta

> 
>>> [1]
>>> https://www.macronix.com/Lists/Datasheet/Attachments/7397/MX25L12835F,%203V,%20128Mb,%20v1.6.pdf
>>> [2]
>>> https://www.mxic.com.tw/Lists/Datasheet/Attachments/7321/MX25L12805D,%203V,%20128Mb,%20v1.2.pdf
>>
>> Both these links are broken.
> 
> mh, they work for me.
> 
> -michael
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

Re: spi-nor: maxronix MX25L12835F support

[zhengxunli]

Hi,

<Tudor.Ambarus@microchip.com> wrote on 2021/02/16 下午 07:15:33:

> <Tudor.Ambarus@microchip.com> 
> 2021/02/16 下午 07:15
> 
> To
> 
> <michael@walle.cc>, <p.yadav@ti.com>, <ycllin@mxic.com.tw>, 
> <zhengxunli@mxic.com.tw>, <juliensu@mxic.com.tw>, 
> 
> cc
> 
> <heiko.thiery@gmail.com>, <linux-mtd@lists.infradead.org>
> 
> Subject
> 
> Re: spi-nor: maxronix MX25L12835F support
> 
> Hi, all,
> 
> +zhengxunli, juliensu & ycllin
> 
> On 2/16/21 11:48 AM, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you 
> know the content is safe
> > 
> > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> >> On 15/02/21 10:53PM, Heiko Thiery wrote:
> >>> Hi all,
> >>>
> >>> I faced an issue with a SPI flash on our board. We use a macronix
> >>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> >>> the MX25L12805D [2].
> >>>
> >>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> >>> while the older doesn't.
> >>>
> >>> I thought that I could do a fixup with a device specific
> >>> post_bfpt_fixups() call but by now this seems not possible. The 
older
> >>> MX25L12805D has no flags set that allows a call to
> >>> spi_nor_sfdp_init_params() and implements the fixup.
> >>>
> >>> Has anyone an idea how to solve this?
> 
> Maybe macronix can help with some suggestions on how to differentiate
> between flashes at runtime.

In fact, the duplicate ID also caused us trouble. Maybe you can refer to
our colleagues' patches and add a new ID before the old ID.

https://patchwork.ozlabs.org/project/linux-mtd/patch/1587631123-25474-2-git-send-email-masonccyang@mxic.com.tw/

> My first thought is to introduce a SPI_NOR_HAS_SFDP flag. For the flash
> that doesn't support SFDP tables, there should be no functional change,
> for the one that support SFDP it should fill the properties from the
> SFDP tables.
> 
> >>
> >> The post_sfdp fixup is always run regardless of whether the flash has
> >> SFDP or not. You can try putting your flash-specific fixups there.
> > 
> > Well the problem here is, that the SFDP setup is skipped though the
> > flash would support SFDP. If the jedec id wasn't already in the table,
> > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > parsed. But because there is already the legacy device (which likely
> > doesn't support SFDP) it really doesn't fit.
> > 
> > Its unclear to me, why the SFDP is only parsed if one of the
> > SPI_NOR_*_READ flags are set.
> 
> My guess is that a new SFDP flag was not necessary. SFDP defines 
multiple
> tables, but there is just one that is mandatory, BFPT. BFPT defines DUAL
> and QUAD parameters. From the spi-nor code, a BFPT without DUAL or QUAD
> support doesn't make sense, even though DUAL or QUAD are not mandatory
> in BFPT as I see in the standard. So probably it was just a way to avoid
> adding a extra flag. We have to check the git history for a more 
accurate
> description, this was just a guess.
> 
> Thinking loud, now we do a static initialization of flash params, that
> can be overwritten dynamically by SFDP. How about doing the params init
> the other way around. Try first to dynamically discover the params via
> SFDP, and if SFDP fails or if it is not defined, do the static init via
> flags. That would spare some code. And new flash IDs will have less 
flags
> declared, and we'll better track faulty SFDP flashes.
> 

Thanks,
Zhengxun


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi Zhengxun,

Am Do., 18. Feb. 2021 um 06:47 Uhr schrieb <zhengxunli@mxic.com.tw>:
>
> Hi,
>
> <Tudor.Ambarus@microchip.com> wrote on 2021/02/16 下午 07:15:33:
>
> > <Tudor.Ambarus@microchip.com>
> > 2021/02/16 下午 07:15
> >
> > To
> >
> > <michael@walle.cc>, <p.yadav@ti.com>, <ycllin@mxic.com.tw>,
> > <zhengxunli@mxic.com.tw>, <juliensu@mxic.com.tw>,
> >
> > cc
> >
> > <heiko.thiery@gmail.com>, <linux-mtd@lists.infradead.org>
> >
> > Subject
> >
> > Re: spi-nor: maxronix MX25L12835F support
> >
> > Hi, all,
> >
> > +zhengxunli, juliensu & ycllin
> >
> > On 2/16/21 11:48 AM, Michael Walle wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > >
> > > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> > >> On 15/02/21 10:53PM, Heiko Thiery wrote:
> > >>> Hi all,
> > >>>
> > >>> I faced an issue with a SPI flash on our board. We use a macronix
> > >>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> > >>> the MX25L12805D [2].
> > >>>
> > >>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> > >>> while the older doesn't.
> > >>>
> > >>> I thought that I could do a fixup with a device specific
> > >>> post_bfpt_fixups() call but by now this seems not possible. The
> older
> > >>> MX25L12805D has no flags set that allows a call to
> > >>> spi_nor_sfdp_init_params() and implements the fixup.
> > >>>
> > >>> Has anyone an idea how to solve this?
> >
> > Maybe macronix can help with some suggestions on how to differentiate
> > between flashes at runtime.
>
> In fact, the duplicate ID also caused us trouble. Maybe you can refer to
> our colleagues' patches and add a new ID before the old ID.

But doesn't that mean I have to explicitly set the right used flash
device in my dtb? Currently I have set "jedec,spi-nor" and let the
mtd/spi subsystem find the right values to use. As we have 2 other
devices as alternatives I wouldn't like to do that.

And what about the old device. Will this one be correctly configured
when detecting automatically?

>
> https://patchwork.ozlabs.org/project/linux-mtd/patch/1587631123-25474-2-git-send-email-masonccyang@mxic.com.tw/
>
> > My first thought is to introduce a SPI_NOR_HAS_SFDP flag. For the flash
> > that doesn't support SFDP tables, there should be no functional change,
> > for the one that support SFDP it should fill the properties from the
> > SFDP tables.
> >
> > >>
> > >> The post_sfdp fixup is always run regardless of whether the flash has
> > >> SFDP or not. You can try putting your flash-specific fixups there.
> > >
> > > Well the problem here is, that the SFDP setup is skipped though the
> > > flash would support SFDP. If the jedec id wasn't already in the table,
> > > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > > parsed. But because there is already the legacy device (which likely
> > > doesn't support SFDP) it really doesn't fit.
> > >
> > > Its unclear to me, why the SFDP is only parsed if one of the
> > > SPI_NOR_*_READ flags are set.
> >
> > My guess is that a new SFDP flag was not necessary. SFDP defines
> multiple
> > tables, but there is just one that is mandatory, BFPT. BFPT defines DUAL
> > and QUAD parameters. From the spi-nor code, a BFPT without DUAL or QUAD
> > support doesn't make sense, even though DUAL or QUAD are not mandatory
> > in BFPT as I see in the standard. So probably it was just a way to avoid
> > adding a extra flag. We have to check the git history for a more
> accurate
> > description, this was just a guess.
> >
> > Thinking loud, now we do a static initialization of flash params, that
> > can be overwritten dynamically by SFDP. How about doing the params init
> > the other way around. Try first to dynamically discover the params via
> > SFDP, and if SFDP fails or if it is not defined, do the static init via
> > flags. That would spare some code. And new flash IDs will have less
> flags
> > declared, and we'll better track faulty SFDP flashes.
> >
>
> Thanks,
> Zhengxun

Thank you
-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi Tudor and all,

Am Di., 16. Feb. 2021 um 12:15 Uhr schrieb <Tudor.Ambarus@microchip.com>:
>
> Hi, all,
>
> +zhengxunli, juliensu & ycllin
>
> On 2/16/21 11:48 AM, Michael Walle wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Am 2021-02-16 10:27, schrieb Pratyush Yadav:
> >> On 15/02/21 10:53PM, Heiko Thiery wrote:
> >>> Hi all,
> >>>
> >>> I faced an issue with a SPI flash on our board. We use a macronix
> >>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> >>> the MX25L12805D [2].
> >>>
> >>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> >>> while the older doesn't.
> >>>
> >>> I thought that I could do a fixup with a device specific
> >>> post_bfpt_fixups() call but by now this seems not possible. The older
> >>> MX25L12805D has no flags set that allows a call to
> >>> spi_nor_sfdp_init_params() and implements the fixup.
> >>>
> >>> Has anyone an idea how to solve this?
>
> Maybe macronix can help with some suggestions on how to differentiate
> between flashes at runtime.
>
> My first thought is to introduce a SPI_NOR_HAS_SFDP flag. For the flash
> that doesn't support SFDP tables, there should be no functional change,
> for the one that support SFDP it should fill the properties from the
> SFDP tables.
>
> >>
> >> The post_sfdp fixup is always run regardless of whether the flash has
> >> SFDP or not. You can try putting your flash-specific fixups there.
> >
> > Well the problem here is, that the SFDP setup is skipped though the
> > flash would support SFDP. If the jedec id wasn't already in the table,
> > there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
> > parsed. But because there is already the legacy device (which likely
> > doesn't support SFDP) it really doesn't fit.
> >
> > Its unclear to me, why the SFDP is only parsed if one of the
> > SPI_NOR_*_READ flags are set.
>
> My guess is that a new SFDP flag was not necessary. SFDP defines multiple
> tables, but there is just one that is mandatory, BFPT. BFPT defines DUAL
> and QUAD parameters. From the spi-nor code, a BFPT without DUAL or QUAD
> support doesn't make sense, even though DUAL or QUAD are not mandatory
> in BFPT as I see in the standard. So probably it was just a way to avoid
> adding a extra flag. We have to check the git history for a more accurate
> description, this was just a guess.
>
> Thinking loud, now we do a static initialization of flash params, that
> can be overwritten dynamically by SFDP. How about doing the params init
> the other way around. Try first to dynamically discover the params via
> SFDP, and if SFDP fails or if it is not defined, do the static init via
> flags. That would spare some code. And new flash IDs will have less flags
> declared, and we'll better track faulty SFDP flashes.

I am a newbie but it sounds reasonable. I made a first attempt and
reversed the order. But this is not enough. After a few debug cycles I
found a few settings that are probably not automatically set by the
SFDP detection and were set in spi_nor_info_init_params() before.


I was able to find the following lines as the cause.


---- 8< ----
 /**
  * spi_nor_info_init_params() - Initialize the flash's parameters and settings
@@ -3094,14 +3095,18 @@ static int spi_nor_init_params(struct spi_nor *nor)
        if (!nor->params)
                return -ENOMEM;

-       spi_nor_info_init_params(nor);
+       nor->params->setup = spi_nor_default_setup;
+       nor->params->writesize = 1;

-       spi_nor_manufacturer_init_params(nor);
+       /* Page Program settings. */
+       nor->params->hwcaps.mask |= SNOR_HWCAPS_PP;
+       spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP],
+                               SPINOR_OP_PP, SNOR_PROTO_1_1_1);
+
+       if (spi_nor_parse_sfdp(nor, nor->params))
+               spi_nor_info_init_params(nor);

-       if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-                                SPI_NOR_OCTAL_READ |
SPI_NOR_OCTAL_DTR_READ)) &&
-           !(nor->info->flags & SPI_NOR_SKIP_SFDP))
-               spi_nor_sfdp_init_params(nor);
+       spi_nor_manufacturer_init_params(nor);

        spi_nor_post_sfdp_fixups(nor);
---- 8< ----

Now I am trying to understand if this makes sense or what is going
wrong. Does anyone here have a hint at which point this would be
correct or if it should also be detected by the SFDP?


--
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

Hi, Zhengxun, Mason,

On 2/18/21 7:45 AM, zhengxunli@mxic.com.tw wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi,
> 
> <Tudor.Ambarus@microchip.com> wrote on 2021/02/16 下午 07:15:33:
> 
>> <Tudor.Ambarus@microchip.com>
>> 2021/02/16 下午 07:15
>>
>> To
>>
>> <michael@walle.cc>, <p.yadav@ti.com>, <ycllin@mxic.com.tw>,
>> <zhengxunli@mxic.com.tw>, <juliensu@mxic.com.tw>,
>>
>> cc
>>
>> <heiko.thiery@gmail.com>, <linux-mtd@lists.infradead.org>
>>
>> Subject
>>
>> Re: spi-nor: maxronix MX25L12835F support
>>
>> Hi, all,
>>
>> +zhengxunli, juliensu & ycllin
>>
>> On 2/16/21 11:48 AM, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>>
>>> Am 2021-02-16 10:27, schrieb Pratyush Yadav:
>>>> On 15/02/21 10:53PM, Heiko Thiery wrote:
>>>>> Hi all,
>>>>>
>>>>> I faced an issue with a SPI flash on our board. We use a macronix
>>>>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
>>>>> the MX25L12805D [2].
>>>>>
>>>>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
>>>>> while the older doesn't.
>>>>>
>>>>> I thought that I could do a fixup with a device specific
>>>>> post_bfpt_fixups() call but by now this seems not possible. The
> older
>>>>> MX25L12805D has no flags set that allows a call to
>>>>> spi_nor_sfdp_init_params() and implements the fixup.
>>>>>
>>>>> Has anyone an idea how to solve this?
>>
>> Maybe macronix can help with some suggestions on how to differentiate
>> between flashes at runtime.
> 
> In fact, the duplicate ID also caused us trouble. Maybe you can refer to
> our colleagues' patches and add a new ID before the old ID.
> 
> https://patchwork.ozlabs.org/project/linux-mtd/patch/1587631123-25474-2-git-send-email-masonccyang@mxic.com.tw/
> 

No, that patch is wrong because mx66l51235l will no longer be
detected. I see that mx66l51235l also supports SECT_4K, as
mx25l51245g does. Do you know why Mason added a new flash ID?
Was it just to get the new flash name? I'll send a patch right
now to correct this.

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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

On 2/18/21 9:56 AM, Tudor Ambarus - M18064 wrote:
> Hi, Zhengxun, Mason,
> 
> On 2/18/21 7:45 AM, zhengxunli@mxic.com.tw wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi,
>>
>> <Tudor.Ambarus@microchip.com> wrote on 2021/02/16 下午 07:15:33:
>>
>>> <Tudor.Ambarus@microchip.com>
>>> 2021/02/16 下午 07:15
>>>
>>> To
>>>
>>> <michael@walle.cc>, <p.yadav@ti.com>, <ycllin@mxic.com.tw>,
>>> <zhengxunli@mxic.com.tw>, <juliensu@mxic.com.tw>,
>>>
>>> cc
>>>
>>> <heiko.thiery@gmail.com>, <linux-mtd@lists.infradead.org>
>>>
>>> Subject
>>>
>>> Re: spi-nor: maxronix MX25L12835F support
>>>
>>> Hi, all,
>>>
>>> +zhengxunli, juliensu & ycllin
>>>
>>> On 2/16/21 11:48 AM, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>>> know the content is safe
>>>>
>>>> Am 2021-02-16 10:27, schrieb Pratyush Yadav:
>>>>> On 15/02/21 10:53PM, Heiko Thiery wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> I faced an issue with a SPI flash on our board. We use a macronix
>>>>>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
>>>>>> the MX25L12805D [2].
>>>>>>
>>>>>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
>>>>>> while the older doesn't.
>>>>>>
>>>>>> I thought that I could do a fixup with a device specific
>>>>>> post_bfpt_fixups() call but by now this seems not possible. The
>> older
>>>>>> MX25L12805D has no flags set that allows a call to
>>>>>> spi_nor_sfdp_init_params() and implements the fixup.
>>>>>>
>>>>>> Has anyone an idea how to solve this?
>>>
>>> Maybe macronix can help with some suggestions on how to differentiate
>>> between flashes at runtime.
>>
>> In fact, the duplicate ID also caused us trouble. Maybe you can refer to
>> our colleagues' patches and add a new ID before the old ID.
>>
>> https://patchwork.ozlabs.org/project/linux-mtd/patch/1587631123-25474-2-git-send-email-masonccyang@mxic.com.tw/
>>
> 
> No, that patch is wrong because mx66l51235l will no longer be
> detected. I see that mx66l51235l also supports SECT_4K, as
> mx25l51245g does. Do you know why Mason added a new flash ID?
> Was it just to get the new flash name? I'll send a patch right
> now to correct this.
> 

I'll wait a bit, I see there's something else about mx66l51235l:
https://linux-mtd.infradead.narkive.com/90mgDZkV/patch-mtd-spi-nor-fix-options-for-mx66l51235f#post1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

Hi,

+ Vignesh

On 2/18/21 9:43 AM, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor and all,
> 
> Am Di., 16. Feb. 2021 um 12:15 Uhr schrieb <Tudor.Ambarus@microchip.com>:
>>
>> Hi, all,
>>
>> +zhengxunli, juliensu & ycllin
>>
>> On 2/16/21 11:48 AM, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Am 2021-02-16 10:27, schrieb Pratyush Yadav:
>>>> On 15/02/21 10:53PM, Heiko Thiery wrote:
>>>>> Hi all,
>>>>>
>>>>> I faced an issue with a SPI flash on our board. We use a macronix
>>>>> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
>>>>> the MX25L12805D [2].
>>>>>
>>>>> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
>>>>> while the older doesn't.
>>>>>
>>>>> I thought that I could do a fixup with a device specific
>>>>> post_bfpt_fixups() call but by now this seems not possible. The older
>>>>> MX25L12805D has no flags set that allows a call to
>>>>> spi_nor_sfdp_init_params() and implements the fixup.
>>>>>
>>>>> Has anyone an idea how to solve this?
>>
>> Maybe macronix can help with some suggestions on how to differentiate
>> between flashes at runtime.
>>
>> My first thought is to introduce a SPI_NOR_HAS_SFDP flag. For the flash
>> that doesn't support SFDP tables, there should be no functional change,
>> for the one that support SFDP it should fill the properties from the
>> SFDP tables.
>>
>>>>
>>>> The post_sfdp fixup is always run regardless of whether the flash has
>>>> SFDP or not. You can try putting your flash-specific fixups there.
>>>
>>> Well the problem here is, that the SFDP setup is skipped though the
>>> flash would support SFDP. If the jedec id wasn't already in the table,
>>> there would be the flag SPI_NOR_QUAD_READ and the SFDP would be
>>> parsed. But because there is already the legacy device (which likely
>>> doesn't support SFDP) it really doesn't fit.
>>>
>>> Its unclear to me, why the SFDP is only parsed if one of the
>>> SPI_NOR_*_READ flags are set.
>>
>> My guess is that a new SFDP flag was not necessary. SFDP defines multiple
>> tables, but there is just one that is mandatory, BFPT. BFPT defines DUAL
>> and QUAD parameters. From the spi-nor code, a BFPT without DUAL or QUAD
>> support doesn't make sense, even though DUAL or QUAD are not mandatory
>> in BFPT as I see in the standard. So probably it was just a way to avoid
>> adding a extra flag. We have to check the git history for a more accurate
>> description, this was just a guess.
>>
>> Thinking loud, now we do a static initialization of flash params, that
>> can be overwritten dynamically by SFDP. How about doing the params init
>> the other way around. Try first to dynamically discover the params via
>> SFDP, and if SFDP fails or if it is not defined, do the static init via
>> flags. That would spare some code. And new flash IDs will have less flags
>> declared, and we'll better track faulty SFDP flashes.
> 
> I am a newbie but it sounds reasonable. I made a first attempt and

Let's first see if all parties find the idea good (I'll have to double check
it myself). Vignesh and others might help.

Until then can you try the patch form below and see if you can do the
reads in quad mode?

Cheers,
ta

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 0522304f52fa..718d0b75df91 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
        spi_nor_manufacturer_init_params(nor);
 
        if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
+                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
+                                SPI_NOR_AIM_SFDP)) &&
            !(nor->info->flags & SPI_NOR_SKIP_SFDP))
                spi_nor_sfdp_init_params(nor);
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 4a3f7f150b5d..3495549815e6 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -338,6 +338,11 @@ struct flash_info {
                                         * protection bits. Usually these will
                                         * power-up in a write-protected state.
                                         */
+#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
+                                        * that share the same JEDEC-ID, but
+                                        * where a flash defines the SFDP tables
+                                        * and the other doesn't.
+                                        */
 
        /* Part specific fixup hooks. */
        const struct spi_nor_fixups *fixups;
diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
index 9203abaac229..1ebce775eae4 100644
--- a/drivers/mtd/spi-nor/macronix.c
+++ b/drivers/mtd/spi-nor/macronix.c
@@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
        { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
        { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
        { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
-       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
+       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
+                             SECT_4K | SPI_NOR_AIM_SFDP) },
        { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
        { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
                              SECT_4K | SPI_NOR_DUAL_READ |


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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi Tudor,

[...]

> >> Thinking loud, now we do a static initialization of flash params, that
> >> can be overwritten dynamically by SFDP. How about doing the params init
> >> the other way around. Try first to dynamically discover the params via
> >> SFDP, and if SFDP fails or if it is not defined, do the static init via
> >> flags. That would spare some code. And new flash IDs will have less flags
> >> declared, and we'll better track faulty SFDP flashes.
> >
> > I am a newbie but it sounds reasonable. I made a first attempt and
>
> Let's first see if all parties find the idea good (I'll have to double check
> it myself). Vignesh and others might help.
>
> Until then can you try the patch form below and see if you can do the
> reads in quad mode?
>
> Cheers,
> ta
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 0522304f52fa..718d0b75df91 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
>         spi_nor_manufacturer_init_params(nor);
>
>         if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> -                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
> +                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
> +                                SPI_NOR_AIM_SFDP)) &&
>             !(nor->info->flags & SPI_NOR_SKIP_SFDP))
>                 spi_nor_sfdp_init_params(nor);
>
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 4a3f7f150b5d..3495549815e6 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -338,6 +338,11 @@ struct flash_info {
>                                          * protection bits. Usually these will
>                                          * power-up in a write-protected state.
>                                          */
> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
> +                                        * that share the same JEDEC-ID, but
> +                                        * where a flash defines the SFDP tables
> +                                        * and the other doesn't.
> +                                        */
>
>         /* Part specific fixup hooks. */
>         const struct spi_nor_fixups *fixups;
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 9203abaac229..1ebce775eae4 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
>                               SECT_4K | SPI_NOR_DUAL_READ |

I tried your patch and it works like expected. I can now read the
whole flash in ~2sec while without that it was ~6sec.

# time dd if=/dev/mtd0 of=dump.bin
32768+0 records in
32768+0 records out
real 0m 2.08s
user 0m 0.01s
sys 0m 2.06s

vs.

# time dd if=/dev/mtd0 of=dump.bin
32768+0 records in
32768+0 records out
real 0m 6.16s
user 0m 0.05s
sys 0m 6.09s


Should I prepare a patch with that change or will you do?

Thank you

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

On 2/18/21 12:15 PM, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Tudor,
> 
> [...]
> 
>>>> Thinking loud, now we do a static initialization of flash params, that
>>>> can be overwritten dynamically by SFDP. How about doing the params init
>>>> the other way around. Try first to dynamically discover the params via
>>>> SFDP, and if SFDP fails or if it is not defined, do the static init via
>>>> flags. That would spare some code. And new flash IDs will have less flags
>>>> declared, and we'll better track faulty SFDP flashes.
>>>
>>> I am a newbie but it sounds reasonable. I made a first attempt and
>>
>> Let's first see if all parties find the idea good (I'll have to double check
>> it myself). Vignesh and others might help.
>>
>> Until then can you try the patch form below and see if you can do the
>> reads in quad mode?
>>
>> Cheers,
>> ta
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 0522304f52fa..718d0b75df91 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
>>         spi_nor_manufacturer_init_params(nor);
>>
>>         if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>> -                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
>> +                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
>> +                                SPI_NOR_AIM_SFDP)) &&
>>             !(nor->info->flags & SPI_NOR_SKIP_SFDP))
>>                 spi_nor_sfdp_init_params(nor);
>>
>> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
>> index 4a3f7f150b5d..3495549815e6 100644
>> --- a/drivers/mtd/spi-nor/core.h
>> +++ b/drivers/mtd/spi-nor/core.h
>> @@ -338,6 +338,11 @@ struct flash_info {
>>                                          * protection bits. Usually these will
>>                                          * power-up in a write-protected state.
>>                                          */
>> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
>> +                                        * that share the same JEDEC-ID, but
>> +                                        * where a flash defines the SFDP tables
>> +                                        * and the other doesn't.
>> +                                        */
>>
>>         /* Part specific fixup hooks. */
>>         const struct spi_nor_fixups *fixups;
>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
>> index 9203abaac229..1ebce775eae4 100644
>> --- a/drivers/mtd/spi-nor/macronix.c
>> +++ b/drivers/mtd/spi-nor/macronix.c
>> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
>>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
>>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
>>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
>> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
>> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
>> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
>>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
>>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
>>                               SECT_4K | SPI_NOR_DUAL_READ |
> 
> I tried your patch and it works like expected. I can now read the
> whole flash in ~2sec while without that it was ~6sec.
> 
> # time dd if=/dev/mtd0 of=dump.bin
> 32768+0 records in
> 32768+0 records out
> real 0m 2.08s
> user 0m 0.01s
> sys 0m 2.06s
> 
> vs.
> 
> # time dd if=/dev/mtd0 of=dump.bin
> 32768+0 records in
> 32768+0 records out
> real 0m 6.16s
> user 0m 0.05s
> sys 0m 6.09s
> 
> 

Great, thanks!

> Should I prepare a patch with that change or will you do?

Let's wait for a few days, so others can intervene. I'd like to
clarify what's happening on mx66l51235l too.

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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Am Do., 18. Feb. 2021 um 11:26 Uhr schrieb <Tudor.Ambarus@microchip.com>:
>
> On 2/18/21 12:15 PM, Heiko Thiery wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi Tudor,
> >
> > [...]
> >
> >>>> Thinking loud, now we do a static initialization of flash params, that
> >>>> can be overwritten dynamically by SFDP. How about doing the params init
> >>>> the other way around. Try first to dynamically discover the params via
> >>>> SFDP, and if SFDP fails or if it is not defined, do the static init via
> >>>> flags. That would spare some code. And new flash IDs will have less flags
> >>>> declared, and we'll better track faulty SFDP flashes.
> >>>
> >>> I am a newbie but it sounds reasonable. I made a first attempt and
> >>
> >> Let's first see if all parties find the idea good (I'll have to double check
> >> it myself). Vignesh and others might help.
> >>
> >> Until then can you try the patch form below and see if you can do the
> >> reads in quad mode?
> >>
> >> Cheers,
> >> ta
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >> index 0522304f52fa..718d0b75df91 100644
> >> --- a/drivers/mtd/spi-nor/core.c
> >> +++ b/drivers/mtd/spi-nor/core.c
> >> @@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
> >>         spi_nor_manufacturer_init_params(nor);
> >>
> >>         if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> >> -                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
> >> +                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
> >> +                                SPI_NOR_AIM_SFDP)) &&
> >>             !(nor->info->flags & SPI_NOR_SKIP_SFDP))
> >>                 spi_nor_sfdp_init_params(nor);
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> >> index 4a3f7f150b5d..3495549815e6 100644
> >> --- a/drivers/mtd/spi-nor/core.h
> >> +++ b/drivers/mtd/spi-nor/core.h
> >> @@ -338,6 +338,11 @@ struct flash_info {
> >>                                          * protection bits. Usually these will
> >>                                          * power-up in a write-protected state.
> >>                                          */
> >> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
> >> +                                        * that share the same JEDEC-ID, but
> >> +                                        * where a flash defines the SFDP tables
> >> +                                        * and the other doesn't.
> >> +                                        */
> >>
> >>         /* Part specific fixup hooks. */
> >>         const struct spi_nor_fixups *fixups;
> >> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> >> index 9203abaac229..1ebce775eae4 100644
> >> --- a/drivers/mtd/spi-nor/macronix.c
> >> +++ b/drivers/mtd/spi-nor/macronix.c
> >> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
> >>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
> >>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
> >>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
> >> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
> >> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
> >> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
> >>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
> >>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
> >>                               SECT_4K | SPI_NOR_DUAL_READ |
> >
> > I tried your patch and it works like expected. I can now read the
> > whole flash in ~2sec while without that it was ~6sec.
> >
> > # time dd if=/dev/mtd0 of=dump.bin
> > 32768+0 records in
> > 32768+0 records out
> > real 0m 2.08s
> > user 0m 0.01s
> > sys 0m 2.06s
> >
> > vs.
> >
> > # time dd if=/dev/mtd0 of=dump.bin
> > 32768+0 records in
> > 32768+0 records out
> > real 0m 6.16s
> > user 0m 0.05s
> > sys 0m 6.09s
> >
> >
>
> Great, thanks!
>
> > Should I prepare a patch with that change or will you do?
>
> Let's wait for a few days, so others can intervene. I'd like to
> clarify what's happening on mx66l51235l too.

Sounds good. Thanks.

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[zhengxunli]

Hi,

> 
> Subject
> 
> Re: spi-nor: maxronix MX25L12835F support
> 
> On 2/18/21 12:15 PM, Heiko Thiery wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you 
> know the content is safe
> > 
> > Hi Tudor,
> > 
> > [...]
> > 
> >>>> Thinking loud, now we do a static initialization of flash params, 
that
> >>>> can be overwritten dynamically by SFDP. How about doing the params 
init
> >>>> the other way around. Try first to dynamically discover the params 
via
> >>>> SFDP, and if SFDP fails or if it is not defined, do the static init 
via
> >>>> flags. That would spare some code. And new flash IDs will have less 
flags
> >>>> declared, and we'll better track faulty SFDP flashes.
> >>>
> >>> I am a newbie but it sounds reasonable. I made a first attempt and
> >>
> >> Let's first see if all parties find the idea good (I'll have to 
> double check
> >> it myself). Vignesh and others might help.
> >>
> >> Until then can you try the patch form below and see if you can do the
> >> reads in quad mode?
> >>
> >> Cheers,
> >> ta
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >> index 0522304f52fa..718d0b75df91 100644
> >> --- a/drivers/mtd/spi-nor/core.c
> >> +++ b/drivers/mtd/spi-nor/core.c
> >> @@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor 
*nor)
> >>         spi_nor_manufacturer_init_params(nor);
> >>
> >>         if ((nor->info->flags & (SPI_NOR_DUAL_READ | 
SPI_NOR_QUAD_READ |
> >> -                                SPI_NOR_OCTAL_READ | 
> SPI_NOR_OCTAL_DTR_READ)) &&
> >> +                                SPI_NOR_OCTAL_READ | 
> SPI_NOR_OCTAL_DTR_READ |
> >> +                                SPI_NOR_AIM_SFDP)) &&
> >>             !(nor->info->flags & SPI_NOR_SKIP_SFDP))
> >>                 spi_nor_sfdp_init_params(nor);
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> >> index 4a3f7f150b5d..3495549815e6 100644
> >> --- a/drivers/mtd/spi-nor/core.h
> >> +++ b/drivers/mtd/spi-nor/core.h
> >> @@ -338,6 +338,11 @@ struct flash_info {
> >>                                          * protection bits. 
> Usually these will
> >>                                          * power-up in a write-
> protected state.
> >>                                          */
> >> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. 
> Used by flashes
> >> +                                        * that share the same 
> JEDEC-ID, but
> >> +                                        * where a flash defines 
> the SFDP tables
> >> +                                        * and the other doesn't.
> >> +                                        */
> >>
> >>         /* Part specific fixup hooks. */
> >>         const struct spi_nor_fixups *fixups;
> >> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-
> nor/macronix.c
> >> index 9203abaac229..1ebce775eae4 100644
> >> --- a/drivers/mtd/spi-nor/macronix.c
> >> +++ b/drivers/mtd/spi-nor/macronix.c
> >> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
> >>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) 
},
> >>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) 
},
> >>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) 
},
> >> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) 
},
> >> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
> >> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
> >>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
> >>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
> >>                               SECT_4K | SPI_NOR_DUAL_READ |
> > 
> > I tried your patch and it works like expected. I can now read the
> > whole flash in ~2sec while without that it was ~6sec.
> > 
> > # time dd if=/dev/mtd0 of=dump.bin
> > 32768+0 records in
> > 32768+0 records out
> > real 0m 2.08s
> > user 0m 0.01s
> > sys 0m 2.06s
> > 
> > vs.
> > 
> > # time dd if=/dev/mtd0 of=dump.bin
> > 32768+0 records in
> > 32768+0 records out
> > real 0m 6.16s
> > user 0m 0.05s
> > sys 0m 6.09s
> > 
> > 
> 
> Great, thanks!
> 
> > Should I prepare a patch with that change or will you do?
> 
> Let's wait for a few days, so others can intervene. I'd like to
> clarify what's happening on mx66l51235l too.

I asked our former colleague and he said that the product
mx66u51235f was discontinued for some reason. And mx25u51245g
uses a new design to replace it.

Thanks,
Zhengxun

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi Tudor,

Am Do., 18. Feb. 2021 um 11:26 Uhr schrieb <Tudor.Ambarus@microchip.com>:
>
> On 2/18/21 12:15 PM, Heiko Thiery wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi Tudor,
> >
> > [...]
> >
> >>>> Thinking loud, now we do a static initialization of flash params, that
> >>>> can be overwritten dynamically by SFDP. How about doing the params init
> >>>> the other way around. Try first to dynamically discover the params via
> >>>> SFDP, and if SFDP fails or if it is not defined, do the static init via
> >>>> flags. That would spare some code. And new flash IDs will have less flags
> >>>> declared, and we'll better track faulty SFDP flashes.
> >>>
> >>> I am a newbie but it sounds reasonable. I made a first attempt and
> >>
> >> Let's first see if all parties find the idea good (I'll have to double check
> >> it myself). Vignesh and others might help.
> >>
> >> Until then can you try the patch form below and see if you can do the
> >> reads in quad mode?
> >>
> >> Cheers,
> >> ta
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> >> index 0522304f52fa..718d0b75df91 100644
> >> --- a/drivers/mtd/spi-nor/core.c
> >> +++ b/drivers/mtd/spi-nor/core.c
> >> @@ -3099,7 +3099,8 @@ static int spi_nor_init_params(struct spi_nor *nor)
> >>         spi_nor_manufacturer_init_params(nor);
> >>
> >>         if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> >> -                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ)) &&
> >> +                                SPI_NOR_OCTAL_READ | SPI_NOR_OCTAL_DTR_READ |
> >> +                                SPI_NOR_AIM_SFDP)) &&
> >>             !(nor->info->flags & SPI_NOR_SKIP_SFDP))
> >>                 spi_nor_sfdp_init_params(nor);
> >>
> >> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> >> index 4a3f7f150b5d..3495549815e6 100644
> >> --- a/drivers/mtd/spi-nor/core.h
> >> +++ b/drivers/mtd/spi-nor/core.h
> >> @@ -338,6 +338,11 @@ struct flash_info {
> >>                                          * protection bits. Usually these will
> >>                                          * power-up in a write-protected state.
> >>                                          */
> >> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
> >> +                                        * that share the same JEDEC-ID, but
> >> +                                        * where a flash defines the SFDP tables
> >> +                                        * and the other doesn't.
> >> +                                        */
> >>
> >>         /* Part specific fixup hooks. */
> >>         const struct spi_nor_fixups *fixups;
> >> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> >> index 9203abaac229..1ebce775eae4 100644
> >> --- a/drivers/mtd/spi-nor/macronix.c
> >> +++ b/drivers/mtd/spi-nor/macronix.c
> >> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
> >>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
> >>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
> >>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
> >> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
> >> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
> >> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
> >>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
> >>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
> >>                               SECT_4K | SPI_NOR_DUAL_READ |
> >
> > I tried your patch and it works like expected. I can now read the
> > whole flash in ~2sec while without that it was ~6sec.
> >
> > # time dd if=/dev/mtd0 of=dump.bin
> > 32768+0 records in
> > 32768+0 records out
> > real 0m 2.08s
> > user 0m 0.01s
> > sys 0m 2.06s
> >
> > vs.
> >
> > # time dd if=/dev/mtd0 of=dump.bin
> > 32768+0 records in
> > 32768+0 records out
> > real 0m 6.16s
> > user 0m 0.05s
> > sys 0m 6.09s
> >
> >
>
> Great, thanks!
>
> > Should I prepare a patch with that change or will you do?
>
> Let's wait for a few days, so others can intervene. I'd like to
> clarify what's happening on mx66l51235l too.

Since a few days have passed and no one has commented, I would like to
bring up the subject again.

I can send a patch for the changes you suggested. What do you think?

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Vignesh Raghavendra]

Hi,

On 2/28/21 3:22 AM, Heiko Thiery wrote:

[...]
>>>> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
>>>> +                                        * that share the same JEDEC-ID, but
>>>> +                                        * where a flash defines the SFDP tables
>>>> +                                        * and the other doesn't.
>>>> +                                        */
>>>>
>>>>         /* Part specific fixup hooks. */
>>>>         const struct spi_nor_fixups *fixups;
>>>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
>>>> index 9203abaac229..1ebce775eae4 100644
>>>> --- a/drivers/mtd/spi-nor/macronix.c
>>>> +++ b/drivers/mtd/spi-nor/macronix.c
>>>> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
>>>>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
>>>>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
>>>>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
>>>> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
>>>> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
>>>> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
>>>>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
>>>>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
>>>>                               SECT_4K | SPI_NOR_DUAL_READ |
>>>
>>> I tried your patch and it works like expected. I can now read the
>>> whole flash in ~2sec while without that it was ~6sec.
>>>
>>> # time dd if=/dev/mtd0 of=dump.bin
>>> 32768+0 records in
>>> 32768+0 records out
>>> real 0m 2.08s
>>> user 0m 0.01s
>>> sys 0m 2.06s
>>>
>>> vs.
>>>
>>> # time dd if=/dev/mtd0 of=dump.bin
>>> 32768+0 records in
>>> 32768+0 records out
>>> real 0m 6.16s
>>> user 0m 0.05s
>>> sys 0m 6.09s
>>>
>>>
>>
>> Great, thanks!
>>
>>> Should I prepare a patch with that change or will you do?
>>
>> Let's wait for a few days, so others can intervene. I'd like to
>> clarify what's happening on mx66l51235l too.
> 
> Since a few days have passed and no one has commented, I would like to
> bring up the subject again.
> 
> I can send a patch for the changes you suggested. What do you think?
> 

Why not have a single entry for mx66l51235l/mx25l12805d with superset
capabilities declared. And then use info->fixups->post_sfdp() to fixup
capabilities for mx66l51235l based on absence of SFDP tables?

SPI_NOR_AIM_SFDP seems redundant to me. SPI NOR Framework should anyway
be using SFDP for detecting flash capabilities and away from flash_info
based static data.


Regards
Vignesh


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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

On 3/1/21 12:52 PM, Vignesh Raghavendra wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi,
> 
> On 2/28/21 3:22 AM, Heiko Thiery wrote:
> 
> [...]
>>>>> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
>>>>> +                                        * that share the same JEDEC-ID, but
>>>>> +                                        * where a flash defines the SFDP tables
>>>>> +                                        * and the other doesn't.
>>>>> +                                        */
>>>>>
>>>>>         /* Part specific fixup hooks. */
>>>>>         const struct spi_nor_fixups *fixups;
>>>>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
>>>>> index 9203abaac229..1ebce775eae4 100644
>>>>> --- a/drivers/mtd/spi-nor/macronix.c
>>>>> +++ b/drivers/mtd/spi-nor/macronix.c
>>>>> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
>>>>>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
>>>>>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
>>>>>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
>>>>> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
>>>>> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
>>>>> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
>>>>>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
>>>>>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
>>>>>                               SECT_4K | SPI_NOR_DUAL_READ |
>>>>
>>>> I tried your patch and it works like expected. I can now read the
>>>> whole flash in ~2sec while without that it was ~6sec.
>>>>
>>>> # time dd if=/dev/mtd0 of=dump.bin
>>>> 32768+0 records in
>>>> 32768+0 records out
>>>> real 0m 2.08s
>>>> user 0m 0.01s
>>>> sys 0m 2.06s
>>>>
>>>> vs.
>>>>
>>>> # time dd if=/dev/mtd0 of=dump.bin
>>>> 32768+0 records in
>>>> 32768+0 records out
>>>> real 0m 6.16s
>>>> user 0m 0.05s
>>>> sys 0m 6.09s
>>>>
>>>>
>>>
>>> Great, thanks!
>>>
>>>> Should I prepare a patch with that change or will you do?
>>>
>>> Let's wait for a few days, so others can intervene. I'd like to
>>> clarify what's happening on mx66l51235l too.
>>
>> Since a few days have passed and no one has commented, I would like to
>> bring up the subject again.
>>
>> I can send a patch for the changes you suggested. What do you think?
>>
> 
> Why not have a single entry for mx66l51235l/mx25l12805d with superset
> capabilities declared. And then use info->fixups->post_sfdp() to fixup
> capabilities for mx66l51235l based on absence of SFDP tables?

do you mean to add SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ flags in order to
trigger the SFDP parsing and then to undo the read init that is done in
spi_nor_info_init_params()? Too much hustle.

> 
> SPI_NOR_AIM_SFDP seems redundant to me. SPI NOR Framework should anyway
> be using SFDP for detecting flash capabilities and away from flash_info
> based static data.

Yeah, that's what I suggested a bit earlier in the thread. To first try to
detect the caps by parsing SFDP and then if SFDP not supported then to do
the static init via the flash flags. I'll have to check the implications,
it will impact every flash.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: spi-nor: maxronix MX25L12835F support

[Pratyush Yadav]

On 01/03/21 11:11AM, Tudor.Ambarus@microchip.com wrote:
> On 3/1/21 12:52 PM, Vignesh Raghavendra wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > Hi,
> > 
> > On 2/28/21 3:22 AM, Heiko Thiery wrote:
> > 
> > [...]
> >>>>> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
> >>>>> +                                        * that share the same JEDEC-ID, but
> >>>>> +                                        * where a flash defines the SFDP tables
> >>>>> +                                        * and the other doesn't.
> >>>>> +                                        */
> >>>>>
> >>>>>         /* Part specific fixup hooks. */
> >>>>>         const struct spi_nor_fixups *fixups;
> >>>>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> >>>>> index 9203abaac229..1ebce775eae4 100644
> >>>>> --- a/drivers/mtd/spi-nor/macronix.c
> >>>>> +++ b/drivers/mtd/spi-nor/macronix.c
> >>>>> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
> >>>>>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
> >>>>>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
> >>>>>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
> >>>>> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
> >>>>> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
> >>>>> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
> >>>>>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
> >>>>>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
> >>>>>                               SECT_4K | SPI_NOR_DUAL_READ |
> >>>>
> >>>> I tried your patch and it works like expected. I can now read the
> >>>> whole flash in ~2sec while without that it was ~6sec.
> >>>>
> >>>> # time dd if=/dev/mtd0 of=dump.bin
> >>>> 32768+0 records in
> >>>> 32768+0 records out
> >>>> real 0m 2.08s
> >>>> user 0m 0.01s
> >>>> sys 0m 2.06s
> >>>>
> >>>> vs.
> >>>>
> >>>> # time dd if=/dev/mtd0 of=dump.bin
> >>>> 32768+0 records in
> >>>> 32768+0 records out
> >>>> real 0m 6.16s
> >>>> user 0m 0.05s
> >>>> sys 0m 6.09s
> >>>>
> >>>>
> >>>
> >>> Great, thanks!
> >>>
> >>>> Should I prepare a patch with that change or will you do?
> >>>
> >>> Let's wait for a few days, so others can intervene. I'd like to
> >>> clarify what's happening on mx66l51235l too.
> >>
> >> Since a few days have passed and no one has commented, I would like to
> >> bring up the subject again.
> >>
> >> I can send a patch for the changes you suggested. What do you think?
> >>
> > 
> > Why not have a single entry for mx66l51235l/mx25l12805d with superset
> > capabilities declared. And then use info->fixups->post_sfdp() to fixup
> > capabilities for mx66l51235l based on absence of SFDP tables?

I think printing the correct flash name is somewhat important. Other 
than the handful of people who are reading this thread, few would know 
that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can 
cause a lot of confusion among people trying to debug any issues.

So my vote goes for having separate entries for both the flashes and 
then adding a fixup hook to select the correct one by checking if SFDP 
read works.

> 
> do you mean to add SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ flags in order to
> trigger the SFDP parsing and then to undo the read init that is done in
> spi_nor_info_init_params()? Too much hustle.
> 
> > 
> > SPI_NOR_AIM_SFDP seems redundant to me. SPI NOR Framework should anyway
> > be using SFDP for detecting flash capabilities and away from flash_info
> > based static data.
> 
> Yeah, that's what I suggested a bit earlier in the thread. To first try to
> detect the caps by parsing SFDP and then if SFDP not supported then to do
> the static init via the flash flags. I'll have to check the implications,
> it will impact every flash.

I don't think it is a proper fix for this situation because of the 
reasons mentioned above.

I do think it is a good change in general. It makes sense to ask the 
device what it supports before falling back to the hard-coded values. My 
biggest concern is all the things that SFDP _can't_ tell us. We need to 
look at all the flash_info flags and see which ones can be detected from 
SFDP. If there are too many flashes using flags that can't be detected 
via SFDP then all those will have to be activated via fixup hooks. This 
can easily turn into a big maintenance burden.

A quick look at spi_nor_info_init_params() doesn't reveal anything too 
problematic though. There is SPI_NOR_NO_FR but IIUC it is for legacy 
flashes so they won't have SFDP anyway.

There are also SPI_NOR_OCTAL_READ and SPI_NOR_OCTAL_DTR_READ. The former 
can be detected via BFPT dword 17 but the BFPT parser does not support 
it yet. The latter is a bit more complicated. BFPT does not advertise 
it. The presence of the Profile 1.0 table signals this command is 
supported. Not every 8D-8D-8D capable flash needs to have this table but 
then a fixup would be needed anyway to specify the opcode, dummy, etc. 
In short, it should not be too hard to add support for detecting these 
flags.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: spi-nor: maxronix MX25L12835F support

[Michael Walle]

Am 2021-03-01 14:36, schrieb Pratyush Yadav:
> I think printing the correct flash name is somewhat important. Other
> than the handful of people who are reading this thread, few would know
> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
> cause a lot of confusion among people trying to debug any issues.

Unfortunately, this is kind of a mess. If multiple flash devices
share the same id, it seems to be first come first serve. The kernel
will print the name which was introduced first.

This isn't the only flash which is affected. Have a look at
   drivers/mtd/spi-nor/winbond.c
There are all kind of flash names, some of them are not even existing
as this particular string, eg. take w25q64jwm, its actually "Winbond
W25Q64JW-IM or W25Q64JW-JM".

So yes, it would be nice to have such a thing, but for now, I will
take the kernel output as a rough estimation what might really be
used on the board.

-michael

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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

On 3/1/21 3:36 PM, Pratyush Yadav wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 01/03/21 11:11AM, Tudor.Ambarus@microchip.com wrote:
>> On 3/1/21 12:52 PM, Vignesh Raghavendra wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi,
>>>
>>> On 2/28/21 3:22 AM, Heiko Thiery wrote:
>>>
>>> [...]
>>>>>>> +#define SPI_NOR_AIM_SFDP       BIT(23) /* Try to parse SFDP. Used by flashes
>>>>>>> +                                        * that share the same JEDEC-ID, but
>>>>>>> +                                        * where a flash defines the SFDP tables
>>>>>>> +                                        * and the other doesn't.
>>>>>>> +                                        */
>>>>>>>
>>>>>>>         /* Part specific fixup hooks. */
>>>>>>>         const struct spi_nor_fixups *fixups;
>>>>>>> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
>>>>>>> index 9203abaac229..1ebce775eae4 100644
>>>>>>> --- a/drivers/mtd/spi-nor/macronix.c
>>>>>>> +++ b/drivers/mtd/spi-nor/macronix.c
>>>>>>> @@ -50,7 +50,8 @@ static const struct flash_info macronix_parts[] = {
>>>>>>>         { "mx25u4035",   INFO(0xc22533, 0, 64 * 1024,   8, SECT_4K) },
>>>>>>>         { "mx25u8035",   INFO(0xc22534, 0, 64 * 1024,  16, SECT_4K) },
>>>>>>>         { "mx25u6435f",  INFO(0xc22537, 0, 64 * 1024, 128, SECT_4K) },
>>>>>>> -       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256, SECT_4K) },
>>>>>>> +       { "mx25l12805d", INFO(0xc22018, 0, 64 * 1024, 256,
>>>>>>> +                             SECT_4K | SPI_NOR_AIM_SFDP) },
>>>>>>>         { "mx25l12855e", INFO(0xc22618, 0, 64 * 1024, 256, 0) },
>>>>>>>         { "mx25r1635f",  INFO(0xc22815, 0, 64 * 1024,  32,
>>>>>>>                               SECT_4K | SPI_NOR_DUAL_READ |
>>>>>>
>>>>>> I tried your patch and it works like expected. I can now read the
>>>>>> whole flash in ~2sec while without that it was ~6sec.
>>>>>>
>>>>>> # time dd if=/dev/mtd0 of=dump.bin
>>>>>> 32768+0 records in
>>>>>> 32768+0 records out
>>>>>> real 0m 2.08s
>>>>>> user 0m 0.01s
>>>>>> sys 0m 2.06s
>>>>>>
>>>>>> vs.
>>>>>>
>>>>>> # time dd if=/dev/mtd0 of=dump.bin
>>>>>> 32768+0 records in
>>>>>> 32768+0 records out
>>>>>> real 0m 6.16s
>>>>>> user 0m 0.05s
>>>>>> sys 0m 6.09s
>>>>>>
>>>>>>
>>>>>
>>>>> Great, thanks!
>>>>>
>>>>>> Should I prepare a patch with that change or will you do?
>>>>>
>>>>> Let's wait for a few days, so others can intervene. I'd like to
>>>>> clarify what's happening on mx66l51235l too.
>>>>
>>>> Since a few days have passed and no one has commented, I would like to
>>>> bring up the subject again.
>>>>
>>>> I can send a patch for the changes you suggested. What do you think?
>>>>
>>>
>>> Why not have a single entry for mx66l51235l/mx25l12805d with superset
>>> capabilities declared. And then use info->fixups->post_sfdp() to fixup
>>> capabilities for mx66l51235l based on absence of SFDP tables?
> 
> I think printing the correct flash name is somewhat important. Other
> than the handful of people who are reading this thread, few would know
> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
> cause a lot of confusion among people trying to debug any issues.
> 
> So my vote goes for having separate entries for both the flashes and
> then adding a fixup hook to select the correct one by checking if SFDP
> read works.
> 

You can't have separate flash entries with the same JEDEC ID, because
you will always hit the first one that it is defined in the array.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

On 3/1/21 3:50 PM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2021-03-01 14:36, schrieb Pratyush Yadav:
>> I think printing the correct flash name is somewhat important. Other
>> than the handful of people who are reading this thread, few would know
>> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
>> cause a lot of confusion among people trying to debug any issues.
> 
> Unfortunately, this is kind of a mess. If multiple flash devices
> share the same id, it seems to be first come first serve. The kernel
> will print the name which was introduced first.
> 
> This isn't the only flash which is affected. Have a look at
>   drivers/mtd/spi-nor/winbond.c
> There are all kind of flash names, some of them are not even existing
> as this particular string, eg. take w25q64jwm, its actually "Winbond
> W25Q64JW-IM or W25Q64JW-JM".
> 
> So yes, it would be nice to have such a thing, but for now, I will
> take the kernel output as a rough estimation what might really be
> used on the board.
> 

How about naming them something like "updated-flash-name || first-name".
Anyway, these are just workarounds. Manufacturers shouldn't use the same
JEDEC ID for new flashes. They should at least add an extended ID.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: spi-nor: maxronix MX25L12835F support

[Michael Walle]

Am 2021-03-01 15:09, schrieb Tudor.Ambarus@microchip.com:
> On 3/1/21 3:50 PM, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know 
>> the content is safe
>> 
>> Am 2021-03-01 14:36, schrieb Pratyush Yadav:
>>> I think printing the correct flash name is somewhat important. Other
>>> than the handful of people who are reading this thread, few would 
>>> know
>>> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
>>> cause a lot of confusion among people trying to debug any issues.
>> 
>> Unfortunately, this is kind of a mess. If multiple flash devices
>> share the same id, it seems to be first come first serve. The kernel
>> will print the name which was introduced first.
>> 
>> This isn't the only flash which is affected. Have a look at
>>   drivers/mtd/spi-nor/winbond.c
>> There are all kind of flash names, some of them are not even existing
>> as this particular string, eg. take w25q64jwm, its actually "Winbond
>> W25Q64JW-IM or W25Q64JW-JM".
>> 
>> So yes, it would be nice to have such a thing, but for now, I will
>> take the kernel output as a rough estimation what might really be
>> used on the board.
>> 
> 
> How about naming them something like "updated-flash-name || 
> first-name".
> Anyway, these are just workarounds. Manufacturers shouldn't use the 
> same
> JEDEC ID for new flashes. They should at least add an extended ID.

Mh, what about a list of names? I mean yes it is a workaround, but
there is actual hardware doing this, so IMHO linux has to deal with
it in some way. OTOH that list might be long and doesn't look good
in dmesg (or wherever that string might be used).

It might come in handy to have a mechanism in place if someone
really cares about it though.

-michael

-- 
-michael

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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

On 3/1/21 4:42 PM, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Am 2021-03-01 15:09, schrieb Tudor.Ambarus@microchip.com:
>> On 3/1/21 3:50 PM, Michael Walle wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>> the content is safe
>>>
>>> Am 2021-03-01 14:36, schrieb Pratyush Yadav:
>>>> I think printing the correct flash name is somewhat important. Other
>>>> than the handful of people who are reading this thread, few would
>>>> know
>>>> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
>>>> cause a lot of confusion among people trying to debug any issues.
>>>
>>> Unfortunately, this is kind of a mess. If multiple flash devices
>>> share the same id, it seems to be first come first serve. The kernel
>>> will print the name which was introduced first.
>>>
>>> This isn't the only flash which is affected. Have a look at
>>>   drivers/mtd/spi-nor/winbond.c
>>> There are all kind of flash names, some of them are not even existing
>>> as this particular string, eg. take w25q64jwm, its actually "Winbond
>>> W25Q64JW-IM or W25Q64JW-JM".
>>>
>>> So yes, it would be nice to have such a thing, but for now, I will
>>> take the kernel output as a rough estimation what might really be
>>> used on the board.
>>>
>>
>> How about naming them something like "updated-flash-name ||
>> first-name".
>> Anyway, these are just workarounds. Manufacturers shouldn't use the
>> same
>> JEDEC ID for new flashes. They should at least add an extended ID.
> 
> Mh, what about a list of names? I mean yes it is a workaround, but
> there is actual hardware doing this, so IMHO linux has to deal with
> it in some way. OTOH that list might be long and doesn't look good
> in dmesg (or wherever that string might be used).
> 
> It might come in handy to have a mechanism in place if someone
> really cares about it though.
> 

A list of names with differentiation at run-time, where possible,
sounds good. Otherwise we'll stick to a default name, whatever that
will be. Do you care to scratch a patch for the list of names idea?

We'll still have a single flash entry, with a list of names, and we
still need to either do the SFDP detection first, or to trigger the
SFDP detection with an explicit flash info flag. I'll follow Pratyush's
steps and evaluate the "detect SFDP first" idea.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: spi-nor: maxronix MX25L12835F support

[Pratyush Yadav]

On 01/03/21 02:50PM, Michael Walle wrote:
> Am 2021-03-01 14:36, schrieb Pratyush Yadav:
> > I think printing the correct flash name is somewhat important. Other
> > than the handful of people who are reading this thread, few would know
> > that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
> > cause a lot of confusion among people trying to debug any issues.
> 
> Unfortunately, this is kind of a mess. If multiple flash devices
> share the same id, it seems to be first come first serve. The kernel
> will print the name which was introduced first.

Install the same default_init() hook on both flashes [0]. In that hook 
check if SFDP is supported or not. Set nor->info to the correct entry 
based on this hook. I admit this is a bit hacky but getting the flash 
name right is worth the trouble IMO.

[0] Currently the flash that comes first in the order will be selected 
but that might change later and doing this will help guard against that.

> 
> This isn't the only flash which is affected. Have a look at
>   drivers/mtd/spi-nor/winbond.c
> There are all kind of flash names, some of them are not even existing
> as this particular string, eg. take w25q64jwm, its actually "Winbond
> W25Q64JW-IM or W25Q64JW-JM".
> 
> So yes, it would be nice to have such a thing, but for now, I will
> take the kernel output as a rough estimation what might really be
> used on the board.
> 
> -michael

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

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

Re: spi-nor: maxronix MX25L12835F support

[Vignesh Raghavendra]

On 3/1/21 8:55 PM, Tudor.Ambarus@microchip.com wrote:
> On 3/1/21 4:42 PM, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Am 2021-03-01 15:09, schrieb Tudor.Ambarus@microchip.com:
>>> On 3/1/21 3:50 PM, Michael Walle wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>>>> the content is safe
>>>>
>>>> Am 2021-03-01 14:36, schrieb Pratyush Yadav:
>>>>> I think printing the correct flash name is somewhat important. Other
>>>>> than the handful of people who are reading this thread, few would
>>>>> know
>>>>> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
>>>>> cause a lot of confusion among people trying to debug any issues.
>>>>
>>>> Unfortunately, this is kind of a mess. If multiple flash devices
>>>> share the same id, it seems to be first come first serve. The kernel
>>>> will print the name which was introduced first.
>>>>
>>>> This isn't the only flash which is affected. Have a look at
>>>>   drivers/mtd/spi-nor/winbond.c
>>>> There are all kind of flash names, some of them are not even existing
>>>> as this particular string, eg. take w25q64jwm, its actually "Winbond
>>>> W25Q64JW-IM or W25Q64JW-JM".
>>>>
>>>> So yes, it would be nice to have such a thing, but for now, I will
>>>> take the kernel output as a rough estimation what might really be
>>>> used on the board.
>>>>
>>>
>>> How about naming them something like "updated-flash-name ||
>>> first-name".
>>> Anyway, these are just workarounds. Manufacturers shouldn't use the
>>> same
>>> JEDEC ID for new flashes. They should at least add an extended ID.
>>
>> Mh, what about a list of names? I mean yes it is a workaround, but
>> there is actual hardware doing this, so IMHO linux has to deal with
>> it in some way. OTOH that list might be long and doesn't look good
>> in dmesg (or wherever that string might be used).
>>
>> It might come in handy to have a mechanism in place if someone
>> really cares about it though.
>>
> 
> A list of names with differentiation at run-time, where possible,
> sounds good. Otherwise we'll stick to a default name, whatever that
> will be. Do you care to scratch a patch for the list of names idea?
> 
> We'll still have a single flash entry, with a list of names, and we
> still need to either do the SFDP detection first, or to trigger the
> SFDP detection with an explicit flash info flag. I'll follow Pratyush's
> steps and evaluate the "detect SFDP first" idea.
> 

If we do go down the road of "detect SFDP first", we should add
SPI_NOR_SKIP_SFDP to flashes that currently don't claim DUAL/QUAD/OCTAL
capability currently in order to avoid any surprises due to wrong values
in the table.

Regards
Vignesh



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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi Vignesh,

Am Di., 2. März 2021 um 06:49 Uhr schrieb Vignesh Raghavendra <vigneshr@ti.com>:
>
>
>
> On 3/1/21 8:55 PM, Tudor.Ambarus@microchip.com wrote:
> > On 3/1/21 4:42 PM, Michael Walle wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>
> >> Am 2021-03-01 15:09, schrieb Tudor.Ambarus@microchip.com:
> >>> On 3/1/21 3:50 PM, Michael Walle wrote:
> >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >>>> the content is safe
> >>>>
> >>>> Am 2021-03-01 14:36, schrieb Pratyush Yadav:
> >>>>> I think printing the correct flash name is somewhat important. Other
> >>>>> than the handful of people who are reading this thread, few would
> >>>>> know
> >>>>> that SPI NOR calls mx25l12835f as mx25l12805d or vice versa. This can
> >>>>> cause a lot of confusion among people trying to debug any issues.
> >>>>
> >>>> Unfortunately, this is kind of a mess. If multiple flash devices
> >>>> share the same id, it seems to be first come first serve. The kernel
> >>>> will print the name which was introduced first.
> >>>>
> >>>> This isn't the only flash which is affected. Have a look at
> >>>>   drivers/mtd/spi-nor/winbond.c
> >>>> There are all kind of flash names, some of them are not even existing
> >>>> as this particular string, eg. take w25q64jwm, its actually "Winbond
> >>>> W25Q64JW-IM or W25Q64JW-JM".
> >>>>
> >>>> So yes, it would be nice to have such a thing, but for now, I will
> >>>> take the kernel output as a rough estimation what might really be
> >>>> used on the board.
> >>>>
> >>>
> >>> How about naming them something like "updated-flash-name ||
> >>> first-name".
> >>> Anyway, these are just workarounds. Manufacturers shouldn't use the
> >>> same
> >>> JEDEC ID for new flashes. They should at least add an extended ID.
> >>
> >> Mh, what about a list of names? I mean yes it is a workaround, but
> >> there is actual hardware doing this, so IMHO linux has to deal with
> >> it in some way. OTOH that list might be long and doesn't look good
> >> in dmesg (or wherever that string might be used).
> >>
> >> It might come in handy to have a mechanism in place if someone
> >> really cares about it though.
> >>
> >
> > A list of names with differentiation at run-time, where possible,
> > sounds good. Otherwise we'll stick to a default name, whatever that
> > will be. Do you care to scratch a patch for the list of names idea?
> >
> > We'll still have a single flash entry, with a list of names, and we
> > still need to either do the SFDP detection first, or to trigger the
> > SFDP detection with an explicit flash info flag. I'll follow Pratyush's
> > steps and evaluate the "detect SFDP first" idea.
> >
>
> If we do go down the road of "detect SFDP first", we should add
> SPI_NOR_SKIP_SFDP to flashes that currently don't claim DUAL/QUAD/OCTAL
> capability currently in order to avoid any surprises due to wrong values
> in the table.

Does this mean that all entries that have DUAL/QUAD/OCTAL defined can
have them removed And the correct values will be detected/set by SFDP?

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Vignesh Raghavendra]

On 3/3/21 7:14 PM, Heiko Thiery wrote:
> Hi Vignesh,
> 
[...]
>>>>>
>>>>> How about naming them something like "updated-flash-name ||
>>>>> first-name".
>>>>> Anyway, these are just workarounds. Manufacturers shouldn't use the
>>>>> same
>>>>> JEDEC ID for new flashes. They should at least add an extended ID.
>>>>
>>>> Mh, what about a list of names? I mean yes it is a workaround, but
>>>> there is actual hardware doing this, so IMHO linux has to deal with
>>>> it in some way. OTOH that list might be long and doesn't look good
>>>> in dmesg (or wherever that string might be used).
>>>>
>>>> It might come in handy to have a mechanism in place if someone
>>>> really cares about it though.
>>>>
>>>
>>> A list of names with differentiation at run-time, where possible,
>>> sounds good. Otherwise we'll stick to a default name, whatever that
>>> will be. Do you care to scratch a patch for the list of names idea?
>>>
>>> We'll still have a single flash entry, with a list of names, and we
>>> still need to either do the SFDP detection first, or to trigger the
>>> SFDP detection with an explicit flash info flag. I'll follow Pratyush's
>>> steps and evaluate the "detect SFDP first" idea.
>>>
>>
>> If we do go down the road of "detect SFDP first", we should add
>> SPI_NOR_SKIP_SFDP to flashes that currently don't claim DUAL/QUAD/OCTAL
>> capability currently in order to avoid any surprises due to wrong values
>> in the table.
> 
> Does this mean that all entries that have DUAL/QUAD/OCTAL defined can
> have them removed And the correct values will be detected/set by SFDP?
> 

No, not all Dual/Quad/Octal SPI flashes have SFDP tables populated.
Removing DUAL/QUAD/OCTAL capabilities for a flash that does not have
SFDP tables populated (or has wrong values) will cause regression as
code may fallback to legacy SPI mode.


Regards
Vignesh


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

Re: spi-nor: maxronix MX25L12835F support

[Heiko Thiery]

Hi Vignesh,

Am Do., 4. März 2021 um 08:02 Uhr schrieb Vignesh Raghavendra <vigneshr@ti.com>:
>
>
>
> On 3/3/21 7:14 PM, Heiko Thiery wrote:
> > Hi Vignesh,
> >
> [...]
> >>>>>
> >>>>> How about naming them something like "updated-flash-name ||
> >>>>> first-name".
> >>>>> Anyway, these are just workarounds. Manufacturers shouldn't use the
> >>>>> same
> >>>>> JEDEC ID for new flashes. They should at least add an extended ID.
> >>>>
> >>>> Mh, what about a list of names? I mean yes it is a workaround, but
> >>>> there is actual hardware doing this, so IMHO linux has to deal with
> >>>> it in some way. OTOH that list might be long and doesn't look good
> >>>> in dmesg (or wherever that string might be used).
> >>>>
> >>>> It might come in handy to have a mechanism in place if someone
> >>>> really cares about it though.
> >>>>
> >>>
> >>> A list of names with differentiation at run-time, where possible,
> >>> sounds good. Otherwise we'll stick to a default name, whatever that
> >>> will be. Do you care to scratch a patch for the list of names idea?
> >>>
> >>> We'll still have a single flash entry, with a list of names, and we
> >>> still need to either do the SFDP detection first, or to trigger the
> >>> SFDP detection with an explicit flash info flag. I'll follow Pratyush's
> >>> steps and evaluate the "detect SFDP first" idea.
> >>>
> >>
> >> If we do go down the road of "detect SFDP first", we should add
> >> SPI_NOR_SKIP_SFDP to flashes that currently don't claim DUAL/QUAD/OCTAL
> >> capability currently in order to avoid any surprises due to wrong values
> >> in the table.
> >
> > Does this mean that all entries that have DUAL/QUAD/OCTAL defined can
> > have them removed And the correct values will be detected/set by SFDP?
> >
>
> No, not all Dual/Quad/Octal SPI flashes have SFDP tables populated.
> Removing DUAL/QUAD/OCTAL capabilities for a flash that does not have
> SFDP tables populated (or has wrong values) will cause regression as
> code may fallback to legacy SPI mode.

So in that case removing the flags can only be done on chips/flashes
that are checked and reviewed for a valid functional SFDP detection.

-- 
Heiko

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

Re: spi-nor: maxronix MX25L12835F support

[Stefan Herbrechtsmeier]

Hi,

Am 04.03.2021 um 08:10 schrieb Heiko Thiery:
> Hi Vignesh,
> 
> Am Do., 4. März 2021 um 08:02 Uhr schrieb Vignesh Raghavendra <vigneshr@ti.com>:
>>
>>
>>
>> On 3/3/21 7:14 PM, Heiko Thiery wrote:
>>> Hi Vignesh,
>>>
>> [...]
>>>>>>>
>>>>>>> How about naming them something like "updated-flash-name ||
>>>>>>> first-name".
>>>>>>> Anyway, these are just workarounds. Manufacturers shouldn't use the
>>>>>>> same
>>>>>>> JEDEC ID for new flashes. They should at least add an extended ID.
>>>>>>
>>>>>> Mh, what about a list of names? I mean yes it is a workaround, but
>>>>>> there is actual hardware doing this, so IMHO linux has to deal with
>>>>>> it in some way. OTOH that list might be long and doesn't look good
>>>>>> in dmesg (or wherever that string might be used).
>>>>>>
>>>>>> It might come in handy to have a mechanism in place if someone
>>>>>> really cares about it though.
>>>>>>
>>>>>
>>>>> A list of names with differentiation at run-time, where possible,
>>>>> sounds good. Otherwise we'll stick to a default name, whatever that
>>>>> will be. Do you care to scratch a patch for the list of names idea?
>>>>>
>>>>> We'll still have a single flash entry, with a list of names, and we
>>>>> still need to either do the SFDP detection first, or to trigger the
>>>>> SFDP detection with an explicit flash info flag. I'll follow Pratyush's
>>>>> steps and evaluate the "detect SFDP first" idea.
>>>>>
>>>>
>>>> If we do go down the road of "detect SFDP first", we should add
>>>> SPI_NOR_SKIP_SFDP to flashes that currently don't claim DUAL/QUAD/OCTAL
>>>> capability currently in order to avoid any surprises due to wrong values
>>>> in the table.
>>>
>>> Does this mean that all entries that have DUAL/QUAD/OCTAL defined can
>>> have them removed And the correct values will be detected/set by SFDP?
>>>
>>
>> No, not all Dual/Quad/Octal SPI flashes have SFDP tables populated.
>> Removing DUAL/QUAD/OCTAL capabilities for a flash that does not have
>> SFDP tables populated (or has wrong values) will cause regression as
>> code may fallback to legacy SPI mode.
> 
> So in that case removing the flags can only be done on chips/flashes
> that are checked and reviewed for a valid functional SFDP detection.
> 

Isn't the assumption wrong that legacy flashes doesn't support SFDP? At 
the moment the driver enables SFDP only if the flash supports 
DUAL/QUAD/OCTAL mode. We should enable it for all flashes or allow the 
legacy flashes to explicit enable it.

What is the meaning of SPI_NOR_SKIP_SFDP? Is this a leftover and should 
be replaced by post_sfdp fixup?

What is the advantage of SFDP first? Because of backward compatibility 
you have to check the flash info data and you need a flag for compatible 
or incompatible flashes.

Independently of the SFDP read position we could add the 
SPI_NOR_SKIP_SFDP flag to all legacy flashes and enable SFDP for all 
flashes. Or we could remove the SPI_NOR_SKIP_SFDP flag and add an 
SPI_NOR_SFDP flag to explicitly enable SFDP.

Kind regards
   Stefan

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

Re: spi-nor: maxronix MX25L12835F support

[Tudor.Ambarus]

On 2/15/21 11:53 PM, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi all,
> 
> I faced an issue with a SPI flash on our board. We use a macronix
> MX25L12835F [1]. Unfortunately this flash has the same JEDEC ID like
> the MX25L12805D [2].
> 
> The newer MX25L12835F has support for dual/quad read mode and RDSFDP
> while the older doesn't.
> 
> I thought that I could do a fixup with a device specific
> post_bfpt_fixups() call but by now this seems not possible. The older
> MX25L12805D has no flags set that allows a call to
> spi_nor_sfdp_init_params() and implements the fixup.
> 
> Has anyone an idea how to solve this?
> 
> 

we'll differentiate at runtime, and the differentiator will be whether
SFDP is supported or not. I'm taking care of this, patch will follow.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/