Repository navigation
Pcie: phytium: update phytium pcie controller driver support to 6.6.0.4 - #2254
Open
wangchenlu2236 wants to merge 10 commits into
Open
wangchenlu2236 wants to merge 10 commits into
wangchenlu2236 wants to merge 10 commits into
Conversation
In the phytium ps2308 platforms, when host PCIe bridge operate hotreset, then host pcie bridge config register will restore default value. So before hotreset, it need save host PCIe bridge config register. Then after hotreset, it need restore save host PCIe bridge config register. Mainline: NA Signed-off-by: Xiao Cong <xiaocong1866@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Some PCI devices have IO space access limit and only supporting 16 bit addresses. We update upper 16 bits of I/O base/limit according to the bridge and the RC root port. Mainline: NA Signed-off-by: Xiao Cong <xiaocong1866@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Some Phytium-based systems can come up with reduced PCIe link speed or lane count after a hotreset. A second reset attempt often restores the original link characteristics. Mainline: NA Signed-off-by: Li Tongfeng <litongfeng1497@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
When the PCIe device is unplugged or under hotreset, the PCIe controller's protrction mechanism is triggered, which will make the link inaccessible. This patch disables the protection after the link is up and makes the PCIe hotplug or hotreset process work well. Mainline: NA Signed-off-by: Long Shixiang <longshixiang1718@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
On Phytium 24080 platform, reading the PCIe Link Status (LNKSTA) register through the standard PCIe capability interface may return an incorrect value for the Data Link Layer Link Active (DLLLA) bit. This causes the kernel to report wrong link state after a device is hot-unplugged. Replace the DLLLA bit in LNKSTA with the correct value obtained via an SMC call to ensure accurate link status reporting. Mainline: NA Signed-off-by: Li Tongfeng <litongfeng1497@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Add the device tree binding schema for phytium,pe2201-pcie-ep. Mainline: NA Signed-off-by: Li Tongfeng <litongfeng1497@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Bugfix find_first_zero_bit function call problem, because the bitmap of ob_region_map is of type long and BITS_PER_LONG is sufficient. Signed-off-by: Mao Hongbo <maohongbo@phytium.com.cn> Signed-off-by: Li Mingzhe <limingzhe1839@phytium.com.cn> Signed-off-by: Chen Baozi <chenbaozi@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Add phytium ep driver DMA Controller support and add function_num_map to the configfs of EPC. Mainline: NA Signed-off-by: Li Tongfeng <litongfeng1497@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
On PD2308 platform, it only setup root bridge I/O BAR address, and don't modify other bridge I/O BAR address after root. Mainline: NA Signed-off-by: Li Wencheng <liwencheng@phytium.com.cn> Signed-off-by: Wang Yinfeng <wangyinfeng@phytium.com.cn> Signed-off-by: Wang Chenlu <wangchenlu2236@phytium.com.cn>
Remove the duplicate helper introduced by reapplying the hotplug workaround. Keep the updated SMC helper body with its matching int return type and definitions.
Reviewer's GuideUpdates the Phytium PCIe driver with PE2201 endpoint and virtual-network DMA support, corrected endpoint BAR/function mapping and bridge I/O handling, and SoC-specific SMC, hotplug, reset, link-status, and link-degradation workarounds. Sequence diagram for Phytium endpoint DMA transfersequenceDiagram
participant EPF as PCI_endpoint_function
participant Core as PCI_EPC_core
participant PHY as Phytium_endpoint
participant DMA as DMA_engine
EPF->>Core: pci_epc_start_dma()
Core->>PHY: start_dma()
PHY->>DMA: Configure source and destination
PHY->>DMA: DMA_LENGTH()
PHY->>DMA: DMA_CONTROL()
DMA-->>PHY: Transfer status
EPF->>Core: pci_epc_dma_status()
Core->>PHY: dma_status()
PHY->>DMA: DMA_STATUS()
DMA-->>Core: Status
Sequence diagram for Phytium link-aware secondary-bus resetsequenceDiagram
participant PCI as PCI_core
participant Bridge as Phytium_PCIe_bridge
participant SMC as Phytium_SMC
participant PCIeLink as PCIe_link
PCI->>Bridge: pci_bridge_secondary_bus_reset()
Bridge->>PCI: pci_save_state()
Bridge->>SMC: phytium_pcie_ctrl_smc_op(HOTRESET)
Bridge->>PCI: pcibios_reset_secondary_bus()
PCI->>PCIeLink: pci_bridge_wait_for_secondary_bus()
PCIeLink-->>Bridge: Link speed and width
alt Link degraded on PS24080
Bridge->>Bridge: Retry secondary-bus reset
end
Bridge->>PCI: pci_restore_state()
Sequence diagram for Phytium hotplug link-status handlingsequenceDiagram
participant Hotplug as PCIe_hotplug
participant SMC as Phytium_SMC
participant PCIeLink as PCIe_link
Hotplug->>SMC: phytium_pcie_ctrl_smc_op(GET_LNKSTA)
SMC-->>Hotplug: Link status
Hotplug->>PCIeLink: Update PCI_EXP_LNKSTA_DLLLA
Hotplug->>SMC: phytium_pcie_ctrl_smc_op(HOTPLUG)
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 9 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="drivers/pci/endpoint/pci-ep-cfs.c" line_range="226-227" />
<code_context>
+ if (ret)
+ return ret;
+
+ epc_group->function_num_map = function_num_map;
+ epc->function_num_map = function_num_map;
+
+ return len;
</code_context>
<issue_to_address>
**Function numbers collide or linking fails**
When the configfs map is written while EPFs are attached or being added, or a nonzero map is set before linking, `pci_epc_function_num_map_store()` writes the configured map into `epc->function_num_map`, which `pci_epc_add_epf()` also uses for allocated function numbers. The write can clear bits for attached EPFs and let a later attach reuse their numbers, while a nonzero map can be mistaken for already-allocated numbers and make linking fail; the configfs copy can also become stale after EPFs are linked or removed.
Keep the configured function map separate from the allocation bitmap, update allocation state under `epc->list_lock`, and have configfs show the authoritative map.
Also at `drivers/pci/endpoint/pci-ep-cfs.c:234-235`.
</issue_to_address>
### Comment 2
<location path="drivers/pci/controller/pcie-phytium-ep.c" line_range="78" />
<code_context>
- }
- dev_dbg(epc->dev.parent, "set bar%d mapping address 0x%pa size 0x%lx\n",
+ if (barno == 0 || barno == 3 || barno == 5)
+ return 0;
+
+ dev_dbg(&epc->dev, "set bar%d mapping address 0x%pa size 0x%zx\n",
</code_context>
<issue_to_address>
**Unprogrammed BARs report success**
When an endpoint function requests BAR0, BAR3, or BAR5, `phytium_pcie_ep_set_bar()` returns success for BAR0, BAR3, and BAR5 without programming them, and the EPC does not advertise these BARs as reserved. Endpoint functions therefore proceed as if the BARs were configured, but the host cannot use them.
Program these BARs or advertise them as reserved, and return an error rather than success for any BAR left unconfigured.
</issue_to_address>
### Comment 3
<location path="Documentation/devicetree/bindings/pci/phytium,pe2201-pcie-ep.yaml" line_range="20-25" />
<code_context>
+ const: phytium,pe2201-pcie-ep
+
+ reg:
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: reg
+ - const: mem
+
+required:
</code_context>
<issue_to_address>
**Required HPB resource fails validation**
When a PE2201 endpoint node supplies the three resources required by the driver, the binding limits `reg` to two entries and lists only `reg` and `mem`, but the driver requires an `hpb` resource and the example supplies a third resource. Schema validation therefore rejects the driver’s required configuration, including the example.
Allow three `reg` entries and add `hpb` to the `reg-names` schema.
Also at `Documentation/devicetree/bindings/pci/phytium,pe2201-pcie-ep.yaml:38`.
</issue_to_address>
### Comment 4
<location path="drivers/pci/endpoint/pci-epc-core.c" line_range="229-230" />
<code_context>
+ if (IS_ERR(epc))
+ return -EINVAL;
+
+ if (!epc->ops->start_dma)
+ return 0;
+
+ spin_lock_irqsave(&epc_dma_lock, flags);
</code_context>
<issue_to_address>
**Unsupported DMA reports success**
When a caller uses either DMA API on an EPC without the corresponding callback, `pci_epc_start_dma()` and `pci_epc_dma_status()` return `0` when the EPC lacks the requested callback. Callers see success even though no transfer was started or no status was queried.
Return an error when the EPC does not implement the requested DMA operation.
Also at `drivers/pci/endpoint/pci-epc-core.c:248-249`.
</issue_to_address>
### Comment 5
<location path="drivers/pci/controller/pcie-phytium-ep.c" line_range="386" />
<code_context>
+ value = (pci_addr >> 32);
+ phytium_pcie_writel(priv, func_no, DMA_DESTADDR_UP(mode), value);
+ }
+ phytium_pcie_writel(priv, func_no, DMA_LENGTH(mode), size);
+ phytium_pcie_writel(priv, func_no, DMA_CONTROL(mode), PHYTIUM_PCIE_EP_DMA_CONTROL_VALUE);
+
</code_context>
<issue_to_address>
**Large DMA requests are shortened**
When a caller requests a transfer larger than the 32-bit DMA length register can represent, `phytium_pcie_ep_start_dma()` writes `size` to the 32-bit DMA length register, truncating the request to its low 32 bits. The engine transfers a different length while the API reports success.
Reject sizes above the register’s supported range before programming the DMA engine.
</issue_to_address>
### Comment 6
<location path="drivers/pci/endpoint/pci-epc-core.c" line_range="226-233" />
<code_context>
+ int ret;
+ unsigned long flags;
+
+ if (IS_ERR(epc))
+ return -EINVAL;
+
+ if (!epc->ops->start_dma)
+ return 0;
+
+ spin_lock_irqsave(&epc_dma_lock, flags);
+ ret = epc->ops->start_dma(epc, func_no, cpu_addr, pci_addr, size, mode);
+ spin_unlock_irqrestore(&epc_dma_lock, flags);
+
</code_context>
<issue_to_address>
**Invalid function numbers access hardware**
When a caller passes a function number greater than or equal to the EPC's configured maximum, the new DMA wrappers do not check `func_no` against `epc->max_functions`, unlike other EPC operations. An out-of-range function number reaches the Phytium driver, whose function-base calculation masks the number and can address a different function's registers; callers see DMA started or status read for the wrong function.
Validate the EPC pointer and function number in both DMA wrappers before invoking driver ops.
Also at `drivers/pci/endpoint/pci-epc-core.c:245-252`.
</issue_to_address>
### Comment 7
<location path="drivers/pci/controller/pcie-phytium-ep.c" line_range="388" />
<code_context>
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/phytium,pe2201-pcie-ep.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
</code_context>
<issue_to_address>
**Concurrent DMA transfers overwrite each other**
When two callers start DMA for the same function and mode before the first transfer completes, a second start for the same function and DMA mode rewrites the engine's address and length registers while the first transfer is still running, then starts it again; the first transfer uses corrupted parameters and the caller sees incorrect or lost data.
Track each engine's in-flight state and reject or queue another start until that transfer completes.
</issue_to_address>
### Comment 8
<location path="drivers/pci/controller/pcie-phytium-ep.c" line_range="85-101" />
<code_context>
- sz = max_t(size_t, epf_bar->size, BAR_IO_MIN_APERTURE);
- sz = 1 << fls64(sz - 1);
- sz_mask = ~(sz - 1);
+ sz_mask = ~(epf_bar->size / 2 - 1);
setting |= sz_mask;
trsl_param = TRSL_ID_IO;
} else {
setting = BAR_MEM_TYPE;
- sz = max_t(size_t, epf_bar->size, BAR_MEM_MIN_APERTURE);
- sz = 1 << fls64(sz - 1);
- sz_mask = ~(sz - 1);
- setting |= lower_32_bits(sz_mask);
if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
setting |= BAR_MEM_64BIT;
if (flags & PCI_BASE_ADDRESS_MEM_PREFETCH)
setting |= BAR_MEM_PREFETCHABLE;
-
+ sz_mask = ~(epf_bar->size / 2 - 1);
+ setting |= lower_32_bits(sz_mask);
trsl_param = TRSL_ID_MASTER;
}
+ sz = max_t(size_t, epf_bar->size / 2, BAR_MEM_MIN_APERTURE);
+ sz = 1 << fls64(sz - 1);
+ sz = ALIGN(sz, mem->window.page_size);
</code_context>
<issue_to_address>
**BAR apertures are half the requested size**
When an endpoint function configures a BAR with a non-minimum aperture, `phytium_pcie_ep_set_bar()` computes both the BAR size mask and translation-window size from `epf_bar->size / 2`. The host therefore discovers an aperture half the size requested by the endpoint function, leaving part of the configured BAR inaccessible.
Calculate the BAR mask and translation-window size from the full requested BAR size, applying only the hardware-required minimum and alignment.
</issue_to_address>
### Comment 9
<location path="include/linux/pci-epc.h" line_range="91-94" />
<code_context>
void (*stop)(struct pci_epc *epc);
+
+#ifdef CONFIG_ARCH_PHYTIUM
+ int (*start_dma)(struct pci_epc *epc, u8 func_no, u64 cpu_addr, u64 pci_addr,
+ size_t size, u8 mode);
+ int (*dma_status)(struct pci_epc *epc, u8 func_no, u8 mode);
+#endif
+
const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
</code_context>
<issue_to_address>
**DMA mode values are hidden from callers**
The public DMA callbacks and functions accept a `u8 mode`, but the read/write mode constants are declared only in the Phytium-private register header. A caller using the public EPC API cannot name the supported modes through its API header and must duplicate numeric values, making it easy to select the wrong engine.
Expose a public mode enum or named constants with the EPC DMA API and use that type in the callback signatures.
</issue_to_address>
Comment on lines
+226
to
+227
| epc_group->function_num_map = function_num_map; | ||
| epc->function_num_map = function_num_map; |
There was a problem hiding this comment.
🟠 High · Function numbers collide or linking fails
When the configfs map is written while EPFs are attached or being added, or a nonzero map is set before linking, pci_epc_function_num_map_store() writes the configured map into epc->function_num_map, which pci_epc_add_epf() also uses for allocated function numbers. The write can clear bits for attached EPFs and let a later attach reuse their numbers, while a nonzero map can be mistaken for already-allocated numbers and make linking fail; the configfs copy can also become stale after EPFs are linked or removed.
Keep the configured function map separate from the allocation bitmap, update allocation state under epc->list_lock, and have configfs show the authoritative map.
Also at drivers/pci/endpoint/pci-ep-cfs.c:234-235.
Prompt for AI agents
In `drivers/pci/endpoint/pci-ep-cfs.c` at lines 226-227:
**Function numbers collide or linking fails**
When the configfs map is written while EPFs are attached or being added, or a nonzero map is set before linking, `pci_epc_function_num_map_store()` writes the configured map into `epc->function_num_map`, which `pci_epc_add_epf()` also uses for allocated function numbers. The write can clear bits for attached EPFs and let a later attach reuse their numbers, while a nonzero map can be mistaken for already-allocated numbers and make linking fail; the configfs copy can also become stale after EPFs are linked or removed.
Keep the configured function map separate from the allocation bitmap, update allocation state under `epc->list_lock`, and have configfs show the authoritative map.
Also at `drivers/pci/endpoint/pci-ep-cfs.c:234-235`.| } | ||
| dev_dbg(epc->dev.parent, "set bar%d mapping address 0x%pa size 0x%lx\n", | ||
| if (barno == 0 || barno == 3 || barno == 5) | ||
| return 0; |
There was a problem hiding this comment.
🟠 High · Unprogrammed BARs report success
When an endpoint function requests BAR0, BAR3, or BAR5, phytium_pcie_ep_set_bar() returns success for BAR0, BAR3, and BAR5 without programming them, and the EPC does not advertise these BARs as reserved. Endpoint functions therefore proceed as if the BARs were configured, but the host cannot use them.
Program these BARs or advertise them as reserved, and return an error rather than success for any BAR left unconfigured.
Prompt for AI agents
In `drivers/pci/controller/pcie-phytium-ep.c` at line 78:
**Unprogrammed BARs report success**
When an endpoint function requests BAR0, BAR3, or BAR5, `phytium_pcie_ep_set_bar()` returns success for BAR0, BAR3, and BAR5 without programming them, and the EPC does not advertise these BARs as reserved. Endpoint functions therefore proceed as if the BARs were configured, but the host cannot use them.
Program these BARs or advertise them as reserved, and return an error rather than success for any BAR left unconfigured.
Comment on lines
+20
to
+25
| maxItems: 2 | ||
|
|
||
| reg-names: | ||
| items: | ||
| - const: reg | ||
| - const: mem |
There was a problem hiding this comment.
🟠 High · Required HPB resource fails validation
When a PE2201 endpoint node supplies the three resources required by the driver, the binding limits reg to two entries and lists only reg and mem, but the driver requires an hpb resource and the example supplies a third resource. Schema validation therefore rejects the driver’s required configuration, including the example.
Allow three reg entries and add hpb to the reg-names schema.
Also at Documentation/devicetree/bindings/pci/phytium,pe2201-pcie-ep.yaml:38.
Prompt for AI agents
In `Documentation/devicetree/bindings/pci/phytium,pe2201-pcie-ep.yaml` at lines 20-25:
**Required HPB resource fails validation**
When a PE2201 endpoint node supplies the three resources required by the driver, the binding limits `reg` to two entries and lists only `reg` and `mem`, but the driver requires an `hpb` resource and the example supplies a third resource. Schema validation therefore rejects the driver’s required configuration, including the example.
Allow three `reg` entries and add `hpb` to the `reg-names` schema.
Also at `Documentation/devicetree/bindings/pci/phytium,pe2201-pcie-ep.yaml:38`.
Comment on lines
+229
to
+230
| if (!epc->ops->start_dma) | ||
| return 0; |
There was a problem hiding this comment.
🟡 Medium · Unsupported DMA reports success
When a caller uses either DMA API on an EPC without the corresponding callback, pci_epc_start_dma() and pci_epc_dma_status() return 0 when the EPC lacks the requested callback. Callers see success even though no transfer was started or no status was queried.
Return an error when the EPC does not implement the requested DMA operation.
Also at drivers/pci/endpoint/pci-epc-core.c:248-249.
Prompt for AI agents
In `drivers/pci/endpoint/pci-epc-core.c` at lines 229-230:
**Unsupported DMA reports success**
When a caller uses either DMA API on an EPC without the corresponding callback, `pci_epc_start_dma()` and `pci_epc_dma_status()` return `0` when the EPC lacks the requested callback. Callers see success even though no transfer was started or no status was queried.
Return an error when the EPC does not implement the requested DMA operation.
Also at `drivers/pci/endpoint/pci-epc-core.c:248-249`.| value = (pci_addr >> 32); | ||
| phytium_pcie_writel(priv, func_no, DMA_DESTADDR_UP(mode), value); | ||
| } | ||
| phytium_pcie_writel(priv, func_no, DMA_LENGTH(mode), size); |
There was a problem hiding this comment.
🟡 Medium · Large DMA requests are shortened
When a caller requests a transfer larger than the 32-bit DMA length register can represent, phytium_pcie_ep_start_dma() writes size to the 32-bit DMA length register, truncating the request to its low 32 bits. The engine transfers a different length while the API reports success.
Reject sizes above the register’s supported range before programming the DMA engine.
Prompt for AI agents
In `drivers/pci/controller/pcie-phytium-ep.c` at line 386:
**Large DMA requests are shortened**
When a caller requests a transfer larger than the 32-bit DMA length register can represent, `phytium_pcie_ep_start_dma()` writes `size` to the 32-bit DMA length register, truncating the request to its low 32 bits. The engine transfers a different length while the API reports success.
Reject sizes above the register’s supported range before programming the DMA engine.
Comment on lines
+226
to
+233
| if (IS_ERR(epc)) | ||
| return -EINVAL; | ||
|
|
||
| if (!epc->ops->start_dma) | ||
| return 0; | ||
|
|
||
| spin_lock_irqsave(&epc_dma_lock, flags); | ||
| ret = epc->ops->start_dma(epc, func_no, cpu_addr, pci_addr, size, mode); |
There was a problem hiding this comment.
🟡 Medium · Invalid function numbers access hardware
When a caller passes a function number greater than or equal to the EPC's configured maximum, the new DMA wrappers do not check func_no against epc->max_functions, unlike other EPC operations. An out-of-range function number reaches the Phytium driver, whose function-base calculation masks the number and can address a different function's registers; callers see DMA started or status read for the wrong function.
Validate the EPC pointer and function number in both DMA wrappers before invoking driver ops.
Also at drivers/pci/endpoint/pci-epc-core.c:245-252.
Prompt for AI agents
In `drivers/pci/endpoint/pci-epc-core.c` at lines 226-233:
**Invalid function numbers access hardware**
When a caller passes a function number greater than or equal to the EPC's configured maximum, the new DMA wrappers do not check `func_no` against `epc->max_functions`, unlike other EPC operations. An out-of-range function number reaches the Phytium driver, whose function-base calculation masks the number and can address a different function's registers; callers see DMA started or status read for the wrong function.
Validate the EPC pointer and function number in both DMA wrappers before invoking driver ops.
Also at `drivers/pci/endpoint/pci-epc-core.c:245-252`.| } | ||
| phytium_pcie_writel(priv, func_no, DMA_LENGTH(mode), size); | ||
| phytium_pcie_writel(priv, func_no, DMA_CONTROL(mode), PHYTIUM_PCIE_EP_DMA_CONTROL_VALUE); | ||
|
|
There was a problem hiding this comment.
🟠 High · Concurrent DMA transfers overwrite each other
When two callers start DMA for the same function and mode before the first transfer completes, a second start for the same function and DMA mode rewrites the engine's address and length registers while the first transfer is still running, then starts it again; the first transfer uses corrupted parameters and the caller sees incorrect or lost data.
Track each engine's in-flight state and reject or queue another start until that transfer completes.
Prompt for AI agents
In `drivers/pci/controller/pcie-phytium-ep.c` at line 388:
**Concurrent DMA transfers overwrite each other**
When two callers start DMA for the same function and mode before the first transfer completes, a second start for the same function and DMA mode rewrites the engine's address and length registers while the first transfer is still running, then starts it again; the first transfer uses corrupted parameters and the caller sees incorrect or lost data.
Track each engine's in-flight state and reject or queue another start until that transfer completes.
Comment on lines
+85
to
+101
| sz_mask = ~(epf_bar->size / 2 - 1); | ||
| setting |= sz_mask; | ||
| trsl_param = TRSL_ID_IO; | ||
| } else { | ||
| setting = BAR_MEM_TYPE; | ||
| sz = max_t(size_t, epf_bar->size, BAR_MEM_MIN_APERTURE); | ||
| sz = 1 << fls64(sz - 1); | ||
| sz_mask = ~(sz - 1); | ||
| setting |= lower_32_bits(sz_mask); | ||
|
|
||
| if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) | ||
| setting |= BAR_MEM_64BIT; | ||
|
|
||
| if (flags & PCI_BASE_ADDRESS_MEM_PREFETCH) | ||
| setting |= BAR_MEM_PREFETCHABLE; | ||
|
|
||
| sz_mask = ~(epf_bar->size / 2 - 1); | ||
| setting |= lower_32_bits(sz_mask); | ||
| trsl_param = TRSL_ID_MASTER; | ||
| } | ||
|
|
||
| sz = max_t(size_t, epf_bar->size / 2, BAR_MEM_MIN_APERTURE); |
There was a problem hiding this comment.
🟠 High · BAR apertures are half the requested size
When an endpoint function configures a BAR with a non-minimum aperture, phytium_pcie_ep_set_bar() computes both the BAR size mask and translation-window size from epf_bar->size / 2. The host therefore discovers an aperture half the size requested by the endpoint function, leaving part of the configured BAR inaccessible.
Calculate the BAR mask and translation-window size from the full requested BAR size, applying only the hardware-required minimum and alignment.
Prompt for AI agents
In `drivers/pci/controller/pcie-phytium-ep.c` at lines 85-101:
**BAR apertures are half the requested size**
When an endpoint function configures a BAR with a non-minimum aperture, `phytium_pcie_ep_set_bar()` computes both the BAR size mask and translation-window size from `epf_bar->size / 2`. The host therefore discovers an aperture half the size requested by the endpoint function, leaving part of the configured BAR inaccessible.
Calculate the BAR mask and translation-window size from the full requested BAR size, applying only the hardware-required minimum and alignment.
Comment on lines
+91
to
+94
| int (*start_dma)(struct pci_epc *epc, u8 func_no, u64 cpu_addr, u64 pci_addr, | ||
| size_t size, u8 mode); | ||
| int (*dma_status)(struct pci_epc *epc, u8 func_no, u8 mode); | ||
| #endif |
There was a problem hiding this comment.
🟡 Medium · DMA mode values are hidden from callers
The public DMA callbacks and functions accept a u8 mode, but the read/write mode constants are declared only in the Phytium-private register header. A caller using the public EPC API cannot name the supported modes through its API header and must duplicate numeric values, making it easy to select the wrong engine.
Expose a public mode enum or named constants with the EPC DMA API and use that type in the callback signatures.
Prompt for AI agents
In `include/linux/pci-epc.h` at lines 91-94:
**DMA mode values are hidden from callers**
The public DMA callbacks and functions accept a `u8 mode`, but the read/write mode constants are declared only in the Phytium-private register header. A caller using the public EPC API cannot name the supported modes through its API header and must duplicate numeric values, making it easy to select the wrong engine.
Expose a public mode enum or named constants with the EPC DMA API and use that type in the callback signatures.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical defects can report down links as active, allocate duplicate endpoint functions, misroute MSI interrupts, and create incorrectly sized or incomplete BAR mappings.
7 open issues
What changed in this PR
Updates Phytium PCIe controller support to 6.6.0.4, adding PE2201 endpoint capabilities and platform-specific reliability changes across the PCI core and hotplug subsystem.
Changes:
- Adds PE2201 endpoint bindings, DMA operations, and function-number configuration.
- Revises endpoint BAR mappings, MSI function handling, and outbound-region allocation.
- Consolidates firmware calls and adds link-status, I/O-window, and reset workarounds.
| File | Description |
|---|---|
| include/linux/pci-epc.h | Declares Phytium endpoint DMA callbacks and APIs. |
| drivers/pci/setup-bus.c | Applies firmware-provided upper I/O-window values. |
| drivers/pci/pci.h | Consolidates Phytium PCIe firmware operations. |
| drivers/pci/pci.c | Adds reset retries and configuration restoration. |
| drivers/pci/hotplug/pciehp_hpc.c | Queries firmware for PS24080 link status. |
| drivers/pci/hotplug/pciehp_ctrl.c | Uses the consolidated hotplug firmware helper. |
| drivers/pci/endpoint/pci-epc-core.c | Implements serialized endpoint DMA API wrappers. |
| drivers/pci/endpoint/pci-ep-cfs.c | Exposes the function-number bitmap through configfs. |
| drivers/pci/controller/pcie-phytium-register.h | Adds DMA and chip-specific register definitions. |
| drivers/pci/controller/pcie-phytium-ep.h | Stores the endpoint platform-device pointer. |
| drivers/pci/controller/pcie-phytium-ep.c | Adds PE2201 and DMA support; revises endpoint mappings. |
| drivers/pci/controller/Kconfig | Restricts the endpoint driver to Phytium platforms. |
| Documentation/devicetree/bindings/pci/phytium,pe2201-pcie-ep.yaml | Documents the PE2201 endpoint device-tree interface. |
Review effort: Balanced
Comment on lines
+96
to
+97
| sz_mask = ~(epf_bar->size / 2 - 1); | ||
| setting |= lower_32_bits(sz_mask); |
| atr_size = fls64(sz - 1) - 1; | ||
|
|
||
| if (barno == 2) { | ||
| phytium_pcie_writel(priv, fn, PHYTIUM_PCI_BAR((barno + 1)), 0); |
| { | ||
| struct phytium_pcie_ep *priv = epc_get_drvdata(epc); | ||
|
|
||
| fn++; |
Comment on lines
+226
to
+227
| epc_group->function_num_map = function_num_map; | ||
| epc->function_num_map = function_num_map; |
| #ifdef CONFIG_ARCH_PHYTIUM | ||
| if (is_ps24080()) { | ||
| /* PS24080 sometimes reports link up even when it's down */ | ||
| static u16 lnksta; |
Comment on lines
+19
to
+25
| reg: | ||
| maxItems: 2 | ||
|
|
||
| reg-names: | ||
| items: | ||
| - const: reg | ||
| - const: mem |
Comment on lines
+77
to
+78
| if (barno == 0 || barno == 3 || barno == 5) | ||
| return 0; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patches updates the support for phytium pcie controller driver.
1.PCI: phytium: Fix duplicated SMC helper implementation
2.pci: phytium: Fix middle bridge I/O BAR address error issue
3.pci/pcie_phytium_ep: add EPC support for virtual network devices
4.PCI: phytium: Fix find_first_zero_bit() limit
5.dt-bindings: add phytium,pe2201-pcie-ep.yaml file
6.pci: phytium: Fix incorrect LNKSTA_DLLLA status on Phytium 24080 Soc
7.PCI: phytium: Add hotplug and hotreset workaround patch for phytium SoCs
8.pci: phytium: Retry secondary-bus reset when link degrades
9.pcie: phytium: Fix PCIe IO space access restriction issue
10.pci: phytium: Restore PCIe config space after hotrest
Summary by Sourcery
Update Phytium PCIe controller support with PE2201 endpoint capabilities and platform-specific reliability fixes.
New Features:
Bug Fixes:
Enhancements:
Documentation: