Improvement for serial_nor complex lib to support dummy cmd and 4-bytes address for normal SPI Topic is solved

Use this forum for requesting small changes in ChibiOS. Large changes should be discussed in the development forum. This forum is NOT for support.
choowf
Posts: 9
Joined: Tue Oct 10, 2017 8:31 am
Has thanked: 3 times
Been thanked: 3 times

Improvement for serial_nor complex lib to support dummy cmd and 4-bytes address for normal SPI  Topic is solved

Postby choowf » Thu Mar 17, 2022 7:10 am

One of our serial nor flash required address to be 4-bytes and some commands receive need to have dummy cycles in normal SPI mode.
Suggest bus_cmd_addr_dummy_receive() and bus_cmd_dummy_receive() to be enabled for normal SPI mode as well and add SNOR_SPI_4BYTES_ADDRESS flag to tell the driver it's using 4 bytes addressing.

Code: Select all

diff --git a/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.h b/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.h
index cca029775..b5041b668 100644
--- a/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.h
+++ b/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.h
@@ -65,6 +65,15 @@
 #if !defined(SNOR_SHARED_BUS) || defined(__DOXYGEN__)
 #define SNOR_SHARED_BUS                     TRUE
 #endif
+
+/**
+ * @brief   SPI 4-bytes address switch
+ * @details If set to @p TRUE the device will use 4-bytes address
+ *          in SPI bus, only relevant if SPI is used
+ */
+#if !defined(SNOR_SPI_4BYTES_ADDRESS) || defined(__DOXYGEN__)
+#define SNOR_SPI_4BYTES_ADDRESS             FALSE
+#endif
 /** @} */
 
 /*===========================================================================*/
