spi/spi-v2: phytium: upgrade to 1.0.15 - #2128
Conversation
Reviewer's GuideUpgrades the Phytium SPI-V2 implementation to 1.0.15 by adding capability-driven full-duplex transfers with shared-memory/DMA paths, updating regfile-based address and feature detection, tightening command completion handling, and revising GPIO, logging, and suspend/resume setup. Sequence diagram for capability-driven SPI transfersequenceDiagram
participant SPI as SPI core
participant Controller as Phytium controller
participant Regfile as Regfile
participant RV as RV firmware
participant Memory as Shared memory or DMA buffers
SPI->>Controller: spi_phyt_transfer_one
Controller->>Controller: spi_phytium_xfer
alt DMA supported and length > 16
Controller->>Memory: Translate tx and rx addresses
Controller->>RV: spi_phytium_set with PHYTSPI_MSG_CMD_DATA_DMA_XFER
RV-->>Controller: Completion interrupt
else Shared-memory transfer
Controller->>Memory: Copy tx data to shared memory
Controller->>RV: spi_phytium_set with PHYTSPI_MSG_CMD_DATA_XFER
RV-->>Controller: Completion interrupt
Controller->>Memory: Copy rx data from shared memory
end
Controller->>Regfile: phytium_write_regfile AP2RV_INTR_STATE
Regfile-->>Controller: Command completion
Controller-->>SPI: Transfer result
State diagram for SPI-V2 duplex capabilitystateDiagram-v2
[*] --> DetectCapabilities
DetectCapabilities --> FullDuplex: SPI_REGFILE_FULL_DUPLEX set
DetectCapabilities --> HalfDuplex: SPI_REGFILE_FULL_DUPLEX clear
FullDuplex --> Transfer: tx and rx buffers present
HalfDuplex --> Transfer: one-direction transfer
HalfDuplex --> Rejected: tx and rx buffers present
Transfer --> [*]
Rejected --> [*]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="drivers/spi/spi-phytium-common.c" line_range="113" />
<code_context>
int spi_phytium_check_result(struct phytium_spi *fts)
{
- unsigned long long ms = 300000;
+ unsigned long long ms = 20000;
struct msg *msg = (struct msg *)fts->tx_shmem_addr;
</code_context>
<issue_to_address>
**issue (bug_risk):** The new default timeout is 20 seconds, but the long-timeout condition is checked through `fts->flash_erase == 2`; the chip-erase path calls `spi_phytium_flash_erase()` while `flash_erase` is still zero and sets it to 2 only after that call returns. A chip erase taking more than 20 seconds therefore times out before the intended 200-second timeout is selected.
**Triggers:** When a chip erase takes longer than 20 seconds.
**Suggested fix:** Mark the operation as a long-running chip erase before calling `spi_phytium_flash_erase()`, or pass the operation-specific timeout explicitly.
</issue_to_address>| int spi_phytium_check_result(struct phytium_spi *fts) | ||
| { | ||
| unsigned long long ms = 300000; | ||
| unsigned long long ms = 20000; |
There was a problem hiding this comment.
issue (bug_risk): The new default timeout is 20 seconds, but the long-timeout condition is checked through fts->flash_erase == 2; the chip-erase path calls spi_phytium_flash_erase() while flash_erase is still zero and sets it to 2 only after that call returns. A chip erase taking more than 20 seconds therefore times out before the intended 200-second timeout is selected.
Triggers: When a chip erase takes longer than 20 seconds.
Suggested fix: Mark the operation as a long-running chip erase before calling spi_phytium_flash_erase(), or pass the operation-specific timeout explicitly.
There was a problem hiding this comment.
🟡 Changes recommended
Critical DMA-mapping and MMIO failure-handling defects, plus multiple transfer, timeout, and timer correctness issues, remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Upgrades Phytium SPI-V2 to v1.0.15 with capability-aware transfers and reliability improvements.
Changes:
- Adds full-duplex and optional DMA transfers.
- Detects DMA, DDR-addressing, and duplex capabilities.
- Revises completion, logging, and power-management behavior.
File summaries
| File | Description |
|---|---|
drivers/spi/spi-phytium.h |
Adds capability flags and transfer definitions. |
drivers/spi/spi-phytium-v2.c |
Updates duplex, log mapping, and timer lifecycle handling. |
drivers/spi/spi-phytium-plat-v2.c |
Updates version and capability detection. |
drivers/spi/spi-phytium-common.c |
Implements full-duplex transfers and revised completion handling. |
Review details
Suppressed comments (1)
drivers/spi/spi-phytium-v2.c:467
- The newly write-combined mapping must be cleared with an I/O accessor rather than ordinary
memset(). Usememset_io()for this MMIO range.
memset(fts->log, 0, fts->log_size);
- Files reviewed: 4/4 changed files
- Comments generated: 13
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| if (len > 16 && fts->dma_get_ddrdata) { | ||
| fts->msg->cmd_subid = PHYTSPI_MSG_CMD_DATA_DMA_XFER; | ||
| tx_addr = __virt_to_phys((u64)fts->tx); |
| if (IS_ERR(fts->log)) { | ||
| dev_err(dev, "log_addr is err\n"); | ||
| return; | ||
| } |
| if (fts->flash_erase == 2) | ||
| ms = 200000; |
| reinit_completion(&fts->cmd_completion); | ||
| ms = wait_for_completion_timeout(&fts->cmd_completion, msecs_to_jiffies(ms)); | ||
| phytium_write_regfile(fts, SPI_REGFILE_AP2RV_INTR_STATE, 0x10); | ||
| ms = wait_for_completion_interruptible_timeout(&fts->cmd_completion, msecs_to_jiffies(ms)); |
| if (first == 1) | ||
| fts->msg->data[24] = 1; | ||
| else | ||
| fts->msg->data[24] = flags; | ||
| fts->msg->data[24] = first; |
| u64 tx_addr, rx_addr; | ||
| u64 *data = (u64 *)fts->tx; |
|
|
||
| fts->dma_get_ddrdata = false; | ||
| if ((read_cpuid_id() & MIDR_CPU_MODEL_MASK) == MIDR_PHYTIUM_FTC872) | ||
| fts->regfile_version = phytium_read_regfile(fts, SPI_REGFILE_VERSION_REG); |
|
|
||
| for (i = 0; i < fts->log_size; i++) | ||
| fts->log[i] = 0; | ||
| memset(fts->log, 0, fts->log_size); |
| static void spi_phyt_hw_init(struct device *dev, struct phytium_spi *fts) | ||
| { | ||
| u32 reg, i; | ||
| u32 reg, i, reg_ddr_high; |
| #define SPI_REGFILE_DDR_HIGH_REG (0x4c) | ||
| #define SPI_REGFILE_VERSION_REG (0x700) |
cf5f124 to
e636e15
Compare
This driver is exclusively for the PHYTIUM platform and is not compatible with other SoCs. This restriction prevents errors on unsupported platform. Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: Ic613fa79fd85a1a7bc8cf87e9b5e1c68eaf124dd
The driver uses global-cs register(0x100) for chip selection by default. Slove the problem of not being able to read the device ID when using the internal chip selection register(0x10). Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: I6deab9ce5a572d1e7e1f67ada9983cad550b9411
This driver is exclusively for the PHYTIUM platform and is not compatible with other SoCs. This restriction prevents errors on unsupported platform. Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: Ib6ce4ada8ac4ff88c96289a00e8482010a4ceb9a
Change the wait_for_completion_timeout function to the wait_for_completion_interruptable_timeout function. To Slove the issue of the "hung_task" during system restart, which will lead to system crash occasionally. Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: Ic4ad40fa926f1c71e5777f1cc8135460c78d7d72
When the SPI device wakes up from sleep mode, it may hang up or timeout at extremely low probability. So We delete the timer during hibernation and restore it upon waking up. Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: Ifd48d7ca011b181d5a54e1c3a3b3e21486c3be40
Replace the original loop assignment with the more standard and efficient memset function to achieve the clearing operation of the debug-log buffer. Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: I5193d537ab6068d72f87ac684d8471918ec84e83
If the SPI interrupt is enabled before the waiting period is over, it will cause the SPI controller to timeout at low probability. Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: Ie91f1793a902eb57360b175c72981f29036a04a6
The time required for most requests is less than 1 second. Only the erase-chip takes a longer time, which will take several minutes. Therefore, it is not appropriate to use a uniform maxmum duration as the timeout period. Mainline: Open-Source Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: I15b0e2f1462f9ca427559775cb3c6b686b726de3
In order to accommodate devices such as spidev and tpm that support full-duplex transmission, full-duplex support has been added to the spi-v2 driver. Mainline: NA Signed-off-by: Peng Min <pengmin1540@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: I6b5a96337b8713ed5300d4c1651ea2d14c74bfdb
Determine whether to enable DMA andwhether it is compatible with 32-bit and 45-bit physical memory addresses by reading the regfile version register added to the spi-v2 driver. Mainline: NA Signed-off-by: Peng Yao <pengyao2712@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: I4402a96de7f60f31f2a245d33832f45839c39403
Enable DMA when the SPI controller is described by ACPI using FixedDMA. The driver now detects the ACPI firmware path, acquires RX/TX channels by index, and arms the DMA path accordingly, while preserving the existing Device Tree behavior. This prevents unintended fallback to PIO on ACPI platforms. Mainline: Open-Source Signed-off-by: zhuling <zhuling2709@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: I5229227fb377251d75d3ec9fd8f345b77fd4ab8c
This patch addresses two issues with Phytium SPI-V2 CAN device handling: 1. Add hardware connection validation before accessing SPI device driver data. When CAN device is described in DTS but not physically connected, prevent crashes by checking spi_device validity before obtaining spi_mem drv data. 2. Enable GPIO-based chip select simulation in SPI subsystem. Implement GPIO CS control by extracting GPIO chip select description from DTS/ACPI configuration and managing GPIO CS state within the SPI subsystem stack. 3.The cs-gpios control is placed in the SPI subsystem,Due to code redundancy,the phytium controller driver code cs-gpios has been removed. This ensures proper handling of disconnected CAN devices and provides flexible chip select control for SPI-based CAN implementations. Mainline: NA Signed-off-by: Peng Yao <pengyao2712@phytium.com.cn> gned-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Change-Id: I28686ca7046c5a7133428d3420d8dd319303989b
Signed-off-by: liutianyu1250 <liutianyu1250@phytium.com.cn>
Summary by Sourcery
Upgrade the Phytium SPI-V2 driver for 1.0.15 hardware capabilities and more robust transfer handling.
New Features:
Bug Fixes:
Enhancements:
Chores: