netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status")
@ 2023-08-03 13:58 Vladimir Oltean
  2023-08-03 13:58 ` [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver Vladimir Oltean
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Vladimir Oltean @ 2023-08-03 13:58 UTC (permalink / raw)
  To: linux-pci, netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Bjorn Helgaas, Rob Herring, Claudiu Manoil, Michael Walle,
	linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou, Huacai Chen

I'm not sure who should take this patch set (net maintainers or PCI
maintainers). Everyone could pick up just their part, and that would
work (no compile time dependencies). However, the entire series needs
ACK from both sides and Rob for sure.

v1 at:
https://lore.kernel.org/netdev/20230521115141.2384444-1-vladimir.oltean@nxp.com/

Vladimir Oltean (3):
  PCI: move OF status = "disabled" detection to dev->match_driver
  net: enetc: reimplement RFS/RSS memory clearing as PCI quirk
  net: enetc: remove of_device_is_available() handling

 .../net/ethernet/freescale/enetc/enetc_pf.c   | 111 +++++++++++-------
 drivers/pci/bus.c                             |   4 +-
 drivers/pci/of.c                              |   5 -
 3 files changed, 74 insertions(+), 46 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver
  2023-08-03 13:58 [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Vladimir Oltean
@ 2023-08-03 13:58 ` Vladimir Oltean
  2023-08-08 22:21   ` Bjorn Helgaas
  2023-08-03 13:58 ` [PATCH v2 pci/net 2/3] net: enetc: reimplement RFS/RSS memory clearing as PCI quirk Vladimir Oltean
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Vladimir Oltean @ 2023-08-03 13:58 UTC (permalink / raw)
  To: linux-pci, netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Bjorn Helgaas, Rob Herring, Claudiu Manoil, Michael Walle,
	linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou, Huacai Chen

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(-)

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 pci/net 2/3] net: enetc: reimplement RFS/RSS memory clearing as PCI quirk
  2023-08-03 13:58 [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Vladimir Oltean
  2023-08-03 13:58 ` [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver Vladimir Oltean
@ 2023-08-03 13:58 ` Vladimir Oltean
  2023-08-03 13:58 ` [PATCH v2 pci/net 3/3] net: enetc: remove of_device_is_available() handling Vladimir Oltean
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Vladimir Oltean @ 2023-08-03 13:58 UTC (permalink / raw)
  To: linux-pci, netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Bjorn Helgaas, Rob Herring, Claudiu Manoil, Michael Walle,
	linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou, Huacai Chen

The workaround implemented in commit 3222b5b613db ("net: enetc:
initialize RFS/RSS memories for unused ports too") is no longer
effective after commit 6fffbc7ae137 ("PCI: Honor firmware's device
disabled status"). Thus, it has introduced a regression and we see AER
errors being reported again:

$ ip link set sw2p0 up && dhclient -i sw2p0 && ip addr show sw2p0
fsl_enetc 0000:00:00.2 eno2: configuring for fixed/internal link mode
fsl_enetc 0000:00:00.2 eno2: Link is Up - 2.5Gbps/Full - flow control rx/tx
mscc_felix 0000:00:00.5 swp2: configuring for fixed/sgmii link mode
mscc_felix 0000:00:00.5 swp2: Link is Up - 1Gbps/Full - flow control off
sja1105 spi2.2 sw2p0: configuring for phy/rgmii-id link mode
sja1105 spi2.2 sw2p0: Link is Up - 1Gbps/Full - flow control off
pcieport 0000:00:1f.0: AER: Multiple Corrected error received: 0000:00:00.0
pcieport 0000:00:1f.0: AER: can't find device of ID0000

Rob's suggestion is to reimplement the enetc driver workaround as a
PCI fixup, and to modify the PCI core to run the fixups for all PCI
functions. This change handles the first part.

We refactor the common code in enetc_psi_create() and enetc_psi_destroy(),
and use the PCI fixup only for those functions for which enetc_pf_probe()
won't get called. This avoids some work being done twice for the PFs
which are enabled.

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>
---
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 103 +++++++++++++-----
 1 file changed, 73 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 1416262d4296..d52ac0b6add6 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -1208,50 +1208,81 @@ static int enetc_pf_register_with_ierb(struct pci_dev *pdev)
 	return enetc_ierb_register_pf(ierb_pdev, pdev);
 }
 
-static int enetc_pf_probe(struct pci_dev *pdev,
-			  const struct pci_device_id *ent)
+static struct enetc_si *enetc_psi_create(struct pci_dev *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
-	struct enetc_ndev_priv *priv;
-	struct net_device *ndev;
 	struct enetc_si *si;
-	struct enetc_pf *pf;
 	int err;
 
-	err = enetc_pf_register_with_ierb(pdev);
-	if (err == -EPROBE_DEFER)
-		return err;
-	if (err)
-		dev_warn(&pdev->dev,
-			 "Could not register with IERB driver: %pe, please update the device tree\n",
-			 ERR_PTR(err));
-
-	err = enetc_pci_probe(pdev, KBUILD_MODNAME, sizeof(*pf));
-	if (err)
-		return dev_err_probe(&pdev->dev, err, "PCI probing failed\n");
+	err = enetc_pci_probe(pdev, KBUILD_MODNAME, sizeof(struct enetc_pf));
+	if (err) {
+		dev_err_probe(&pdev->dev, err, "PCI probing failed\n");
+		goto out;
+	}
 
 	si = pci_get_drvdata(pdev);
 	if (!si->hw.port || !si->hw.global) {
 		err = -ENODEV;
 		dev_err(&pdev->dev, "could not map PF space, probing a VF?\n");
-		goto err_map_pf_space;
+		goto out_pci_remove;
 	}
 
 	err = enetc_setup_cbdr(&pdev->dev, &si->hw, ENETC_CBDR_DEFAULT_SIZE,
 			       &si->cbd_ring);
 	if (err)
-		goto err_setup_cbdr;
+		goto out_pci_remove;
 
 	err = enetc_init_port_rfs_memory(si);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to initialize RFS memory\n");
-		goto err_init_port_rfs;
+		goto out_teardown_cbdr;
 	}
 
 	err = enetc_init_port_rss_memory(si);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to initialize RSS memory\n");
-		goto err_init_port_rss;
+		goto out_teardown_cbdr;
+	}
+
+	return si;
+
+out_teardown_cbdr:
+	enetc_teardown_cbdr(&si->cbd_ring);
+out_pci_remove:
+	enetc_pci_remove(pdev);
+out:
+	return ERR_PTR(err);
+}
+
+static void enetc_psi_destroy(struct pci_dev *pdev)
+{
+	struct enetc_si *si = pci_get_drvdata(pdev);
+
+	enetc_teardown_cbdr(&si->cbd_ring);
+	enetc_pci_remove(pdev);
+}
+
+static int enetc_pf_probe(struct pci_dev *pdev,
+			  const struct pci_device_id *ent)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct enetc_ndev_priv *priv;
+	struct net_device *ndev;
+	struct enetc_si *si;
+	struct enetc_pf *pf;
+	int err;
+
+	err = enetc_pf_register_with_ierb(pdev);
+	if (err == -EPROBE_DEFER)
+		return err;
+	if (err)
+		dev_warn(&pdev->dev,
+			 "Could not register with IERB driver: %pe, please update the device tree\n",
+			 ERR_PTR(err));
+
+	si = enetc_psi_create(pdev);
+	if (IS_ERR(si)) {
+		err = PTR_ERR(si);
+		goto err_psi_create;
 	}
 
 	if (node && !of_device_is_available(node)) {
@@ -1339,15 +1370,10 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	si->ndev = NULL;
 	free_netdev(ndev);
 err_alloc_netdev:
-err_init_port_rss:
-err_init_port_rfs:
 err_device_disabled:
 err_setup_mac_addresses:
-	enetc_teardown_cbdr(&si->cbd_ring);
-err_setup_cbdr:
-err_map_pf_space:
-	enetc_pci_remove(pdev);
-
+	enetc_psi_destroy(pdev);
+err_psi_create:
 	return err;
 }
 
@@ -1370,12 +1396,29 @@ static void enetc_pf_remove(struct pci_dev *pdev)
 	enetc_free_msix(priv);
 
 	enetc_free_si_resources(priv);
-	enetc_teardown_cbdr(&si->cbd_ring);
 
 	free_netdev(si->ndev);
 
-	enetc_pci_remove(pdev);
+	enetc_psi_destroy(pdev);
+}
+
+static void enetc_fixup_clear_rss_rfs(struct pci_dev *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct enetc_si *si;
+
+	/* Only apply quirk for disabled functions. For the ones
+	 * that are enabled, enetc_pf_probe() will apply it.
+	 */
+	if (node && of_device_is_available(node))
+		return;
+
+	si = enetc_psi_create(pdev);
+	if (si)
+		enetc_psi_destroy(pdev);
 }
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PF,
+			enetc_fixup_clear_rss_rfs);
 
 static const struct pci_device_id enetc_pf_id_table[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, ENETC_DEV_ID_PF) },
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 pci/net 3/3] net: enetc: remove of_device_is_available() handling
  2023-08-03 13:58 [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Vladimir Oltean
  2023-08-03 13:58 ` [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver Vladimir Oltean
  2023-08-03 13:58 ` [PATCH v2 pci/net 2/3] net: enetc: reimplement RFS/RSS memory clearing as PCI quirk Vladimir Oltean
@ 2023-08-03 13:58 ` Vladimir Oltean
  2023-08-04 14:12 ` [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Simon Horman
  2023-08-09  8:30 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 9+ messages in thread
From: Vladimir Oltean @ 2023-08-03 13:58 UTC (permalink / raw)
  To: linux-pci, netdev
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Bjorn Helgaas, Rob Herring, Claudiu Manoil, Michael Walle,
	linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou, Huacai Chen

Since commit 6fffbc7ae137 ("PCI: Honor firmware's device disabled
status"), this is redundant and does nothing, because enetc_pf_probe()
no longer even gets called.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index d52ac0b6add6..e0a4cb7e3f50 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -1186,14 +1186,9 @@ static int enetc_init_port_rss_memory(struct enetc_si *si)
 
 static int enetc_pf_register_with_ierb(struct pci_dev *pdev)
 {
-	struct device_node *node = pdev->dev.of_node;
 	struct platform_device *ierb_pdev;
 	struct device_node *ierb_node;
 
-	/* Don't register with the IERB if the PF itself is disabled */
-	if (!node || !of_device_is_available(node))
-		return 0;
-
 	ierb_node = of_find_compatible_node(NULL, NULL,
 					    "fsl,ls1028a-enetc-ierb");
 	if (!ierb_node || !of_device_is_available(ierb_node))
@@ -1285,12 +1280,6 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 		goto err_psi_create;
 	}
 
-	if (node && !of_device_is_available(node)) {
-		dev_info(&pdev->dev, "device is disabled, skipping\n");
-		err = -ENODEV;
-		goto err_device_disabled;
-	}
-
 	pf = enetc_si_priv(si);
 	pf->si = si;
 	pf->total_vfs = pci_sriov_get_totalvfs(pdev);
@@ -1370,7 +1359,6 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 	si->ndev = NULL;
 	free_netdev(ndev);
 err_alloc_netdev:
-err_device_disabled:
 err_setup_mac_addresses:
 	enetc_psi_destroy(pdev);
 err_psi_create:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status")
  2023-08-03 13:58 [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Vladimir Oltean
                   ` (2 preceding siblings ...)
  2023-08-03 13:58 ` [PATCH v2 pci/net 3/3] net: enetc: remove of_device_is_available() handling Vladimir Oltean
@ 2023-08-04 14:12 ` Simon Horman
  2023-08-09  8:30 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2023-08-04 14:12 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-pci, netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Bjorn Helgaas, Rob Herring, Claudiu Manoil,
	Michael Walle, linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou,
	Huacai Chen

On Thu, Aug 03, 2023 at 04:58:55PM +0300, Vladimir Oltean wrote:
> I'm not sure who should take this patch set (net maintainers or PCI
> maintainers). Everyone could pick up just their part, and that would
> work (no compile time dependencies). However, the entire series needs
> ACK from both sides and Rob for sure.
> 
> v1 at:
> https://lore.kernel.org/netdev/20230521115141.2384444-1-vladimir.oltean@nxp.com/
> 
> Vladimir Oltean (3):
>   PCI: move OF status = "disabled" detection to dev->match_driver
>   net: enetc: reimplement RFS/RSS memory clearing as PCI quirk
>   net: enetc: remove of_device_is_available() handling

FWIIW, this looks good to me.

For the series,

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver
  2023-08-03 13:58 ` [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver Vladimir Oltean
@ 2023-08-08 22:21   ` Bjorn Helgaas
  2023-08-09 14:45     ` Vladimir Oltean
  0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2023-08-08 22:21 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-pci, netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Bjorn Helgaas, Rob Herring, Claudiu Manoil,
	Michael Walle, linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou,
	Huacai Chen

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status")
  2023-08-03 13:58 [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Vladimir Oltean
                   ` (3 preceding siblings ...)
  2023-08-04 14:12 ` [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Simon Horman
@ 2023-08-09  8:30 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-08-09  8:30 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-pci, netdev, davem, edumazet, kuba, pabeni, bhelgaas, robh,
	claudiu.manoil, michael, linux-kernel, lvjianmin, liupeibao,
	zhoubinbin, chenhuacai

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu,  3 Aug 2023 16:58:55 +0300 you wrote:
> I'm not sure who should take this patch set (net maintainers or PCI
> maintainers). Everyone could pick up just their part, and that would
> work (no compile time dependencies). However, the entire series needs
> ACK from both sides and Rob for sure.
> 
> v1 at:
> https://lore.kernel.org/netdev/20230521115141.2384444-1-vladimir.oltean@nxp.com/
> 
> [...]

Here is the summary with links:
  - [v2,pci/net,1/3] PCI: move OF status = "disabled" detection to dev->match_driver
    https://git.kernel.org/netdev/net/c/1a8c251cff20
  - [v2,pci/net,2/3] net: enetc: reimplement RFS/RSS memory clearing as PCI quirk
    https://git.kernel.org/netdev/net/c/f0168042a212
  - [v2,pci/net,3/3] net: enetc: remove of_device_is_available() handling
    https://git.kernel.org/netdev/net/c/bfce089ddd0e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver
  2023-08-08 22:21   ` Bjorn Helgaas
@ 2023-08-09 14:45     ` Vladimir Oltean
  2023-08-09 15:37       ` Bjorn Helgaas
  0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Oltean @ 2023-08-09 14:45 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Bjorn Helgaas, Rob Herring, Claudiu Manoil,
	Michael Walle, linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou,
	Huacai Chen

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? :)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver
  2023-08-09 14:45     ` Vladimir Oltean
@ 2023-08-09 15:37       ` Bjorn Helgaas
  0 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2023-08-09 15:37 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: linux-pci, netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Bjorn Helgaas, Rob Herring, Claudiu Manoil,
	Michael Walle, linux-kernel, Jianmin Lv, Liu Peibao, Binbin Zhou,
	Huacai Chen

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-08-09 15:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-03 13:58 [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Vladimir Oltean
2023-08-03 13:58 ` [PATCH v2 pci/net 1/3] PCI: move OF status = "disabled" detection to dev->match_driver Vladimir Oltean
2023-08-08 22:21   ` Bjorn Helgaas
2023-08-09 14:45     ` Vladimir Oltean
2023-08-09 15:37       ` Bjorn Helgaas
2023-08-03 13:58 ` [PATCH v2 pci/net 2/3] net: enetc: reimplement RFS/RSS memory clearing as PCI quirk Vladimir Oltean
2023-08-03 13:58 ` [PATCH v2 pci/net 3/3] net: enetc: remove of_device_is_available() handling Vladimir Oltean
2023-08-04 14:12 ` [PATCH v2 pci/net 0/3] Fix ENETC probing after 6fffbc7ae137 ("PCI: Honor firmware's device disabled status") Simon Horman
2023-08-09  8:30 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).