STMicroelectronics / stm32h7xx-hal-driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32H7 series.
BSD 3-Clause "New" or "Revised" License
97 stars 43 forks source link

ospi: driver is more restrictive than necessary #77

Closed sageve closed 4 days ago

sageve commented 3 weeks ago

According to my tests the the OSPI supports not only instruction phase only, but also data phase only. However, the HAL driver does not allow this configuration.

I guess that the focus at the OSPI was on flash devices, where a single data phase is not intended, But there are other use cases.

I kindly ask for allowing single data phase instead of throwing an error. This won't break any software, but allows a broader usage of the OSPI. I am aware that this is an undocumented feature.

I can submit a pull request if you agree.

https://github.com/STMicroelectronics/stm32h7xx-hal-driver/blob/6f5e35e30ac09ba7bf44139a97b289d3df53cf9c/Src/stm32h7xx_hal_ospi.c#L3076C1-L3079C1

+      if (cmd->DataMode != HAL_OSPI_DATA_NONE)
+      {
+        /* ---- Command with data ---- */
+
+        /* Configure the CCR register with all communication parameters */
+        MODIFY_REG((*ccr_reg), (OCTOSPI_CCR_DMODE  | OCTOSPI_CCR_DDTR),
+                   (cmd->DataMode | cmd->DataDtrMode));
+      }
+      else
+      {
+        /* ---- invalid command configuration (no instruction, no address, no data) ---- */
+
+        status = HAL_ERROR;
+        hospi->ErrorCode = HAL_OSPI_ERROR_INVALID_PARAM;
+      }
+      /* MODIFIED */
+
       /* ---- Invalid command configuration (no instruction, no address) ---- */
-      status = HAL_ERROR;
-      hospi->ErrorCode = HAL_OSPI_ERROR_INVALID_PARAM;
KRASTM commented 3 weeks ago

Hello @sageve,

Thank you for the report.

As you said and according to the RM0468, for single phase command, the only case supported is instruction phase Any of these phases can be configured to be skipped but, in case of single-phase command, the only use case supported is instruction-phase-only.

However, I will share your proposal with our team in order to confirm it, and I will keep you informed.

With regards,

KRASTM commented 5 days ago

Hello @sageve,

I checked with our team, and unfortunately, they rejected the proposal because it does not align with the RM. however, there are other IP such as SPI or PSSI that support data phase only, or you can make the change that suits your application.

Please, allow me to close the issue.

Thank you in advance for your comprehension.

With regards,

sageve commented 4 days ago

Hi

Thanks for your investigation. Actually, neither SPI nor PSSI support 4bit data lane. It's a pitty that there is no possibility with the STM32H7 to just send 4bit data using your HAL. Also you won't be able to provide a MSPI (multi bit SPI) driver in zephyr, altough OSPI supports it. In case the restriction is arbitrary, the RM should be changed.

If you persist to the decision, feel free to close the issue.

Best regards