* spi-nor: maxronix MX25L12835F support @ 2021-02-15 21:53 Heiko Thiery 2021-02-16 9:27 ` Pratyush Yadav 2021-06-28 7:29 ` Tudor.Ambarus 0 siblings, 2 replies; 37+ messages in thread From: Heiko Thiery @ 2021-02-15 21:53 UTC (permalink / raw) To: linux-mtd; +Cc: Michael Walle 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-15 21:53 spi-nor: maxronix MX25L12835F support Heiko Thiery @ 2021-02-16 9:27 ` Pratyush Yadav 2021-02-16 9:45 ` Heiko Thiery 2021-02-16 9:48 ` Michael Walle 2021-06-28 7:29 ` Tudor.Ambarus 1 sibling, 2 replies; 37+ messages in thread From: Pratyush Yadav @ 2021-02-16 9:27 UTC (permalink / raw) To: Heiko Thiery; +Cc: Michael Walle, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 9:27 ` Pratyush Yadav @ 2021-02-16 9:45 ` Heiko Thiery 2021-02-16 9:48 ` Michael Walle 1 sibling, 0 replies; 37+ messages in thread From: Heiko Thiery @ 2021-02-16 9:45 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Michael Walle, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 9:27 ` Pratyush Yadav 2021-02-16 9:45 ` Heiko Thiery @ 2021-02-16 9:48 ` Michael Walle 2021-02-16 10:16 ` Pratyush Yadav 2021-02-16 11:15 ` Tudor.Ambarus 1 sibling, 2 replies; 37+ messages in thread From: Michael Walle @ 2021-02-16 9:48 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Heiko Thiery, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 9:48 ` Michael Walle @ 2021-02-16 10:16 ` Pratyush Yadav 2021-02-16 10:20 ` Pratyush Yadav 2021-02-16 11:15 ` Tudor.Ambarus 1 sibling, 1 reply; 37+ messages in thread From: Pratyush Yadav @ 2021-02-16 10:16 UTC (permalink / raw) To: Michael Walle; +Cc: Heiko Thiery, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 10:16 ` Pratyush Yadav @ 2021-02-16 10:20 ` Pratyush Yadav 2021-02-16 10:41 ` Heiko Thiery 0 siblings, 1 reply; 37+ messages in thread From: Pratyush Yadav @ 2021-02-16 10:20 UTC (permalink / raw) To: Michael Walle; +Cc: Heiko Thiery, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 10:20 ` Pratyush Yadav @ 2021-02-16 10:41 ` Heiko Thiery 2021-02-16 10:48 ` Pratyush Yadav 0 siblings, 1 reply; 37+ messages in thread From: Heiko Thiery @ 2021-02-16 10:41 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Michael Walle, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 10:41 ` Heiko Thiery @ 2021-02-16 10:48 ` Pratyush Yadav 2021-02-16 10:55 ` Heiko Thiery 0 siblings, 1 reply; 37+ messages in thread From: Pratyush Yadav @ 2021-02-16 10:48 UTC (permalink / raw) To: Heiko Thiery; +Cc: Michael Walle, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 10:48 ` Pratyush Yadav @ 2021-02-16 10:55 ` Heiko Thiery 2021-02-16 11:05 ` Pratyush Yadav 0 siblings, 1 reply; 37+ messages in thread From: Heiko Thiery @ 2021-02-16 10:55 UTC (permalink / raw) To: Pratyush Yadav; +Cc: Michael Walle, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 10:55 ` Heiko Thiery @ 2021-02-16 11:05 ` Pratyush Yadav 0 siblings, 0 replies; 37+ messages in thread From: Pratyush Yadav @ 2021-02-16 11:05 UTC (permalink / raw) To: Heiko Thiery; +Cc: Michael Walle, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 9:48 ` Michael Walle 2021-02-16 10:16 ` Pratyush Yadav @ 2021-02-16 11:15 ` Tudor.Ambarus 2021-02-18 5:45 ` zhengxunli 2021-02-18 7:43 ` Heiko Thiery 1 sibling, 2 replies; 37+ messages in thread From: Tudor.Ambarus @ 2021-02-16 11:15 UTC (permalink / raw) To: michael, p.yadav, ycllin, zhengxunli, juliensu; +Cc: heiko.thiery, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 11:15 ` Tudor.Ambarus @ 2021-02-18 5:45 ` zhengxunli 2021-02-18 7:15 ` Heiko Thiery 2021-02-18 7:56 ` Tudor.Ambarus 2021-02-18 7:43 ` Heiko Thiery 1 sibling, 2 replies; 37+ messages in thread From: zhengxunli @ 2021-02-18 5:45 UTC (permalink / raw) To: Tudor.Ambarus; +Cc: juliensu, ycllin, michael, linux-mtd, heiko.thiery, p.yadav 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 5:45 ` zhengxunli @ 2021-02-18 7:15 ` Heiko Thiery 2021-02-18 7:56 ` Tudor.Ambarus 1 sibling, 0 replies; 37+ messages in thread From: Heiko Thiery @ 2021-02-18 7:15 UTC (permalink / raw) To: zhengxunli Cc: Tudor.Ambarus, juliensu, ycllin, Michael Walle, linux-mtd, p.yadav 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 5:45 ` zhengxunli 2021-02-18 7:15 ` Heiko Thiery @ 2021-02-18 7:56 ` Tudor.Ambarus 2021-02-18 8:49 ` Tudor.Ambarus 1 sibling, 1 reply; 37+ messages in thread From: Tudor.Ambarus @ 2021-02-18 7:56 UTC (permalink / raw) To: zhengxunli, masonccyang Cc: juliensu, ycllin, michael, linux-mtd, heiko.thiery, p.yadav 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 7:56 ` Tudor.Ambarus @ 2021-02-18 8:49 ` Tudor.Ambarus 0 siblings, 0 replies; 37+ messages in thread From: Tudor.Ambarus @ 2021-02-18 8:49 UTC (permalink / raw) To: zhengxunli, masonccyang Cc: juliensu, ycllin, michael, linux-mtd, heiko.thiery, p.yadav 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-16 11:15 ` Tudor.Ambarus 2021-02-18 5:45 ` zhengxunli @ 2021-02-18 7:43 ` Heiko Thiery 2021-02-18 9:27 ` Tudor.Ambarus 1 sibling, 1 reply; 37+ messages in thread From: Heiko Thiery @ 2021-02-18 7:43 UTC (permalink / raw) To: Tudor.Ambarus Cc: juliensu, ycllin, Michael Walle, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 7:43 ` Heiko Thiery @ 2021-02-18 9:27 ` Tudor.Ambarus 2021-02-18 10:15 ` Heiko Thiery 0 siblings, 1 reply; 37+ messages in thread From: Tudor.Ambarus @ 2021-02-18 9:27 UTC (permalink / raw) To: heiko.thiery, vigneshr Cc: juliensu, ycllin, michael, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply related [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 9:27 ` Tudor.Ambarus @ 2021-02-18 10:15 ` Heiko Thiery 2021-02-18 10:26 ` Tudor.Ambarus 0 siblings, 1 reply; 37+ messages in thread From: Heiko Thiery @ 2021-02-18 10:15 UTC (permalink / raw) To: Tudor.Ambarus Cc: vigneshr, juliensu, ycllin, Michael Walle, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 10:15 ` Heiko Thiery @ 2021-02-18 10:26 ` Tudor.Ambarus 2021-02-18 10:36 ` Heiko Thiery ` (2 more replies) 0 siblings, 3 replies; 37+ messages in thread From: Tudor.Ambarus @ 2021-02-18 10:26 UTC (permalink / raw) To: heiko.thiery Cc: vigneshr, juliensu, ycllin, michael, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 10:26 ` Tudor.Ambarus @ 2021-02-18 10:36 ` Heiko Thiery 2021-02-19 2:45 ` zhengxunli 2021-02-27 21:52 ` Heiko Thiery 2 siblings, 0 replies; 37+ messages in thread From: Heiko Thiery @ 2021-02-18 10:36 UTC (permalink / raw) To: Tudor.Ambarus Cc: vigneshr, juliensu, ycllin, Michael Walle, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 10:26 ` Tudor.Ambarus 2021-02-18 10:36 ` Heiko Thiery @ 2021-02-19 2:45 ` zhengxunli 2021-02-27 21:52 ` Heiko Thiery 2 siblings, 0 replies; 37+ messages in thread From: zhengxunli @ 2021-02-19 2:45 UTC (permalink / raw) To: Tudor.Ambarus Cc: vigneshr, juliensu, ycllin, michael, linux-mtd, heiko.thiery, p.yadav 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-18 10:26 ` Tudor.Ambarus 2021-02-18 10:36 ` Heiko Thiery 2021-02-19 2:45 ` zhengxunli @ 2021-02-27 21:52 ` Heiko Thiery 2021-03-01 10:52 ` Vignesh Raghavendra 2 siblings, 1 reply; 37+ messages in thread From: Heiko Thiery @ 2021-02-27 21:52 UTC (permalink / raw) To: Tudor.Ambarus Cc: vigneshr, juliensu, ycllin, Michael Walle, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-27 21:52 ` Heiko Thiery @ 2021-03-01 10:52 ` Vignesh Raghavendra 2021-03-01 11:11 ` Tudor.Ambarus 0 siblings, 1 reply; 37+ messages in thread From: Vignesh Raghavendra @ 2021-03-01 10:52 UTC (permalink / raw) To: Heiko Thiery, Tudor.Ambarus Cc: juliensu, ycllin, Michael Walle, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 10:52 ` Vignesh Raghavendra @ 2021-03-01 11:11 ` Tudor.Ambarus 2021-03-01 13:36 ` Pratyush Yadav 0 siblings, 1 reply; 37+ messages in thread From: Tudor.Ambarus @ 2021-03-01 11:11 UTC (permalink / raw) To: vigneshr, heiko.thiery Cc: juliensu, ycllin, michael, linux-mtd, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 11:11 ` Tudor.Ambarus @ 2021-03-01 13:36 ` Pratyush Yadav 2021-03-01 13:50 ` Michael Walle 2021-03-01 14:03 ` Tudor.Ambarus 0 siblings, 2 replies; 37+ messages in thread From: Pratyush Yadav @ 2021-03-01 13:36 UTC (permalink / raw) To: Tudor.Ambarus Cc: vigneshr, juliensu, ycllin, michael, linux-mtd, heiko.thiery, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 13:36 ` Pratyush Yadav @ 2021-03-01 13:50 ` Michael Walle 2021-03-01 14:09 ` Tudor.Ambarus 2021-03-01 15:40 ` Pratyush Yadav 2021-03-01 14:03 ` Tudor.Ambarus 1 sibling, 2 replies; 37+ messages in thread From: Michael Walle @ 2021-03-01 13:50 UTC (permalink / raw) To: Pratyush Yadav Cc: vigneshr, Tudor.Ambarus, juliensu, ycllin, linux-mtd, heiko.thiery, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 13:50 ` Michael Walle @ 2021-03-01 14:09 ` Tudor.Ambarus 2021-03-01 14:42 ` Michael Walle 2021-03-01 15:40 ` Pratyush Yadav 1 sibling, 1 reply; 37+ messages in thread From: Tudor.Ambarus @ 2021-03-01 14:09 UTC (permalink / raw) To: michael, p.yadav Cc: vigneshr, juliensu, ycllin, linux-mtd, heiko.thiery, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 14:09 ` Tudor.Ambarus @ 2021-03-01 14:42 ` Michael Walle 2021-03-01 15:25 ` Tudor.Ambarus 0 siblings, 1 reply; 37+ messages in thread From: Michael Walle @ 2021-03-01 14:42 UTC (permalink / raw) To: Tudor.Ambarus Cc: vigneshr, juliensu, ycllin, linux-mtd, heiko.thiery, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 14:42 ` Michael Walle @ 2021-03-01 15:25 ` Tudor.Ambarus 2021-03-02 5:49 ` Vignesh Raghavendra 0 siblings, 1 reply; 37+ messages in thread From: Tudor.Ambarus @ 2021-03-01 15:25 UTC (permalink / raw) To: michael Cc: vigneshr, juliensu, ycllin, linux-mtd, heiko.thiery, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 15:25 ` Tudor.Ambarus @ 2021-03-02 5:49 ` Vignesh Raghavendra 2021-03-03 13:44 ` Heiko Thiery 0 siblings, 1 reply; 37+ messages in thread From: Vignesh Raghavendra @ 2021-03-02 5:49 UTC (permalink / raw) To: Tudor.Ambarus, michael Cc: juliensu, ycllin, linux-mtd, heiko.thiery, p.yadav, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-02 5:49 ` Vignesh Raghavendra @ 2021-03-03 13:44 ` Heiko Thiery 2021-03-04 7:02 ` Vignesh Raghavendra 0 siblings, 1 reply; 37+ messages in thread From: Heiko Thiery @ 2021-03-03 13:44 UTC (permalink / raw) To: Vignesh Raghavendra Cc: Tudor.Ambarus, Michael Walle, p.yadav, ycllin, zhengxunli, juliensu, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-03 13:44 ` Heiko Thiery @ 2021-03-04 7:02 ` Vignesh Raghavendra 2021-03-04 7:10 ` Heiko Thiery 0 siblings, 1 reply; 37+ messages in thread From: Vignesh Raghavendra @ 2021-03-04 7:02 UTC (permalink / raw) To: Heiko Thiery Cc: Tudor.Ambarus, Michael Walle, p.yadav, ycllin, zhengxunli, juliensu, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-04 7:02 ` Vignesh Raghavendra @ 2021-03-04 7:10 ` Heiko Thiery 2021-03-19 14:33 ` Stefan Herbrechtsmeier 0 siblings, 1 reply; 37+ messages in thread From: Heiko Thiery @ 2021-03-04 7:10 UTC (permalink / raw) To: Vignesh Raghavendra Cc: Tudor.Ambarus, Michael Walle, p.yadav, ycllin, zhengxunli, juliensu, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-04 7:10 ` Heiko Thiery @ 2021-03-19 14:33 ` Stefan Herbrechtsmeier 0 siblings, 0 replies; 37+ messages in thread From: Stefan Herbrechtsmeier @ 2021-03-19 14:33 UTC (permalink / raw) To: Heiko Thiery, Vignesh Raghavendra Cc: Tudor.Ambarus, Michael Walle, p.yadav, ycllin, zhengxunli, juliensu, linux-mtd 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 13:50 ` Michael Walle 2021-03-01 14:09 ` Tudor.Ambarus @ 2021-03-01 15:40 ` Pratyush Yadav 1 sibling, 0 replies; 37+ messages in thread From: Pratyush Yadav @ 2021-03-01 15:40 UTC (permalink / raw) To: Michael Walle Cc: vigneshr, Tudor.Ambarus, juliensu, ycllin, linux-mtd, heiko.thiery, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-03-01 13:36 ` Pratyush Yadav 2021-03-01 13:50 ` Michael Walle @ 2021-03-01 14:03 ` Tudor.Ambarus 1 sibling, 0 replies; 37+ messages in thread From: Tudor.Ambarus @ 2021-03-01 14:03 UTC (permalink / raw) To: p.yadav Cc: vigneshr, juliensu, ycllin, michael, linux-mtd, heiko.thiery, zhengxunli 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
* Re: spi-nor: maxronix MX25L12835F support 2021-02-15 21:53 spi-nor: maxronix MX25L12835F support Heiko Thiery 2021-02-16 9:27 ` Pratyush Yadav @ 2021-06-28 7:29 ` Tudor.Ambarus 1 sibling, 0 replies; 37+ messages in thread From: Tudor.Ambarus @ 2021-06-28 7:29 UTC (permalink / raw) To: heiko.thiery, linux-mtd; +Cc: michael 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/ ^ permalink raw reply [flat|nested] 37+ messages in thread
end of thread, other threads:[~2021-06-28 7:30 UTC | newest] Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-15 21:53 spi-nor: maxronix MX25L12835F support Heiko Thiery 2021-02-16 9:27 ` Pratyush Yadav 2021-02-16 9:45 ` Heiko Thiery 2021-02-16 9:48 ` Michael Walle 2021-02-16 10:16 ` Pratyush Yadav 2021-02-16 10:20 ` Pratyush Yadav 2021-02-16 10:41 ` Heiko Thiery 2021-02-16 10:48 ` Pratyush Yadav 2021-02-16 10:55 ` Heiko Thiery 2021-02-16 11:05 ` Pratyush Yadav 2021-02-16 11:15 ` Tudor.Ambarus 2021-02-18 5:45 ` zhengxunli 2021-02-18 7:15 ` Heiko Thiery 2021-02-18 7:56 ` Tudor.Ambarus 2021-02-18 8:49 ` Tudor.Ambarus 2021-02-18 7:43 ` Heiko Thiery 2021-02-18 9:27 ` Tudor.Ambarus 2021-02-18 10:15 ` Heiko Thiery 2021-02-18 10:26 ` Tudor.Ambarus 2021-02-18 10:36 ` Heiko Thiery 2021-02-19 2:45 ` zhengxunli 2021-02-27 21:52 ` Heiko Thiery 2021-03-01 10:52 ` Vignesh Raghavendra 2021-03-01 11:11 ` Tudor.Ambarus 2021-03-01 13:36 ` Pratyush Yadav 2021-03-01 13:50 ` Michael Walle 2021-03-01 14:09 ` Tudor.Ambarus 2021-03-01 14:42 ` Michael Walle 2021-03-01 15:25 ` Tudor.Ambarus 2021-03-02 5:49 ` Vignesh Raghavendra 2021-03-03 13:44 ` Heiko Thiery 2021-03-04 7:02 ` Vignesh Raghavendra 2021-03-04 7:10 ` Heiko Thiery 2021-03-19 14:33 ` Stefan Herbrechtsmeier 2021-03-01 15:40 ` Pratyush Yadav 2021-03-01 14:03 ` Tudor.Ambarus 2021-06-28 7:29 ` Tudor.Ambarus
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.