Support enhanced SPI operation in spi_dw driver #77492
Conversation
|
@tbursztyka @teburd Could you review this and let me have your feedback? |
| @@ -51,6 +51,7 @@ struct spi_dw_data { | |||
| struct spi_context ctx; | |||
| uint8_t dfs; /* dfs in bytes: 1,2 or 4 */ | |||
| uint8_t fifo_diff; /* cannot be bigger than FIFO depth */ | |||
| uint32_t version; /* ssi comp version */ | |||
| }; | |||
There was a problem hiding this comment.
Move it above dfs for better struct alignement
|
|
||
| /** @} */ | ||
|
|
||
| /** |
There was a problem hiding this comment.
That's not going to get in. First, it's not well thought through: when proposing a change in generic API, it should be possible to apply such change to a maximum of devices. I can see this has been crafted too much around DW's specs.
If you are willing to support dual/quad/octal modes on dw, SPI API is unfortunately not going to be the place. I tried it before ( see #39991 as a preliminary proposal) and it never came to an agreement on extending SPI API to support these modes.
Instead, you'll have to write a totally new driver for MSPI API.
There was a problem hiding this comment.
Thanks for sharing valuable feedback. I also had the same concern on the DW specific operation configuration and wanted to get some feedback about it first. I am also considering to have a priv_operation in spi_config for the device specific operation rather than creating a new driver since most of DW code is reusable. Do it make sense?
There was a problem hiding this comment.
No it's not the right thing to do because it would open up driver specific features up to the user directly, then user code would become:
- not generic
- thus not portable to another SPI controller.
There was a problem hiding this comment.
Hi @tbursztyka It's not generic only for device specific features. We could move it to common later once other devices need the same features which is very common steps and somewhat expected. It would be simple to create a new driver, but we may have other challenges like code reusability or backward compatibility. There are always pros and cons, but I still think reusing the existing spi_dw driver would be acceptable than creating a new one. What about using an another CONFIG to extend features in include/zephyr/drivers/spi.h with an additional field like espi_operation?
There was a problem hiding this comment.
There would be no alternative such as mspi, I would push for an update of the SPI API like #39991 etc... But we are way over this. I am sorry your PR came too late (I was alone at the time trying to push for an update, which did not get any traction, because only flash memories were the actual use case: ppl were making qspi drivers directly for flash, which made sense at the time).
The RX_SAMPLE_DELAY register controls the number of ssi_clk cycles that are delayed from the default sample time before the actual sample of the rxd input occurs. A 'rx-sample-delay' devicetree property is added to support the delay control. Signed-off-by: Younghyun Park <[email protected]>
The DFS(Data Frame Size) is at CTRLR0[4:0] in HSSI controller. Signed-off-by: Younghyun Park <[email protected]>
The TXFTLR register has 2 major fields which are TFT for triggering interrupt threshold and TXFTLR for starting transfer threshold. This is to ensure that sufficient data is ready for starting transfer. Signed-off-by: Younghyun Park <[email protected]>
myguitar
force-pushed
the
support_spi_dw_espi
branch
from
f2c6194 to
5c38fc5
Compare
Read the Synopsys SSI component version to extend supported capability based on the version. Signed-off-by: Younghyun Park <[email protected]>
Some SPI controllers support the enhanced SPI operations divided into multi phases - Instruction/Address/Data phases. Additional spi operation fields are added for the transfer type, address length and instruction length to support the enhanced operation on each transmissions. Signed-off-by: Younghyun Park <[email protected]>
Some Synopsys SPI controllers support the enhanced SPI operations and the SPI transfers need to configure the transfer type, instruction length and address length along with CTRLR0 FRF(frame format) update. Signed-off-by: Younghyun Park <[email protected]>
myguitar
force-pushed
the
support_spi_dw_espi
branch
from
5c38fc5 to
7ad1b6f
Compare
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
The series of patches is to support enhanced SPI operation in the Synopsys Designware SPI driver.