@@ -156,8 +165,10 @@ typedef struct {
 #ifdef __cplusplus
 extern "C" {
 #endif
+#if SNOR_SHARED_BUS == TRUE
   void bus_acquire(BUSDriver *busp, const BUSConfig *config);
   void bus_release(BUSDriver *busp);
+#endif
   void bus_cmd(BUSDriver *busp, uint32_t cmd);
   void bus_cmd_send(BUSDriver *busp, uint32_t cmd, size_t n, const uint8_t *p);
   void bus_cmd_receive(BUSDriver *busp,
@@ -175,7 +186,6 @@ extern "C" {
                             flash_offset_t offset,
                             size_t n,
                             uint8_t *p);
-#if (SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_WSPI) || defined(__DOXYGEN__)
   void bus_cmd_dummy_receive(BUSDriver *busp,
                              uint32_t cmd,
                              uint32_t dummy,
@@ -187,7 +197,6 @@ extern "C" {
                                   uint32_t dummy,
                                   size_t n,
                                   uint8_t *p);
-#endif
   void snorObjectInit(SNORDriver *devp);
   void snorStart(SNORDriver *devp, const SNORConfig *config);
   void snorStop(SNORDriver *devp);


Code: Select all

diff --git a/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.c b/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.c
index 82d90835e..d369c59cf 100644
--- a/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.c
+++ b/ChibiOS_20.x.x/os/hal/lib/complex/serial_nor/hal_serial_nor.c
@@ -169,9 +169,6 @@ static flash_error_t snor_start_erase_all(void *instance) {
   /* Actual erase implementation.*/
   err = snor_device_start_erase_all(devp);
 
-  /* Ready state again.*/
-  devp->state = FLASH_READY;
-
   /* Bus released.*/
   bus_release(devp->config->busp);
 
@@ -483,14 +480,22 @@ void bus_cmd_addr(BUSDriver *busp, uint32_t cmd, flash_offset_t offset) {
   mode.dummy = 0U;
   wspiCommand(busp, &mode);
 #else
-  uint8_t buf[4];
+  uint8_t buf[5];
 
   spiSelect(busp);
   buf[0] = cmd;
+#if (SNOR_SPI_4BYTES_ADDRESS == TRUE)
+  buf[1] = (uint8_t)(offset >> 24);
+  buf[2] = (uint8_t)(offset >> 16);
+  buf[3] = (uint8_t)(offset >> 8);
+  buf[4] = (uint8_t)(offset >> 0);
+  spiSend(busp, 5, buf);
+#else
   buf[1] = (uint8_t)(offset >> 16);
   buf[2] = (uint8_t)(offset >> 8);
   buf[3] = (uint8_t)(offset >> 0);
   spiSend(busp, 4, buf);
+#endif
   spiUnselect(busp);
 #endif
 }
@@ -522,14 +527,22 @@ void bus_cmd_addr_send(BUSDriver *busp,
   mode.dummy = 0U;
   wspiSend(busp, &mode, n, p);
 #else
-  uint8_t buf[4];
+  uint8_t buf[5];
 
   spiSelect(busp);
   buf[0] = cmd;
+#if (SNOR_SPI_4BYTES_ADDRESS == TRUE)
+  buf[1] = (uint8_t)(offset >> 24);
+  buf[2] = (uint8_t)(offset >> 16);
+  buf[3] = (uint8_t)(offset >> 8);
+  buf[4] = (uint8_t)(offset >> 0);
+  spiSend(busp, 5, buf);
+#else
   buf[1] = (uint8_t)(offset >> 16);
   buf[2] = (uint8_t)(offset >> 8);
   buf[3] = (uint8_t)(offset >> 0);
   spiSend(busp, 4, buf);
+#endif
   spiSend(busp, n, p);
   spiUnselect(busp);
 #endif
@@ -562,20 +575,27 @@ void bus_cmd_addr_receive(BUSDriver *busp,
   mode.dummy = 0U;
   wspiReceive(busp, &mode, n, p);
 #else
-  uint8_t buf[4];
+  uint8_t buf[5];
 
   spiSelect(busp);
   buf[0] = cmd;
+#if (SNOR_SPI_4BYTES_ADDRESS == TRUE)
+  buf[1] = (uint8_t)(offset >> 24);
+  buf[2] = (uint8_t)(offset >> 16);
+  buf[3] = (uint8_t)(offset >> 8);
+  buf[4] = (uint8_t)(offset >> 0);
+  spiSend(busp, 5, buf);
+#else
   buf[1] = (uint8_t)(offset >> 16);
   buf[2] = (uint8_t)(offset >> 8);
   buf[3] = (uint8_t)(offset >> 0);
   spiSend(busp, 4, buf);
+#endif
   spiReceive(busp, n, p);
   spiUnselect(busp);
 #endif
 }
 
-#if (SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_WSPI) || defined(__DOXYGEN__)
 /**
  * @brief   Sends a command followed by dummy cycles and a
  *          data receive phase.
@@ -593,6 +613,7 @@ void bus_cmd_dummy_receive(BUSDriver *busp,
                            uint32_t dummy,
                            size_t n,
                            uint8_t *p) {
+#if SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_WSPI
   wspi_command_t mode;
 
   mode.cmd   = cmd;
@@ -601,6 +622,16 @@ void bus_cmd_dummy_receive(BUSDriver *busp,
   mode.alt   = 0U;
   mode.dummy = dummy;
   wspiReceive(busp, &mode, n, p);
+#else
+  osalDbgAssert(dummy <= 15, "maximum 15 dummy bytes");
+  uint8_t buf[16];
+
+  spiSelect(busp);
+  buf[0] = cmd;
+  spiSend(busp, dummy+1, buf);
+  spiReceive(busp, n, p);
+  spiUnselect(busp);
+#endif
 }
 
 /**
@@ -622,6 +653,7 @@ void bus_cmd_addr_dummy_receive(BUSDriver *busp,
                                 uint32_t dummy,
                                 size_t n,
                                 uint8_t *p) {
+#if SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_WSPI
   wspi_command_t mode;
 
   mode.cmd   = cmd;
@@ -630,8 +662,28 @@ void bus_cmd_addr_dummy_receive(BUSDriver *busp,
   mode.alt   = 0U;
   mode.dummy = dummy;
   wspiReceive(busp, &mode, n, p);
+#else
+  osalDbgAssert(dummy <= 15, "maximum 15 dummy bytes");
+  uint8_t buf[20];
+
+  spiSelect(busp);
+  buf[0] = cmd;
+#if (SNOR_SPI_4BYTES_ADDRESS == TRUE)
+  buf[1] = (uint8_t)(offset >> 24);
+  buf[2] = (uint8_t)(offset >> 16);
+  buf[3] = (uint8_t)(offset >> 8);
+  buf[4] = (uint8_t)(offset >> 0);
+  spiSend(busp, dummy+5, buf);
+#else
+  buf[1] = (uint8_t)(offset >> 16);
+  buf[2] = (uint8_t)(offset >> 8);
+  buf[3] = (uint8_t)(offset >> 0);
+  spiSend(busp, dummy+4, buf);
+#endif
+  spiReceive(busp, n, p);
+  spiUnselect(busp);
+#endif
 }
-#endif /* SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_WSPI */
 
 /**
  * @brief   Initializes an instance.

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: Improvement for serial_nor complex lib to support dummy cmd and 4-bytes address for normal SPI

Postby Giovanni » Fri Apr 15, 2022 11:07 am

Hi,

Added support for 4 bytes address but had to do some changes, "dummy" is meant to be expressed in cycles not in bytes, the assertions checks that the passed value is a multiple of 8. I also avoided the use of that 16 bytes buffer and the number of dummies is now unlimited.

I don't have HW to test this, I would appreciate if you could confirm that the change is functional.

Giovanni

choowf
Posts: 9
Joined: Tue Oct 10, 2017 8:31 am
Has thanked: 3 times
Been thanked: 3 times

Re: Improvement for serial_nor complex lib to support dummy cmd and 4-bytes address for normal SPI

Postby choowf » Fri Apr 22, 2022 8:27 am

Giovanni wrote:Hi,

Added support for 4 bytes address but had to do some changes, "dummy" is meant to be expressed in cycles not in bytes, the assertions checks that the passed value is a multiple of 8. I also avoided the use of that 16 bytes buffer and the number of dummies is now unlimited.

I don't have HW to test this, I would appreciate if you could confirm that the change is functional.

Giovanni


Some corrections on the code:

Missing cmd statement
#if (SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_SPI) || defined(__DOXYGEN__)
void snor_spi_cmd_addr(BUSDriver *busp, uint32_t cmd, flash_offset_t offset) {
#if (SNOR_SPI_4BYTES_ADDRESS == TRUE)
uint8_t buf[5];

buf[0] = cmd;
buf[1] = (uint8_t)(offset >> 24);
buf[2] = (uint8_t)(offset >> 16);
buf[3] = (uint8_t)(offset >> 8);
buf[4] = (uint8_t)(offset >> 0);
spiSend(busp, 5, buf);
#else
uint8_t buf[4];

buf[0] = cmd;
buf[1] = (uint8_t)(offset >> 16);
buf[2] = (uint8_t)(offset >> 8);
buf[3] = (uint8_t)(offset >> 0);
spiSend(busp, 4, buf);
#endif
}
#endif


Don't do spiIgnore if dummy = 0, in both bus_cmd_dummy_receive() and bus_cmd_addr_dummy_receive()
void bus_cmd_dummy_receive(BUSDriver *busp,
uint32_t cmd,
uint32_t dummy,
size_t n,
uint8_t *p) {
#if SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_WSPI
wspi_command_t mode;

mode.cmd = cmd;
mode.cfg = SNOR_WSPI_CFG_CMD_DATA;
mode.addr = 0U;
mode.alt = 0U;
mode.dummy = dummy;
wspiReceive(busp, &mode, n, p);
#else
uint8_t buf[1];

osalDbgAssert((dummy & 7) == 0U, "multiple of 8 dummy cycles");

spiSelect(busp);
buf[0] = cmd;
spiSend(busp, 1, buf);
if(dummy != 0)
spiIgnore(busp, dummy / 8U);

spiReceive(busp, n, p);
spiUnselect(busp);
#endif
}


Also I added below into snorStart(), potential bug if SNOR_SHARED_BUS = FALSE
/* Bus acquisition.*/
bus_acquire(devp->config->busp, devp->config->buscfg);

#if ((SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_WSPI) && \
(SNOR_SHARED_BUS == FALSE)) || defined(__DOXYGEN__)
wspiStart(devp->config->busp, devp->config->buscfg);
#endif
#if (SNOR_BUS_DRIVER == SNOR_BUS_DRIVER_SPI) && \
(SNOR_SHARED_BUS == FALSE)
spiStart(devp->config->busp, devp->config->buscfg);
#endif


/* Device identification and initialization.*/
snor_device_init(devp);



Tested with my hardware and it works. Please check if above make sense.
Thanks!

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: Improvement for serial_nor complex lib to support dummy cmd and 4-bytes address for normal SPI

Postby Giovanni » Fri Apr 22, 2022 10:12 am

Hi,

Fixes committed, thanks a lot.

Giovanni

choowf
Posts: 9
Joined: Tue Oct 10, 2017 8:31 am
Has thanked: 3 times
Been thanked: 3 times

Re: Improvement for serial_nor complex lib to support dummy cmd and 4-bytes address for normal SPI

Postby choowf » Sun Apr 24, 2022 7:58 am

Hi Giovanni,

I looked at your latest fix, the "if dummy != 0" condition should cover the spiIgnore instead of spiReceive.
Besides this, we also need to do the same condition for function bus_cmd_addr_dummy_receive()

User avatar
Giovanni
Site Admin
Posts: 14444
Joined: Wed May 27, 2009 8:48 am
Location: Salerno, Italy
Has thanked: 1074 times
Been thanked: 921 times
Contact:

Re: Improvement for serial_nor complex lib to support dummy cmd and 4-bytes address for normal SPI

Postby Giovanni » Sun Apr 24, 2022 8:09 am

Hi,

Should be fixed now.

Giovanni


Return to “Small Change Requests”

Who is online

Users browsing this forum: No registered users and 12 guests