MT25P vs MT25Q bulk erase difference - how to fix?
As I still get deeper and deeper into why FAT doesn't work on my MT25Q NOR flash device, I have found a minor error in m25px.c The M25P devices have a bulk erase command (0xC7) but this is not supported by the MT25Q, which has a "die erase" command (0xC4) instead and 0xC7 is meaningless. There is a #define for this as expected: #define M25P_BE 0xc7 Adding #define M25Q_DE 0xc4 Make sense but... When the device manufacturer ID is read this is actually saved, so it can't be tested at runtime to choose the right command. The straightforward fix is to move the #define for M25_BE to within the code that tests the manufacturer ID (to ensure it's a valid device for these functions) but that is a bit messy and moves the #define away from all the other related #defines. Doesn't feel right. There is no unique CONFIG_ attribute that says whether it is or isn't expecting to find an M25P or M25Q (which is reasonable) so I can't use conditional #defines. And don't want to mess up other people's boards by changing Kconfig in any way that isn't backwards compatible, although the inviolable "Sometimes Code Duplication is OK" might suggest it would be better to copy out m25px.c as m25lx.c and make the change so that Kconfig demands you choose one or the other or both of the two types...and if I find more errors/differences that may ultimately make sense of course. Can anyone suggest the "NuttX way" to do this, assuming I don't find a myriad of other errors/differences that "force" duplication?
Re: MT25P vs MT25Q bulk erase difference - how to fix?
Hello, If the Die Erase is similar in function to the bulk erase, then it can be used instead, but this has to be done at runtime to support all the chips in the same driver. So no @define or CONFIG_ option, because choosing one would excude the others, and if you have both on a board this would be bad. The runtime fix is not heavy. My recommended way is: -add a uint8_t device_manufacturer field in the m25px structure, initialized when the JEDEC ID is obtained from the device (IIRC, m25px_initialize or something like that) -then, use this field in the m25px_erase method to choose the correct command(, and parameters if needed). I woud appreciate if you agreed to share your changes beforehand (eg pull request) so I can review this simple change, because i'm using this driver in a professional project and I would like to make sure there are no regressions. Thank you, Sebastien Le 18/01/2022 à 16:41, Tim a écrit : As I still get deeper and deeper into why FAT doesn't work on my MT25Q NOR flash device, I have found a minor error in m25px.c The M25P devices have a bulk erase command (0xC7) but this is not supported by the MT25Q, which has a "die erase" command (0xC4) instead and 0xC7 is meaningless. There is a #define for this as expected: #define M25P_BE 0xc7 Adding #define M25Q_DE 0xc4 Make sense but... When the device manufacturer ID is read this is actually saved, so it can't be tested at runtime to choose the right command. The straightforward fix is to move the #define for M25_BE to within the code that tests the manufacturer ID (to ensure it's a valid device for these functions) but that is a bit messy and moves the #define away from all the other related #defines. Doesn't feel right. There is no unique CONFIG_ attribute that says whether it is or isn't expecting to find an M25P or M25Q (which is reasonable) so I can't use conditional #defines. And don't want to mess up other people's boards by changing Kconfig in any way that isn't backwards compatible, although the inviolable "Sometimes Code Duplication is OK" might suggest it would be better to copy out m25px.c as m25lx.c and make the change so that Kconfig demands you choose one or the other or both of the two types...and if I find more errors/differences that may ultimately make sense of course. Can anyone suggest the "NuttX way" to do this, assuming I don't find a myriad of other errors/differences that "force" duplication?
Re: MT25P vs MT25Q bulk erase difference - how to fix?
Thanks - makes sense. I am still not 100% convinced the M25P code is
actually correct for the M25Q. It's a work-in-progress.
This is what I have added, and all other things being equal, I will use
it in the driver where there are subtle differences needed.
structm25p_dev_s
{
structmtd_dev_smtd; /* MTD interface */
FARstructspi_dev_s*dev; /* Saved SPI interface instance */
uint8_tsectorshift; /* 16 or 18 */
uint8_tpageshift; /* 8 */
uint16_tnsectors; /* 128 or 64 */
uint32_tnpages; /* 32,768 or 65,536 */
***uint16_tmanufacturer**; **/* detected manufacturer */*
***uint16_tmemory**; **/* detected memory type */*
#ifdefCONFIG_M25P_SUBSECTOR_ERASE
uint8_tsubsectorshift; /* 0, 12 or 13 (4K or 8K) */
#endif
};
and
SPI_SEND(priv->dev, M25P_RDID);
manufacturer= SPI_SEND(priv->dev, M25P_DUMMY);
memory= SPI_SEND(priv->dev, M25P_DUMMY);
capacity= SPI_SEND(priv->dev, M25P_DUMMY);
/* Deselect the FLASH and unlock the bus */
SPI_SELECT(priv->dev, SPIDEV_FLASH(0), false);
m25p_unlock(priv->dev);
/* save the manufacturer and memory type */
***priv**->**manufacturer**= **manufacturer**;*
***priv**->**memory**= **memory**;*
and in the erase function
/* Send the "Bulk Erase (BE)" instruction or "Die Erase" as appropriate*/
if(priv->manufacturer== M25P_MANUFACTURER&&
(priv->memory== MT25Q_MEMORY_TYPE|| priv->memory== MT25QU_MEMORY_TYPE))
{
SPI_SEND(priv->dev, M25Q_DE);
}
else
{
SPI_SEND(priv->dev, M25P_BE);
}
On 18/01/2022 16:36, Sebastien Lorquet wrote:
Hello,
If the Die Erase is similar in function to the bulk erase, then it can
be used instead, but this has to be done at runtime to support all the
chips in the same driver. So no @define or CONFIG_ option, because
choosing one would excude the others, and if you have both on a board
this would be bad.
The runtime fix is not heavy. My recommended way is:
-add a uint8_t device_manufacturer field in the m25px structure,
initialized when the JEDEC ID is obtained from the device (IIRC,
m25px_initialize or something like that)
-then, use this field in the m25px_erase method to choose the correct
command(, and parameters if needed).
I woud appreciate if you agreed to share your changes beforehand (eg
pull request) so I can review this simple change, because i'm using this
driver in a professional project and I would like to make sure there are
no regressions.
Thank you,
Sebastien
Le 18/01/2022 à 16:41, Tim a écrit :
As I still get deeper and deeper into why FAT doesn't work on my
MT25Q NOR
flash device, I have found a minor error in m25px.c
The M25P devices have a bulk erase command (0xC7) but this is not
supported
by the MT25Q, which has a "die erase" command (0xC4) instead and 0xC7 is
meaningless.
There is a #define for this as expected:
#define M25P_BE 0xc7
Adding
#define M25Q_DE 0xc4
Make sense but...
When the device manufacturer ID is read this is actually saved, so it
can't
be tested at runtime to choose the right command.
The straightforward fix is to move the #define for M25_BE to within
the code
that tests the manufacturer ID (to ensure it's a valid device for these
functions) but that is a bit messy and moves the #define away from
all the
other related #defines. Doesn't feel right.
There is no unique CONFIG_ attribute that says whether it is or isn't
expecting to find an M25P or M25Q (which is reasonable) so I can't use
conditional #defines.
And don't want to mess up other people's boards by changing Kconfig
in any
way that isn't backwards compatible, although the inviolable
"Sometimes Code
Duplication is OK" might suggest it would be better to copy out
m25px.c as
m25lx.c and make the change so that Kconfig demands you choose one or
the
other or both of the two types...and if I find more
errors/differences that
may ultimately make sense of course.
Can anyone suggest the "NuttX way" to do this, assuming I don't find a
myriad of other errors/differences that "force" duplication?
