Repository navigation
Uart: phytium: update phytium uart controller driver support to 6.6.0.4 - #2245
wangchenlu2236 wants to merge 1 commit into
Conversation
In order to decrease some hardware resources, we fulfill this solution. what's more, it may improve transmission efficiency and it can remain compatible with the older version at the same time. Mainline: NA Signed-off-by: Feng Jun <fengjun@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Reviewer's GuideUpdates the Phytium UART v2 driver to version 1.1.2 with support for controllers exposing the DDR feature: probe-time capability detection and buffer allocation enable physical-address-based RX/TX transfers, while the existing shared-memory protocol remains available for non-DDR hardware; the patch also updates protocol constants, clock defaults, naming, and UART configuration handling. Sequence diagram for Phytium UART DDR capability initializationsequenceDiagram
participant Driver as phytium_uart_probe
participant UART as UART controller
participant Memory as DDR buffers
Driver->>UART: check_feature()
UART-->>Driver: REG_CHECK_FEATURE with BIT_SUPPORT_DDR
alt DDR supported
Driver->>Memory: devm_kzalloc(DDR_BUF_SIZE) for rx_addr
Driver->>UART: set_rx_addr()
Driver->>UART: phyt_msg_insert(SET_DDR_BASE)
Driver->>Memory: devm_kzalloc(DDR_BUF_SIZE) for tx_addr
else DDR unsupported
Driver->>Driver: use_ddr = false
end
Sequence diagram for Phytium UART DDR transmit and receivesequenceDiagram
participant TTY as UART TTY layer
participant Driver as Phytium UART driver
participant Ring as Shared-memory ring
participant DDR as DDR buffers
participant UART as UART controller
TTY->>Driver: phytium_tx_chars()
alt use_ddr
Driver->>DDR: write TX bytes to tx_addr
Driver->>Ring: phyt_msg_insert(DDR physical address, length)
Ring->>UART: deliver TX descriptor
UART-->>Ring: RX descriptor with DDR address and length
Driver->>DDR: read RX bytes via rx_addr
else shared-memory protocol
Driver->>Ring: phyt_msg_insert(inline message data)
Ring->>UART: deliver TX message
UART-->>Ring: RX message with inline data
Driver->>Ring: read RX message data
end
Driver-->>TTY: deliver or transmit UART characters
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 |
|
Hi @wangchenlu2236. Thanks for your PR. I'm waiting for a deepin-community member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 13 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="186-187" />
<code_context>
- count = handler_msg->length / 2;
+
+ if (pup->use_ddr == true) {
+ pup->rx_addr = phys_to_virt(*(u64 *)&handler_msg->data[0]);
+ count = (*(u32 *)&handler_msg->data[8]) / 2;
+ index = count;
+ } else
</code_context>
<issue_to_address>
**Forged RX descriptor exposes kernel memory**
When A controller or firmware can place a forged DDR RX descriptor in the shared message ring, `phytium_fifo_to_tty` converts the RX descriptor’s physical address directly with `phys_to_virt` and reads from it without checking that it refers to the configured RX allocation. A controller or firmware that can forge a shared-memory descriptor can make the driver copy kernel memory into the UART receive stream, where a process with access to the TTY can read it.
Validate descriptor addresses and lengths against the configured RX buffer before converting or dereferencing them.
</issue_to_address>
### Comment 2
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="324" />
<code_context>
if (tx_ring_buffer_is_full(pup))
return false;
+ if (pup->use_ddr == true) {
+ u64 tmp_addr = (u64)&pup->tx_addr[xmit->tail];
+ /*
+ * check current addr is aligned by 4bytes? if not, align
</code_context>
<issue_to_address>
**DDR writes corrupt adjacent memory**
When DDR transmission advances the transmit tail beyond the 2048-byte buffer, `phytium_tx_chars` indexes `tx_addr` with the larger UART transmit-ring tail, so it writes beyond the allocation and corrupts adjacent kernel memory.
Allocate a buffer large enough for the transmit ring or bound and wrap every DDR-buffer index and batch.
Also at `drivers/tty/serial/phytium-uart-v2.c:347`, `drivers/tty/serial/phytium-uart-v2.c:1339`, `drivers/tty/serial/phytium-uart-v2.h:67-68`.
</issue_to_address>
### Comment 3
<location path="drivers/tty/serial/phytium-uart-v2.h" line_range="66" />
<code_context>
+#define REG_CHECK_FEATURE 0x700
+#define BIT_SUPPORT_DDR BIT(16)
+#define TYPE_MAX_LEN 12
+#define DATA_MAX_LEN 160
+#define DDR_BUF_SIZE 2048
+#define DDR_RX_MSG_SIZE 0x20
</code_context>
<issue_to_address>
**Legacy messages corrupt shared memory**
When the controller uses the non-DDR message path, `phyt_msg_insert` copies the 168-byte `struct msg` into 128-byte legacy slots, overwriting subsequent slots and, at the end of the TX ring, the RX area; legacy UART traffic is corrupted.
Keep the legacy message within its slot or copy only the legacy message size.
Also at `drivers/tty/serial/phytium-uart-v2.c:61`, `drivers/tty/serial/phytium-uart-v2.c:133`.
</issue_to_address>
### Comment 4
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="335" />
<code_context>
+ shift_bit = shift_addr - tmp_addr;
+ *(u64 *)&msg.data[0] = (u64)__virt_to_phys(shift_addr);
+ } else
+ *(u64 *)&msg.data[0] = (u64)__virt_to_phys((u64)&xmit->buf[xmit->tail]);
+ }
+ if (shift_bit)
</code_context>
<issue_to_address>
**DDR transmits the wrong bytes**
When DDR transmission uses an aligned staging address, or queued data crosses the transmit-ring boundary, `phytium_tx_chars` stages bytes in `tx_addr` but can put an `xmit->buf` address in the descriptor; its aligned-index handling can also make the copy and descriptor refer to different offsets. The controller reads stale, changed, or out-of-bounds data, corrupting transmitted characters.
Make the descriptor address, copy offset, and length consistently refer to the populated DDR TX buffer, including across ring wrap.
Also at `drivers/tty/serial/phytium-uart-v2.c:336`, `drivers/tty/serial/phytium-uart-v2.c:347`.
</issue_to_address>
### Comment 5
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="1234" />
<code_context>
+
+ cmd_subid = getHexValue(cmd);
+ msg_fill(&msg, UART_MODULE_ID, MSG_SET, cmd_subid, 0);
+ mem_addr = (u64)__virt_to_phys((u64)pup->rx_addr);
+ /*
+ * we use first 8 bytes of data array store rx buffer
</code_context>
<issue_to_address>
**DDR transfers use invalid addresses**
When DDR mode runs where CPU physical addresses differ from DMA addresses or memory is non-coherent, the driver passes `__virt_to_phys` addresses for `devm_kzalloc` buffers without DMA mapping or cache synchronization, so the controller accesses invalid or stale memory and DDR transfers fail or corrupt data.
Allocate and map the buffers with the device DMA API and perform the required DMA synchronization.
Also at `drivers/tty/serial/phytium-uart-v2.c:333`, `drivers/tty/serial/phytium-uart-v2.c:1334-1339`.
</issue_to_address>
### Comment 6
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="1341" />
<code_context>
+ pup->rx_addr = devm_kzalloc(&pdev->dev, DDR_BUF_SIZE,
+ GFP_KERNEL);
+ if (!pup->rx_addr)
+ goto free;
+ set_rx_addr(pup);
+ pup->tx_addr = devm_kzalloc(&pdev->dev, DDR_BUF_SIZE,
</code_context>
<issue_to_address>
**Probe failure leaves stale state**
When a DDR TX-buffer allocation or later probe step fails after the RX address is sent, probe exits after publishing the RX address to the controller and assigning the port entry; the failure path frees the buffer without undoing that state, leaving a stale controller address and a consumed port slot.
On probe failure, clear the port entry and withdraw or reset the controller's RX-buffer configuration before freeing the buffer.
Also at `drivers/tty/serial/phytium-uart-v2.c:1337-1338`.
</issue_to_address>
### Comment 7
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="200" />
<code_context>
- ch = (handler_msg->data[2 * fifotaken]);
- ch |= (handler_msg->data[2 * fifotaken + 1] << 8);
+ if (pup->use_ddr == true) {
+ ch = pup->rx_addr[2 * fifotaken];
+ ch |= (pup->rx_addr[2 * fifotaken + 1] << 8);
+ } else {
</code_context>
<issue_to_address>
**RX data changes while copied**
When the firmware produces another RX message while the handler is draining the current DDR buffer, `phytium_fifo_to_tty` releases the RX ring slot before copying from the shared `rx_addr`; the firmware can reuse that buffer during the copy, so the TTY receives corrupted or mixed data.
Keep the RX slot unavailable until the DDR buffer has been copied, or copy the buffer before releasing the slot.
Also at `drivers/tty/serial/phytium-uart-v2.c:186`, `drivers/tty/serial/phytium-uart-v2.c:1240`.
</issue_to_address>
### Comment 8
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="187" />
<code_context>
+
+ if (pup->use_ddr == true) {
+ pup->rx_addr = phys_to_virt(*(u64 *)&handler_msg->data[0]);
+ count = (*(u32 *)&handler_msg->data[8]) / 2;
+ index = count;
+ } else
</code_context>
<issue_to_address>
**DDR receive data is discarded**
When the controller reports more than 61 16-bit characters in one DDR message, `phytium_fifo_to_tty` processes at most `RX_DATA_MAXINUM` characters and then advances past the message, discarding the remaining DDR data.
Process the full reported character count or have the controller split messages into chunks no larger than the loop limit.
</issue_to_address>
### Comment 9
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="130" />
<code_context>
-
- memcpy(pup->shmem_base + TX_MSG_SIZE * tx_tail,
+ if (pup->use_ddr == true)
+ memcpy(pup->shmem_base + DDR_TX_MSG_SIZE * tx_tail,
+ msg, DDR_TX_MSG_SIZE);
+ else
</code_context>
<issue_to_address>
**DDR replies read from wrong slots**
When DDR mode reads a modem-status, get-mctrl, or startup reply from a nonzero TX slot, `phyt_msg_insert` places DDR messages at 32-byte strides, but the modem-status, get-mctrl, and startup readers use the legacy 128-byte stride; they read another slot and report stale or incorrect state.
Use the DDR slot stride when locating replies in DDR mode.
Also at `drivers/tty/serial/phytium-uart-v2.c:131`, `drivers/tty/serial/phytium-uart-v2.c:384`, `drivers/tty/serial/phytium-uart-v2.c:504`, `drivers/tty/serial/phytium-uart-v2.c:760`.
</issue_to_address>
### Comment 10
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="290" />
<code_context>
- phytium_uart_write(1, pup, REG_CHECK_TX);
-
- PHYT_MSG_INSERT(pup, &msg);
+ phyt_msg_insert(pup, &msg);
pup->port.icount.tx++;
</code_context>
<issue_to_address>
**Urgent characters are not transmitted**
When DDR mode sends an `x_char`, such as XON or XOFF, `phytium_tx_xchar` puts the character inline in the message, but the DDR transmit path interprets the payload as a buffer address and size; the controller therefore does not transmit the urgent character.
Encode the character using the DDR payload format or send it through the DDR TX buffer.
Also at `drivers/tty/serial/phytium-uart-v2.c:130`, `drivers/tty/serial/phytium-uart-v2.c:323`.
</issue_to_address>
### Comment 11
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="1334" />
<code_context>
+
+ if (check_feature(pup)) {
+ pup->rx_addr = devm_kzalloc(&pdev->dev, DDR_BUF_SIZE,
+ GFP_KERNEL);
+ if (!pup->rx_addr)
+ goto free;
</code_context>
<issue_to_address>
**DDR allocation errors misreported**
When either DDR buffer allocation fails during probe, `phytium_uart_probe` sends either DDR-buffer allocation failure to the shared `free` label, which returns `-1` rather than `-ENOMEM` and provides no allocation-specific log. Probe failure is therefore reported as the wrong error, obscuring the cause of an unavailable UART. Return `-ENOMEM` and log the failed allocation.
Also at `drivers/tty/serial/phytium-uart-v2.c:1340`.
</issue_to_address>
### Comment 12
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="1337" />
<code_context>
+ GFP_KERNEL);
+ if (!pup->rx_addr)
+ goto free;
+ set_rx_addr(pup);
+ pup->tx_addr = devm_kzalloc(&pdev->dev, DDR_BUF_SIZE,
+ GFP_KERNEL);
</code_context>
<issue_to_address>
**Reset discards the DDR RX setup**
When the default/reset command clears the controller's DDR base configuration, probe sends the DDR base configuration before `phytium_register_port` sends the default command used to reset the UART. That reset can clear the DDR base, and the driver does not send the configuration again before receiving data, leaving DDR RX unconfigured. Send the base configuration after the reset or reapply it during startup.
</issue_to_address>
### Comment 13
<location path="drivers/tty/serial/phytium-uart-v2.c" line_range="987" />
<code_context>
msg.data[5] = crtscts & MSG_DATA_MASK;
+ /* Add uart data bit */
+ if (pup->port.fifosize <= 1)
+ msg.data[6] = 1;
- PHYT_MSG_INSERT(pup, &msg);
</code_context>
<issue_to_address>
**FIFO-less UARTs get wrong data width**
When a FIFO-less controller uses this driver, `phytium_set_termios` checks whether `fifosize` is at most one, but probe sets it to `UART_FIFOSIZE` (64) and does not otherwise change it; the required data-width flag is never sent.
Set `fifosize` from the hardware configuration or identify FIFO-less controllers directly when setting the flag.
Also at `drivers/tty/serial/phytium-uart-v2.c:986`.
</issue_to_address>| pup->rx_addr = phys_to_virt(*(u64 *)&handler_msg->data[0]); | ||
| count = (*(u32 *)&handler_msg->data[8]) / 2; |
There was a problem hiding this comment.
🟠 High · Forged RX descriptor exposes kernel memory
When A controller or firmware can place a forged DDR RX descriptor in the shared message ring, phytium_fifo_to_tty converts the RX descriptor’s physical address directly with phys_to_virt and reads from it without checking that it refers to the configured RX allocation. A controller or firmware that can forge a shared-memory descriptor can make the driver copy kernel memory into the UART receive stream, where a process with access to the TTY can read it.
Validate descriptor addresses and lengths against the configured RX buffer before converting or dereferencing them.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at lines 186-187:
**Forged RX descriptor exposes kernel memory**
When A controller or firmware can place a forged DDR RX descriptor in the shared message ring, `phytium_fifo_to_tty` converts the RX descriptor’s physical address directly with `phys_to_virt` and reads from it without checking that it refers to the configured RX allocation. A controller or firmware that can forge a shared-memory descriptor can make the driver copy kernel memory into the UART receive stream, where a process with access to the TTY can read it.
Validate descriptor addresses and lengths against the configured RX buffer before converting or dereferencing them.| if (tx_ring_buffer_is_full(pup)) | ||
| return false; | ||
| if (pup->use_ddr == true) { | ||
| u64 tmp_addr = (u64)&pup->tx_addr[xmit->tail]; |
There was a problem hiding this comment.
🔴 Critical · DDR writes corrupt adjacent memory
When DDR transmission advances the transmit tail beyond the 2048-byte buffer, phytium_tx_chars indexes tx_addr with the larger UART transmit-ring tail, so it writes beyond the allocation and corrupts adjacent kernel memory.
Allocate a buffer large enough for the transmit ring or bound and wrap every DDR-buffer index and batch.
Also at drivers/tty/serial/phytium-uart-v2.c:347, drivers/tty/serial/phytium-uart-v2.c:1339, drivers/tty/serial/phytium-uart-v2.h:67-68.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 324:
**DDR writes corrupt adjacent memory**
When DDR transmission advances the transmit tail beyond the 2048-byte buffer, `phytium_tx_chars` indexes `tx_addr` with the larger UART transmit-ring tail, so it writes beyond the allocation and corrupts adjacent kernel memory.
Allocate a buffer large enough for the transmit ring or bound and wrap every DDR-buffer index and batch.
Also at `drivers/tty/serial/phytium-uart-v2.c:347`, `drivers/tty/serial/phytium-uart-v2.c:1339`, `drivers/tty/serial/phytium-uart-v2.h:67-68`.| #define REG_CHECK_FEATURE 0x700 | ||
| #define BIT_SUPPORT_DDR BIT(16) | ||
| #define TYPE_MAX_LEN 12 | ||
| #define DATA_MAX_LEN 160 |
There was a problem hiding this comment.
🟠 High · Legacy messages corrupt shared memory
When the controller uses the non-DDR message path, phyt_msg_insert copies the 168-byte struct msg into 128-byte legacy slots, overwriting subsequent slots and, at the end of the TX ring, the RX area; legacy UART traffic is corrupted.
Keep the legacy message within its slot or copy only the legacy message size.
Also at drivers/tty/serial/phytium-uart-v2.c:61, drivers/tty/serial/phytium-uart-v2.c:133.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.h` at line 66:
**Legacy messages corrupt shared memory**
When the controller uses the non-DDR message path, `phyt_msg_insert` copies the 168-byte `struct msg` into 128-byte legacy slots, overwriting subsequent slots and, at the end of the TX ring, the RX area; legacy UART traffic is corrupted.
Keep the legacy message within its slot or copy only the legacy message size.
Also at `drivers/tty/serial/phytium-uart-v2.c:61`, `drivers/tty/serial/phytium-uart-v2.c:133`.| shift_bit = shift_addr - tmp_addr; | ||
| *(u64 *)&msg.data[0] = (u64)__virt_to_phys(shift_addr); | ||
| } else | ||
| *(u64 *)&msg.data[0] = (u64)__virt_to_phys((u64)&xmit->buf[xmit->tail]); |
There was a problem hiding this comment.
🟠 High · DDR transmits the wrong bytes
When DDR transmission uses an aligned staging address, or queued data crosses the transmit-ring boundary, phytium_tx_chars stages bytes in tx_addr but can put an xmit->buf address in the descriptor; its aligned-index handling can also make the copy and descriptor refer to different offsets. The controller reads stale, changed, or out-of-bounds data, corrupting transmitted characters.
Make the descriptor address, copy offset, and length consistently refer to the populated DDR TX buffer, including across ring wrap.
Also at drivers/tty/serial/phytium-uart-v2.c:336, drivers/tty/serial/phytium-uart-v2.c:347.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 335:
**DDR transmits the wrong bytes**
When DDR transmission uses an aligned staging address, or queued data crosses the transmit-ring boundary, `phytium_tx_chars` stages bytes in `tx_addr` but can put an `xmit->buf` address in the descriptor; its aligned-index handling can also make the copy and descriptor refer to different offsets. The controller reads stale, changed, or out-of-bounds data, corrupting transmitted characters.
Make the descriptor address, copy offset, and length consistently refer to the populated DDR TX buffer, including across ring wrap.
Also at `drivers/tty/serial/phytium-uart-v2.c:336`, `drivers/tty/serial/phytium-uart-v2.c:347`.|
|
||
| cmd_subid = getHexValue(cmd); | ||
| msg_fill(&msg, UART_MODULE_ID, MSG_SET, cmd_subid, 0); | ||
| mem_addr = (u64)__virt_to_phys((u64)pup->rx_addr); |
There was a problem hiding this comment.
🟠 High · DDR transfers use invalid addresses
When DDR mode runs where CPU physical addresses differ from DMA addresses or memory is non-coherent, the driver passes __virt_to_phys addresses for devm_kzalloc buffers without DMA mapping or cache synchronization, so the controller accesses invalid or stale memory and DDR transfers fail or corrupt data.
Allocate and map the buffers with the device DMA API and perform the required DMA synchronization.
Also at drivers/tty/serial/phytium-uart-v2.c:333, drivers/tty/serial/phytium-uart-v2.c:1334-1339.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 1234:
**DDR transfers use invalid addresses**
When DDR mode runs where CPU physical addresses differ from DMA addresses or memory is non-coherent, the driver passes `__virt_to_phys` addresses for `devm_kzalloc` buffers without DMA mapping or cache synchronization, so the controller accesses invalid or stale memory and DDR transfers fail or corrupt data.
Allocate and map the buffers with the device DMA API and perform the required DMA synchronization.
Also at `drivers/tty/serial/phytium-uart-v2.c:333`, `drivers/tty/serial/phytium-uart-v2.c:1334-1339`.|
|
||
| memcpy(pup->shmem_base + TX_MSG_SIZE * tx_tail, | ||
| if (pup->use_ddr == true) | ||
| memcpy(pup->shmem_base + DDR_TX_MSG_SIZE * tx_tail, |
There was a problem hiding this comment.
🟠 High · DDR replies read from wrong slots
When DDR mode reads a modem-status, get-mctrl, or startup reply from a nonzero TX slot, phyt_msg_insert places DDR messages at 32-byte strides, but the modem-status, get-mctrl, and startup readers use the legacy 128-byte stride; they read another slot and report stale or incorrect state.
Use the DDR slot stride when locating replies in DDR mode.
Also at drivers/tty/serial/phytium-uart-v2.c:131, drivers/tty/serial/phytium-uart-v2.c:384, drivers/tty/serial/phytium-uart-v2.c:504, drivers/tty/serial/phytium-uart-v2.c:760.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 130:
**DDR replies read from wrong slots**
When DDR mode reads a modem-status, get-mctrl, or startup reply from a nonzero TX slot, `phyt_msg_insert` places DDR messages at 32-byte strides, but the modem-status, get-mctrl, and startup readers use the legacy 128-byte stride; they read another slot and report stale or incorrect state.
Use the DDR slot stride when locating replies in DDR mode.
Also at `drivers/tty/serial/phytium-uart-v2.c:131`, `drivers/tty/serial/phytium-uart-v2.c:384`, `drivers/tty/serial/phytium-uart-v2.c:504`, `drivers/tty/serial/phytium-uart-v2.c:760`.| phytium_uart_write(1, pup, REG_CHECK_TX); | ||
|
|
||
| PHYT_MSG_INSERT(pup, &msg); | ||
| phyt_msg_insert(pup, &msg); |
There was a problem hiding this comment.
🟠 High · Urgent characters are not transmitted
When DDR mode sends an x_char, such as XON or XOFF, phytium_tx_xchar puts the character inline in the message, but the DDR transmit path interprets the payload as a buffer address and size; the controller therefore does not transmit the urgent character.
Encode the character using the DDR payload format or send it through the DDR TX buffer.
Also at drivers/tty/serial/phytium-uart-v2.c:130, drivers/tty/serial/phytium-uart-v2.c:323.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 290:
**Urgent characters are not transmitted**
When DDR mode sends an `x_char`, such as XON or XOFF, `phytium_tx_xchar` puts the character inline in the message, but the DDR transmit path interprets the payload as a buffer address and size; the controller therefore does not transmit the urgent character.
Encode the character using the DDR payload format or send it through the DDR TX buffer.
Also at `drivers/tty/serial/phytium-uart-v2.c:130`, `drivers/tty/serial/phytium-uart-v2.c:323`.|
|
||
| if (check_feature(pup)) { | ||
| pup->rx_addr = devm_kzalloc(&pdev->dev, DDR_BUF_SIZE, | ||
| GFP_KERNEL); |
There was a problem hiding this comment.
⚪ Low · DDR allocation errors misreported
When either DDR buffer allocation fails during probe, phytium_uart_probe sends either DDR-buffer allocation failure to the shared free label, which returns -1 rather than -ENOMEM and provides no allocation-specific log. Probe failure is therefore reported as the wrong error, obscuring the cause of an unavailable UART. Return -ENOMEM and log the failed allocation.
Also at drivers/tty/serial/phytium-uart-v2.c:1340.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 1334:
**DDR allocation errors misreported**
When either DDR buffer allocation fails during probe, `phytium_uart_probe` sends either DDR-buffer allocation failure to the shared `free` label, which returns `-1` rather than `-ENOMEM` and provides no allocation-specific log. Probe failure is therefore reported as the wrong error, obscuring the cause of an unavailable UART. Return `-ENOMEM` and log the failed allocation.
Also at `drivers/tty/serial/phytium-uart-v2.c:1340`.| GFP_KERNEL); | ||
| if (!pup->rx_addr) | ||
| goto free; | ||
| set_rx_addr(pup); |
There was a problem hiding this comment.
🟠 High · Reset discards the DDR RX setup
When the default/reset command clears the controller's DDR base configuration, probe sends the DDR base configuration before phytium_register_port sends the default command used to reset the UART. That reset can clear the DDR base, and the driver does not send the configuration again before receiving data, leaving DDR RX unconfigured. Send the base configuration after the reset or reapply it during startup.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 1337:
**Reset discards the DDR RX setup**
When the default/reset command clears the controller's DDR base configuration, probe sends the DDR base configuration before `phytium_register_port` sends the default command used to reset the UART. That reset can clear the DDR base, and the driver does not send the configuration again before receiving data, leaving DDR RX unconfigured. Send the base configuration after the reset or reapply it during startup.| msg.data[5] = crtscts & MSG_DATA_MASK; | ||
| /* Add uart data bit */ | ||
| if (pup->port.fifosize <= 1) | ||
| msg.data[6] = 1; |
There was a problem hiding this comment.
🟡 Medium · FIFO-less UARTs get wrong data width
When a FIFO-less controller uses this driver, phytium_set_termios checks whether fifosize is at most one, but probe sets it to UART_FIFOSIZE (64) and does not otherwise change it; the required data-width flag is never sent.
Set fifosize from the hardware configuration or identify FIFO-less controllers directly when setting the flag.
Also at drivers/tty/serial/phytium-uart-v2.c:986.
Prompt for AI agents
In `drivers/tty/serial/phytium-uart-v2.c` at line 987:
**FIFO-less UARTs get wrong data width**
When a FIFO-less controller uses this driver, `phytium_set_termios` checks whether `fifosize` is at most one, but probe sets it to `UART_FIFOSIZE` (64) and does not otherwise change it; the required data-width flag is never sent.
Set `fifosize` from the hardware configuration or identify FIFO-less controllers directly when setting the flag.
Also at `drivers/tty/serial/phytium-uart-v2.c:986`.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved UART buffer overruns, unsafe DMA buffer handling and publication, and a dangling port pointer create critical kernel memory corruption risks.
8 open issues
What changed in this PR
Adds optional DDR-backed DMA transfers to the Phytium UART v2 driver, alongside the existing message-buffer transport.
Changes:
- Detect DDR support and configure shared buffers and transfer descriptors.
- Update message capacity, the default clock, and FIFO-less UART configuration.
- Bump the driver version to 1.1.2.
| File | Description |
|---|---|
drivers/tty/serial/phytium-uart-v2.h |
Defines DDR feature flags, buffer sizes, and the buffer-configuration command. |
drivers/tty/serial/phytium-uart-v2.c |
Adds DDR detection, allocation, and transfer handling; updates driver defaults and UART configuration. |
Review effort: Balanced
| memcpy(pup->shmem_base + DDR_TX_MSG_SIZE * tx_tail, | ||
| msg, DDR_TX_MSG_SIZE); |
| memcpy(pup->shmem_base + TX_MSG_SIZE * tx_tail, | ||
| msg, sizeof(struct msg)); |
| shift_bit = shift_addr - tmp_addr; | ||
| *(u64 *)&msg.data[0] = (u64)__virt_to_phys(shift_addr); | ||
| } else | ||
| *(u64 *)&msg.data[0] = (u64)__virt_to_phys((u64)&xmit->buf[xmit->tail]); |
| msg.length = datasize; | ||
| msg.data[datasize - 1] = xmit->buf[xmit->tail]; | ||
| } else | ||
| pup->tx_addr[index++] = xmit->buf[xmit->tail]; |
| if (!pup->rx_addr) | ||
| goto free; |
|
|
||
| if (pup->use_ddr == true) { | ||
| pup->rx_addr = phys_to_virt(*(u64 *)&handler_msg->data[0]); | ||
| count = (*(u32 *)&handler_msg->data[8]) / 2; |
| phytium_uart_write(1, pup, REG_CHECK_TX); | ||
|
|
||
| PHYT_MSG_INSERT(pup, &msg); | ||
| phyt_msg_insert(pup, &msg); |
| handler_msg = | ||
| (struct msg *)(pup->shmem_base + TX_MSG_SIZE * old_tx_tail); |
This patches updates the support for phytium XXX controller driver.
1.uart-v2: phytium: Add DDR-DMA solution for uart
Summary by Sourcery
Enable Phytium UART v2 controllers to use DDR-based DMA transfers while preserving support for existing message-buffer operation.
New Features:
Enhancements:
Chores: