| Message ID | 20260901-q8b-dts-v1-1-7de0b6a73d08@radxa.com |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: Add support for Radxa Dragon Q8B | expand |
On 9/1/26 10:47 AM, Xilin Wu wrote: > The TC9563 PCI power-control driver powers off external downstream ports > whose device tree nodes have status = "disabled". > > On the Radxa Dragon Q8B, reading the Vendor ID from one of these > powered-off port functions during PCI enumeration raises an Arm SError > instead of returning an all-ones PCI error response. This prevents the > board from completing boot. I agree with the premise of the patch, but I'd like to understand why that happens - do you have any details? Konrad
On 9/2/2026 4:43 PM, Konrad Dybcio wrote: > On 9/1/26 10:47 AM, Xilin Wu wrote: >> The TC9563 PCI power-control driver powers off external downstream ports >> whose device tree nodes have status = "disabled". >> >> On the Radxa Dragon Q8B, reading the Vendor ID from one of these >> powered-off port functions during PCI enumeration raises an Arm SError >> instead of returning an all-ones PCI error response. This prevents the >> board from completing boot. > > I agree with the premise of the patch, but I'd like to understand why > that happens - do you have any details? Unfortunately, I don't have further details. The downstream kernel also includes a similar workaround, so I suspect it's related to how the TC9563 chip is designed. > > Konrad >
On 9/2/26 11:31 AM, Xilin Wu wrote: > On 9/2/2026 4:43 PM, Konrad Dybcio wrote: >> On 9/1/26 10:47 AM, Xilin Wu wrote: >>> The TC9563 PCI power-control driver powers off external downstream ports >>> whose device tree nodes have status = "disabled". >>> >>> On the Radxa Dragon Q8B, reading the Vendor ID from one of these >>> powered-off port functions during PCI enumeration raises an Arm SError >>> instead of returning an all-ones PCI error response. This prevents the >>> board from completing boot. >> >> I agree with the premise of the patch, but I'd like to understand why >> that happens - do you have any details? > > Unfortunately, I don't have further details. The downstream kernel also includes a similar workaround, so I suspect it's related to how the TC9563 chip is designed. I'm curious in why the port is still advertised on the bus if the i2c interface writes some magic bits to "disable" it on the IC side.. but perhaps that's just how that IC behaves and we can't do anything about it? Konrad
On 9/3/2026 10:40 PM, Konrad Dybcio wrote: > On 9/2/26 11:31 AM, Xilin Wu wrote: >> On 9/2/2026 4:43 PM, Konrad Dybcio wrote: >>> On 9/1/26 10:47 AM, Xilin Wu wrote: >>>> The TC9563 PCI power-control driver powers off external downstream ports >>>> whose device tree nodes have status = "disabled". >>>> >>>> On the Radxa Dragon Q8B, reading the Vendor ID from one of these >>>> powered-off port functions during PCI enumeration raises an Arm SError >>>> instead of returning an all-ones PCI error response. This prevents the >>>> board from completing boot. >>> >>> I agree with the premise of the patch, but I'd like to understand why >>> that happens - do you have any details? >> >> Unfortunately, I don't have further details. The downstream kernel also includes a similar workaround, so I suspect it's related to how the TC9563 chip is designed. > > I'm curious in why the port is still advertised on the bus if > the i2c interface writes some magic bits to "disable" it on the IC > side.. but perhaps that's just how that IC behaves and we can't > do anything about it? Yeah I think so. > > Konrad >
On Tue, Sep 01, 2026 at 04:47:54PM +0800, Xilin Wu wrote: > The TC9563 PCI power-control driver powers off external downstream ports > whose device tree nodes have status = "disabled". > > On the Radxa Dragon Q8B, reading the Vendor ID from one of these > powered-off port functions during PCI enumeration raises an Arm SError > instead of returning an all-ones PCI error response. This prevents the > board from completing boot. > > Commit 1a8c251cff20 ("PCI: move OF status = "disabled" detection to > dev->match_driver") made disabled PCI functions remain discoverable so > PCI fixups can still be applied, while suppressing driver binding later. > Preserve that behavior for endpoint functions. > > For an unavailable node describing a PCI-to-PCI bridge, however, the > node also represents a subordinate bus that must not be enumerated. > Check the device tree node before reading the Vendor ID and return a PCI > error response without accessing config space when such a bridge is > disabled. > > Functions not described by device tree and disabled endpoint functions > remain discoverable. > > Signed-off-by: Xilin Wu <sophon@radxa.com> Does this patch help? https://lore.kernel.org/linux-pci/20260819-ecam_blocker-v2-1-e7a8fdc1c5cb@oss.qualcomm.com/ - Mani
diff --git a/drivers/pci/of.c b/drivers/pci/of.c index a51dff91b196..37e9062065d7 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -24,8 +24,8 @@ * pci_set_of_node - Find and set device's DT device_node * @dev: the PCI device structure to fill * - * Returns 0 on success with of_node set or when no device is described in the - * DT. Returns -ENODEV if the device is present, but disabled in the DT. + * Return: 0 on success with of_node set or when no device is described in the + * DT. */ int pci_set_of_node(struct pci_dev *dev) { @@ -46,6 +46,29 @@ int pci_set_of_node(struct pci_dev *dev) return 0; } +/** + * pci_of_device_skip_config_read - check whether to skip probing a PCI device + * @bus: PCI bus to scan + * @devfn: device/function number to check + * + * Return: true only when firmware explicitly describes an unavailable PCI + * bridge/port node. Disabled endpoints remain discoverable so fixups still run, + * while driver binding is suppressed later in pci_bus_add_device(). + */ +bool pci_of_device_skip_config_read(struct pci_bus *bus, unsigned int devfn) +{ + if (!bus->dev.of_node) + return false; + + struct device_node *node __free(device_node) = + of_pci_find_child_device(bus->dev.of_node, devfn); + + if (!node || of_device_is_available(node)) + return false; + + return of_node_is_type(node, "pci"); +} + void pci_release_of_node(struct pci_dev *dev) { of_node_put(dev->dev.of_node); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index ba3c3fddddc2..7cb1d04d947d 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -1244,6 +1244,7 @@ u32 of_pci_get_slot_power_limit(struct device_node *node, u8 *slot_power_limit_scale); bool of_pci_preserve_config(struct device_node *node); int pci_set_of_node(struct pci_dev *dev); +bool pci_of_device_skip_config_read(struct pci_bus *bus, unsigned int devfn); void pci_release_of_node(struct pci_dev *dev); void pci_set_bus_of_node(struct pci_bus *bus); void pci_release_bus_of_node(struct pci_bus *bus); @@ -1284,6 +1285,10 @@ static inline bool of_pci_preserve_config(struct device_node *node) } static inline int pci_set_of_node(struct pci_dev *dev) { return 0; } +static inline bool pci_of_device_skip_config_read(struct pci_bus *bus, unsigned int devfn) +{ + return false; +} static inline void pci_release_of_node(struct pci_dev *dev) { } static inline void pci_set_bus_of_node(struct pci_bus *bus) { } static inline void pci_release_bus_of_node(struct pci_bus *bus) { } diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 27008e2ea5af..ce29bf78a6f7 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2586,6 +2586,11 @@ bool pci_bus_generic_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l, bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l, int timeout) { + if (pci_of_device_skip_config_read(bus, devfn)) { + PCI_SET_ERROR_RESPONSE(l); + return false; + } + return pci_bus_generic_read_dev_vendor_id(bus, devfn, l, timeout); } EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
The TC9563 PCI power-control driver powers off external downstream ports whose device tree nodes have status = "disabled". On the Radxa Dragon Q8B, reading the Vendor ID from one of these powered-off port functions during PCI enumeration raises an Arm SError instead of returning an all-ones PCI error response. This prevents the board from completing boot. Commit 1a8c251cff20 ("PCI: move OF status = "disabled" detection to dev->match_driver") made disabled PCI functions remain discoverable so PCI fixups can still be applied, while suppressing driver binding later. Preserve that behavior for endpoint functions. For an unavailable node describing a PCI-to-PCI bridge, however, the node also represents a subordinate bus that must not be enumerated. Check the device tree node before reading the Vendor ID and return a PCI error response without accessing config space when such a bridge is disabled. Functions not described by device tree and disabled endpoint functions remain discoverable. Signed-off-by: Xilin Wu <sophon@radxa.com> --- drivers/pci/of.c | 27 +++++++++++++++++++++++++-- drivers/pci/pci.h | 5 +++++ drivers/pci/probe.c | 5 +++++ 3 files changed, 35 insertions(+), 2 deletions(-)