| Message ID | 20230803135858.2724342-2-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") | expand |
On Thu, Aug 03, 2023 at 04:58:56PM +0300, Vladimir Oltean wrote: > The blamed commit has broken probing on > arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi when &enetc_port0 > (PCI function 0) has status = "disabled". > > Background: pci_scan_slot() has logic to say that if the function 0 of a > device is absent, the entire device is absent and we can skip the other > functions entirely. Traditionally, this has meant that > pci_bus_read_dev_vendor_id() returns an error code for that function. > > However, since the blamed commit, there is an extra confounding > condition: function 0 of the device exists and has a valid vendor id, > but it is disabled in the device tree. In that case, pci_scan_slot() > would incorrectly skip the entire device instead of just that function. > > In the case of NXP LS1028A, status = "disabled" does not mean that the > PCI function's config space is not available for reading. It is, but the > Ethernet port is just not functionally useful with a particular SerDes > protocol configuration (0x9999) due to pinmuxing constraints of the Soc. > So, pci_scan_slot() skips all other functions on the ENETC ECAM > (enetc_port1, enetc_port2, enetc_mdio_pf3 etc) when just enetc_port0 had > to not be probed. > > There is an additional regression introduced by the change, caused by > its fundamental premise. The enetc driver needs to run code for all PCI > functions, regardless of whether they're enabled or not in the device > tree. That is no longer possible if the driver's probe function is no > longer called. But Rob recommends that we move the of_device_is_available() > detection to dev->match_driver, and this makes the PCI fixups still run > on all functions, while just probing drivers for those functions that > are enabled. So, a separate change in the enetc driver will have to move > the workarounds to a PCI fixup. I think this makes good sense, but let me make sure I understand how this works. I *think* what's happening is that this Function 0 responds to config reads, so PCI enumeration starts by discovering it normally. But after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status"), we abort in pci_setup_device() if the DT or ACPI status is "disabled," which means there's no struct pci_dev for it, no quirks can run on it, and no driver can bind to it. And, since PCI multi-function devices must have a Function 0, we don't enumerate the other functions of this device. That's a problem because (1) you need to do some initialization on Function 0 even though you don't want a driver to claim it, and (2) this is a multi-function device and you need to enumerate the other functions. What this patch does is make it so the PCI core enumerates Function 0 normally so there will be a struct pci_dev for it, the normal config space access to it will work, and it will appear in the dmesg log and lspci output, all as usual. But if the DT or ACPI status is "disabled", we will not bind a PCI driver to it. If that's true, I'd like to highlight the PCI details here and move some of the device-specific things to the driver patches, e.g., something like this: PCI: Enumerate device but don't bind driver if firmware status is 'disabled' In some configurations, the NXP LS1028A has a multi-function NIC where Function 0 is not usable as a NIC, but it's accessible via config space and it's needed for device-specific initialization. Function 0 also indicates that the NIC is a multi-function device and the kernel should look for more functions. arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi marks Function 0 as "disabled," and after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status"), Linux doesn't enumerate Function 0, which means the entire NIC is unusable because Linux doesn't enumerate the other functions either. Instead of completely ignoring a function with DT/ACPI "disabled" status, enumerate it as usual but prevent drivers from claiming it. The disabled function will still be accessible via config space, fixups will work, and it will be visible via lspci. So feel free to merge this along with the other patches via the net tree with: Acked-by: Bjorn Helgaas <bhelgaas@google.com> > Fixes: 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") > Link: https://lore.kernel.org/netdev/CAL_JsqLsVYiPLx2kcHkDQ4t=hQVCR7NHziDwi9cCFUFhx48Qow@mail.gmail.com/ > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > drivers/pci/bus.c | 4 +++- > drivers/pci/of.c | 5 ----- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index 5bc81cc0a2de..46b252bbe500 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -11,6 +11,7 @@ > #include <linux/pci.h> > #include <linux/errno.h> > #include <linux/ioport.h> > +#include <linux/of.h> > #include <linux/proc_fs.h> > #include <linux/slab.h> > > @@ -332,6 +333,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { } > */ > void pci_bus_add_device(struct pci_dev *dev) > { > + struct device_node *dn = dev->dev.of_node; > int retval; > > /* > @@ -344,7 +346,7 @@ void pci_bus_add_device(struct pci_dev *dev) > pci_proc_attach_device(dev); > pci_bridge_d3_update(dev); > > - dev->match_driver = true; > + dev->match_driver = !dn || of_device_is_available(dn); > retval = device_attach(&dev->dev); > if (retval < 0 && retval != -EPROBE_DEFER) > pci_warn(dev, "device attach failed (%d)\n", retval); > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index e51219f9f523..3c158b17dcb5 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -34,11 +34,6 @@ int pci_set_of_node(struct pci_dev *dev) > if (!node) > return 0; > > - if (!of_device_is_available(node)) { > - of_node_put(node); > - return -ENODEV; > - } > - > device_set_node(&dev->dev, of_fwnode_handle(node)); > return 0; > } > -- > 2.34.1 >
Hi Bjorn, On Tue, Aug 08, 2023 at 05:21:07PM -0500, Bjorn Helgaas wrote: > I think this makes good sense, but let me make sure I understand how > this works. > > I *think* what's happening is that this Function 0 responds to config > reads, so PCI enumeration starts by discovering it normally. But > after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status"), > we abort in pci_setup_device() if the DT or ACPI status is "disabled," > which means there's no struct pci_dev for it, no quirks can run on it, > and no driver can bind to it. And, since PCI multi-function devices > must have a Function 0, we don't enumerate the other functions of this > device. > > That's a problem because (1) you need to do some initialization on > Function 0 even though you don't want a driver to claim it, Correction: on functions 0, 1, 2 and 6 (all have PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PF), and not just on function 0. The particular nature of a hardware IP bug/afterthought makes this necessary. It may be best to look at the lspci -vvv output on this SoC: 00:00.0 Ethernet controller: Freescale Semiconductor Inc Device e100 (rev 01) (prog-if 01) Subsystem: Freescale Semiconductor Inc Device e100 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/ethernet@0,0 Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Region 0: Memory at 1f8000000 (32-bit, non-prefetchable) [enhanced] [size=256K] Region 2: Memory at 1f8160000 (32-bit, non-prefetchable) [enhanced] [size=64K] Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- FLReset+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, AtomicOpsCtl: ReqEn- Capabilities: [80] MSI-X: Enable- Count=32 Masked- Vector table: BAR=2 offset=00000000 PBA: BAR=2 offset=00000200 Capabilities: [90] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [9c] Enhanced Allocation (EA): NumEntries=4 Entry 0: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 0 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f8000000 MaxOffset: 0003ffff Entry 1: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 2 PrimaryProperties: memory space, prefetchable SecondaryProperties: memory space, non-prefetchable Base: 1f8160000 MaxOffset: 0000ffff Entry 2: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: VF-BAR 0 PrimaryProperties: VF memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f81d0000 MaxOffset: 0000ffff Entry 3: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: VF-BAR 2 PrimaryProperties: VF memory space, prefetchable SecondaryProperties: VF memory space, prefetchable Base: 1f81f0000 MaxOffset: 0000ffff Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [130 v1] Access Control Services ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- Capabilities: [140 v1] Single Root I/O Virtualization (SR-IOV) IOVCap: Migration-, Interrupt Message Number: 000 IOVCtl: Enable- Migration- Interrupt- MSE- ARIHierarchy- IOVSta: Migration- Initial VFs: 2, Total VFs: 2, Number of VFs: 0, Function Dependency Link: 00 VF offset: 8, stride: 1, Device ID: ef00 Supported Page Size: 00000013, System Page Size: 00000010 VF Migration: offset: 00000000, BIR: 0 00:00.1 Ethernet controller: Freescale Semiconductor Inc Device e100 (rev 01) (prog-if 01) Subsystem: Freescale Semiconductor Inc Device e100 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/ethernet@0,1 Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Region 0: Memory at 1f8040000 (32-bit, non-prefetchable) [enhanced] [size=256K] Region 2: Memory at 1f8170000 (32-bit, non-prefetchable) [enhanced] [size=64K] Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- FLReset+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, AtomicOpsCtl: ReqEn- Capabilities: [80] MSI-X: Enable- Count=32 Masked- Vector table: BAR=2 offset=00000000 PBA: BAR=2 offset=00000200 Capabilities: [90] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [9c] Enhanced Allocation (EA): NumEntries=4 Entry 0: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 0 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f8040000 MaxOffset: 0003ffff Entry 1: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 2 PrimaryProperties: memory space, prefetchable SecondaryProperties: memory space, non-prefetchable Base: 1f8170000 MaxOffset: 0000ffff Entry 2: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: VF-BAR 0 PrimaryProperties: VF memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f8210000 MaxOffset: 0000ffff Entry 3: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: VF-BAR 2 PrimaryProperties: VF memory space, prefetchable SecondaryProperties: VF memory space, prefetchable Base: 1f8230000 MaxOffset: 0000ffff Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [130 v1] Access Control Services ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- Capabilities: [140 v1] Single Root I/O Virtualization (SR-IOV) IOVCap: Migration-, Interrupt Message Number: 000 IOVCtl: Enable- Migration- Interrupt- MSE- ARIHierarchy- IOVSta: Migration- Initial VFs: 2, Total VFs: 2, Number of VFs: 0, Function Dependency Link: 01 VF offset: 9, stride: 1, Device ID: ef00 Supported Page Size: 00000013, System Page Size: 00000010 VF Migration: offset: 00000000, BIR: 0 00:00.2 Ethernet controller: Freescale Semiconductor Inc Device e100 (rev 01) (prog-if 01) Subsystem: Freescale Semiconductor Inc Device e100 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/ethernet@0,2 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0 IOMMU group: 2 Region 0: Memory at 1f8080000 (32-bit, non-prefetchable) [enhanced] [size=256K] Region 2: Memory at 1f8180000 (32-bit, non-prefetchable) [enhanced] [size=64K] Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- FLReset+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, AtomicOpsCtl: ReqEn- Capabilities: [80] MSI-X: Enable+ Count=16 Masked- Vector table: BAR=2 offset=00000000 PBA: BAR=2 offset=00000100 Capabilities: [90] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [9c] Enhanced Allocation (EA): NumEntries=3 Entry 0: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 0 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f8080000 MaxOffset: 0003ffff Entry 1: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 2 PrimaryProperties: memory space, prefetchable SecondaryProperties: memory space, non-prefetchable Base: 1f8180000 MaxOffset: 0000ffff Entry 2: Enable- Writable- EntrySize=3 BAR Equivalent Indicator: BAR 4 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1fc000000 MaxOffset: 003fffff Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [130 v1] Access Control Services ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- Kernel driver in use: fsl_enetc 00:00.3 System peripheral: Freescale Semiconductor Inc Device ee01 (rev 01) (prog-if 01) Subsystem: Freescale Semiconductor Inc Device ee01 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/mdio@0,3 Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- IOMMU group: 3 Region 0: Memory at 1f8100000 (32-bit, non-prefetchable) [enhanced] [size=128K] Region 2: Memory at 1f8190000 (32-bit, non-prefetchable) [enhanced] [size=64K] Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- FLReset+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, AtomicOpsCtl: ReqEn- Capabilities: [80] MSI-X: Enable- Count=1 Masked- Vector table: BAR=2 offset=00000000 PBA: BAR=2 offset=00000010 Capabilities: [90] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [9c] Enhanced Allocation (EA): NumEntries=2 Entry 0: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 0 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f8100000 MaxOffset: 0001ffff Entry 1: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 2 PrimaryProperties: memory space, prefetchable SecondaryProperties: memory space, non-prefetchable Base: 1f8190000 MaxOffset: 0000ffff Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [130 v1] Access Control Services ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- Kernel driver in use: fsl_enetc_mdio 00:00.4 System peripheral: Freescale Semiconductor Inc Device ee02 (rev 01) (prog-if 01) Subsystem: Freescale Semiconductor Inc Device ee02 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/ethernet@0,4 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0, Cache Line Size: 32 bytes IOMMU group: 4 Region 0: Memory at 1f8120000 (32-bit, non-prefetchable) [enhanced] [size=128K] Region 2: Memory at 1f81a0000 (32-bit, non-prefetchable) [enhanced] [size=64K] Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- FLReset+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, AtomicOpsCtl: ReqEn- Capabilities: [80] MSI-X: Enable+ Count=2 Masked- Vector table: BAR=2 offset=00000000 PBA: BAR=2 offset=00000020 Capabilities: [90] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [9c] Enhanced Allocation (EA): NumEntries=2 Entry 0: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 0 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f8120000 MaxOffset: 0001ffff Entry 1: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 2 PrimaryProperties: memory space, prefetchable SecondaryProperties: memory space, non-prefetchable Base: 1f81a0000 MaxOffset: 0000ffff Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [130 v1] Access Control Services ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- Kernel driver in use: fsl_enetc_ptp 00:00.5 Fabric controller: Freescale Semiconductor Inc Device eef0 (rev 01) (prog-if 01) Subsystem: Freescale Semiconductor Inc Device eef0 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/ethernet-switch@0,5 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Latency: 0, Cache Line Size: 32 bytes Interrupt: pin B routed to IRQ 96 IOMMU group: 1 Region 0: Memory at 1f8140000 (32-bit, non-prefetchable) [enhanced] [size=128K] Region 2: Memory at 1f81b0000 (32-bit, non-prefetchable) [enhanced] [size=64K] Region 4: Memory at 1fc000000 (32-bit, non-prefetchable) [enhanced] [size=4M] Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- FLReset+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, AtomicOpsCtl: ReqEn- Capabilities: [80] MSI-X: Enable- Count=1 Masked- Vector table: BAR=2 offset=00000000 PBA: BAR=2 offset=00000010 Capabilities: [90] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [9c] Enhanced Allocation (EA): NumEntries=3 Entry 0: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 0 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f8140000 MaxOffset: 0001ffff Entry 1: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 2 PrimaryProperties: memory space, prefetchable SecondaryProperties: memory space, non-prefetchable Base: 1f81b0000 MaxOffset: 0000ffff Entry 2: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 4 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1fc000000 MaxOffset: 003fffff Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [130 v1] Access Control Services ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- Kernel driver in use: mscc_felix 00:00.6 Ethernet controller: Freescale Semiconductor Inc Device e100 (rev 01) (prog-if 01) Subsystem: Freescale Semiconductor Inc Device e100 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/ethernet@0,6 Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Region 0: Memory at 1f80c0000 (32-bit, non-prefetchable) [enhanced] [size=256K] Region 2: Memory at 1f81c0000 (32-bit, non-prefetchable) [enhanced] [size=64K] Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- FLReset+ DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- FLReset- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- AtomicOpsCap: 32bit- 64bit- 128bitCAS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, AtomicOpsCtl: ReqEn- Capabilities: [80] MSI-X: Enable- Count=16 Masked- Vector table: BAR=2 offset=00000000 PBA: BAR=2 offset=00000100 Capabilities: [90] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [9c] Enhanced Allocation (EA): NumEntries=2 Entry 0: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 0 PrimaryProperties: memory space, non-prefetchable SecondaryProperties: entry unavailable for use, PrimaryProperties should be used Base: 1f80c0000 MaxOffset: 0003ffff Entry 1: Enable+ Writable- EntrySize=3 BAR Equivalent Indicator: BAR 2 PrimaryProperties: memory space, prefetchable SecondaryProperties: memory space, non-prefetchable Base: 1f81c0000 MaxOffset: 0000ffff Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 Capabilities: [130 v1] Access Control Services ACSCap: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- ACSCtl: SrcValid- TransBlk- ReqRedir- CmpltRedir- UpstreamFwd- EgressCtrl- DirectTrans- 00:1f.0 Generic system peripheral [0807]: Freescale Semiconductor Inc Device e001 (rev 01) Subsystem: Freescale Semiconductor Inc Device e001 Device tree node: /sys/firmware/devicetree/base/soc/pcie@1f0000000/rcec@1f,0 Control: I/O- Mem- BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx- Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- Interrupt: pin A routed to IRQ 20 Capabilities: [40] Express (v2) Root Complex Event Collector, MSI 00 DevCap: MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE- DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta: CorrErr- NonFatalErr- FatalErr- UnsupReq- AuxPwr- TransPend- RootCap: CRSVisible- RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna+ CRSVisible- RootSta: PME ReqID 0000, PMEStatus- PMEPending- DevCap2: Completion Timeout: Not Supported, TimeoutDis- NROPrPrP- LTR- 10BitTagComp- 10BitTagReq- OBFF Not Supported, ExtFmt- EETLPPrefix- EmergencyPowerReduction Not Supported, EmergencyPowerReductionInit- FRS- DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR- OBFF Disabled, Capabilities: [80] Power Management version 3 Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-) Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME- Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UESvrt: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- CEMsk: RxErr- BadTLP- BadDLLP- Rollover- Timeout- AdvNonFatalErr- AERCap: First Error Pointer: 00, ECRCGenCap- ECRCGenEn- ECRCChkCap- ECRCChkEn- MultHdrRecCap- MultHdrRecEn- TLPPfxPres- HdrLogCap- HeaderLog: 00000000 00000000 00000000 00000000 RootCmd: CERptEn+ NFERptEn+ FERptEn+ RootSta: CERcvd- MultCERcvd- UERcvd- MultUERcvd- FirstFatal- NonFatalMsg- FatalMsg- IntMsg 0 ErrorSrc: ERR_COR: 0000 ERR_FATAL/NONFATAL: 0000 Capabilities: [138 v1] Root Complex Event Collector <?> Kernel driver in use: pcieport > and (2) this is a multi-function device and you need to enumerate the > other functions. correct > What this patch does is make it so the PCI core enumerates Function 0 The patch does not special-case function 0. Fundamentally it changes the meaning of "disabled" in the firmware description from "not enumerable" to "driver shouldn't be bound to it" (which is more or less what the meaning was, prior to the blamed commit, except that it was down to individual drivers to observe the property) and that's absolutely it. > normally so there will be a struct pci_dev for it, the normal config > space access to it will work, and it will appear in the dmesg log and > lspci output, all as usual. But if the DT or ACPI status is > "disabled", we will not bind a PCI driver to it. > > If that's true, I'd like to highlight the PCI details here and move > some of the device-specific things to the driver patches, e.g., > something like this: > > PCI: Enumerate device but don't bind driver if firmware status is 'disabled' > > In some configurations, the NXP LS1028A has a multi-function NIC > where Function 0 is not usable as a NIC, but it's accessible via > config space and it's needed for device-specific initialization. > Function 0 also indicates that the NIC is a multi-function device > and the kernel should look for more functions. Instead of "multi-function NIC", I'd say "PCIe endpoint integrated into a root complex (RCiEP)". Reworded, it's not a NIC, it's 4, plus other device classes (system peripherals, fabric controllers). > > arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi marks Function 0 as > "disabled," and after 6fffbc7ae137 ("PCI: Honor firmware's device > disabled status"), Linux doesn't enumerate Function 0, which means > the entire NIC is unusable because Linux doesn't enumerate the other > functions either. > > Instead of completely ignoring a function with DT/ACPI "disabled" > status, enumerate it as usual but prevent drivers from claiming it. > The disabled function will still be accessible via config space, > fixups will work, and it will be visible via lspci. > > So feel free to merge this along with the other patches via the net > tree with: > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> You may have given the netdev maintainers some mixed signals with the rewording suggestion plus the ack for my wording, and now we have commit 1a8c251cff20 ("PCI: move OF status = "disabled" detection to dev->match_driver") in the net.git tree. I think we are mostly on the same page about what is changing, it's just that we are focusing on different aspects of it in the description. I hope you're ok if we close the topic the way things are now? :)
On Wed, Aug 09, 2023 at 05:45:49PM +0300, Vladimir Oltean wrote: > Hi Bjorn, > > On Tue, Aug 08, 2023 at 05:21:07PM -0500, Bjorn Helgaas wrote: > > I think this makes good sense, but let me make sure I understand how > > this works. > > > > I *think* what's happening is that this Function 0 responds to config > > reads, so PCI enumeration starts by discovering it normally. But > > after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status"), > > we abort in pci_setup_device() if the DT or ACPI status is "disabled," > > which means there's no struct pci_dev for it, no quirks can run on it, > > and no driver can bind to it. And, since PCI multi-function devices > > must have a Function 0, we don't enumerate the other functions of this > > device. > > > > That's a problem because (1) you need to do some initialization on > > Function 0 even though you don't want a driver to claim it, and > > (2) this is a multi-function device and you need to enumerate the > > other functions. > > Correction: on functions 0, 1, 2 and 6 (all have > PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PF), and not just on function > 0. The particular nature of a hardware IP bug/afterthought makes > this necessary. Thanks, so (2) is only relevant to function 0, but (1) applies to other functions as well. > You may have given the netdev maintainers some mixed signals with the > rewording suggestion plus the ack for my wording, and now we have commit > 1a8c251cff20 ("PCI: move OF status = "disabled" detection to > dev->match_driver") in the net.git tree. > > I think we are mostly on the same page about what is changing, it's just > that we are focusing on different aspects of it in the description. > > I hope you're ok if we close the topic the way things are now? :) Yep, my fault for forgetting how netdev works. Thanks for your patience. Bjorn
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 5bc81cc0a2de..46b252bbe500 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -11,6 +11,7 @@ #include <linux/pci.h> #include <linux/errno.h> #include <linux/ioport.h> +#include <linux/of.h> #include <linux/proc_fs.h> #include <linux/slab.h> @@ -332,6 +333,7 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { } */ void pci_bus_add_device(struct pci_dev *dev) { + struct device_node *dn = dev->dev.of_node; int retval; /* @@ -344,7 +346,7 @@ void pci_bus_add_device(struct pci_dev *dev) pci_proc_attach_device(dev); pci_bridge_d3_update(dev); - dev->match_driver = true; + dev->match_driver = !dn || of_device_is_available(dn); retval = device_attach(&dev->dev); if (retval < 0 && retval != -EPROBE_DEFER) pci_warn(dev, "device attach failed (%d)\n", retval); diff --git a/drivers/pci/of.c b/drivers/pci/of.c index e51219f9f523..3c158b17dcb5 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -34,11 +34,6 @@ int pci_set_of_node(struct pci_dev *dev) if (!node) return 0; - if (!of_device_is_available(node)) { - of_node_put(node); - return -ENODEV; - } - device_set_node(&dev->dev, of_fwnode_handle(node)); return 0; }
The blamed commit has broken probing on arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi when &enetc_port0 (PCI function 0) has status = "disabled". Background: pci_scan_slot() has logic to say that if the function 0 of a device is absent, the entire device is absent and we can skip the other functions entirely. Traditionally, this has meant that pci_bus_read_dev_vendor_id() returns an error code for that function. However, since the blamed commit, there is an extra confounding condition: function 0 of the device exists and has a valid vendor id, but it is disabled in the device tree. In that case, pci_scan_slot() would incorrectly skip the entire device instead of just that function. In the case of NXP LS1028A, status = "disabled" does not mean that the PCI function's config space is not available for reading. It is, but the Ethernet port is just not functionally useful with a particular SerDes protocol configuration (0x9999) due to pinmuxing constraints of the Soc. So, pci_scan_slot() skips all other functions on the ENETC ECAM (enetc_port1, enetc_port2, enetc_mdio_pf3 etc) when just enetc_port0 had to not be probed. There is an additional regression introduced by the change, caused by its fundamental premise. The enetc driver needs to run code for all PCI functions, regardless of whether they're enabled or not in the device tree. That is no longer possible if the driver's probe function is no longer called. But Rob recommends that we move the of_device_is_available() detection to dev->match_driver, and this makes the PCI fixups still run on all functions, while just probing drivers for those functions that are enabled. So, a separate change in the enetc driver will have to move the workarounds to a PCI fixup. Fixes: 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Link: https://lore.kernel.org/netdev/CAL_JsqLsVYiPLx2kcHkDQ4t=hQVCR7NHziDwi9cCFUFhx48Qow@mail.gmail.com/ Suggested-by: Rob Herring <robh@kernel.org> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/pci/bus.c | 4 +++- drivers/pci/of.c | 5 ----- 2 files changed, 3 insertions(+), 6 deletions(-)