* [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-04 12:59 ` Uwe Kleine-König
0 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, linux-pci, Alexander Duyck, x86, qat-linux,
oss-drivers, Oliver O'Halloran, H. Peter Anvin, Jiri Olsa,
Thomas Gleixner, Marco Chiappero, Stefano Stabellini, Herbert Xu,
linux-scsi, Rafał Miłecki, Jesse Brandeburg,
Peter Zijlstra, Ingo Molnar, linux-wireless, Jakub Kicinski,
Yisen Zhuang, Suganath Prabu Subramani, Fiona Trahe,
Andrew Donnellan, Arnd Bergmann, Konrad Rzeszutek Wilk,
Ido Schimmel, Simon Horman, linuxppc-dev,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Andy Shevchenko, Juergen Gross, Salil Mehta,
Sreekanth Reddy, xen-devel, Vadym Kochan, MPT-FusionLinux.pdl,
Greg Kroah-Hartman, linux-usb, Wojciech Ziemba, linux-kernel,
Mathias Nyman, Zhou Wang, linux-crypto, kernel, netdev,
Frederic Barrat, Paul Mackerras, Tomaszx Kowalik, Taras Chornyi,
David S. Miller, linux-perf-users
Hello,
this is v6 of the quest to drop the "driver" member from struct pci_dev
which tracks the same data (apart from a constant offset) as dev.driver.
Changes since v5:
- Some Acks added
- Some fixes in "PCI: Replace pci_dev::driver usage by
pci_dev::dev.driver" to properly handle that
to_pci_driver(X) is wrong if X is NULL.
This should fix the problem reported by Ido Schimmel.
Full range diff below.
This patch stack survived an allmodconfig build on arm64, m68k, powerpc,
riscv, s390, sparc64 and x86_64 on top of v5.15-rc3.
Best regards
Uwe
Uwe Kleine-König (11):
PCI: Simplify pci_device_remove()
PCI: Drop useless check from pci_device_probe()
xen/pci: Drop some checks that are always true
bcma: simplify reference to the driver's name
powerpc/eeh: Don't use driver member of struct pci_dev and further
cleanups
ssb: Simplify determination of driver name
PCI: Replace pci_dev::driver usage that gets the driver name
scsi: message: fusion: Remove unused parameter of mpt_pci driver's
probe()
crypto: qat - simplify adf_enable_aer()
PCI: Replace pci_dev::driver usage by pci_dev::dev.driver
PCI: Drop duplicated tracking of a pci_dev's bound driver
arch/powerpc/include/asm/ppc-pci.h | 5 -
arch/powerpc/kernel/eeh.c | 8 ++
arch/powerpc/kernel/eeh_driver.c | 10 +-
arch/x86/events/intel/uncore.c | 2 +-
arch/x86/kernel/probe_roms.c | 10 +-
drivers/bcma/host_pci.c | 6 +-
drivers/crypto/hisilicon/qm.c | 2 +-
drivers/crypto/qat/qat_4xxx/adf_drv.c | 7 +-
drivers/crypto/qat/qat_c3xxx/adf_drv.c | 7 +-
drivers/crypto/qat/qat_c62x/adf_drv.c | 7 +-
drivers/crypto/qat/qat_common/adf_aer.c | 10 +-
.../crypto/qat/qat_common/adf_common_drv.h | 3 +-
drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 +-
drivers/message/fusion/mptbase.c | 7 +-
drivers/message/fusion/mptbase.h | 2 +-
drivers/message/fusion/mptctl.c | 4 +-
drivers/message/fusion/mptlan.c | 2 +-
drivers/misc/cxl/guest.c | 24 +++--
drivers/misc/cxl/pci.c | 30 +++---
.../ethernet/hisilicon/hns3/hns3_ethtool.c | 2 +-
.../ethernet/marvell/prestera/prestera_pci.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/pci.c | 2 +-
.../ethernet/netronome/nfp/nfp_net_ethtool.c | 3 +-
drivers/pci/iov.c | 33 +++++--
drivers/pci/pci-driver.c | 96 ++++++++++---------
drivers/pci/pci.c | 4 +-
drivers/pci/pcie/err.c | 36 +++----
drivers/pci/xen-pcifront.c | 63 ++++++------
drivers/ssb/pcihost_wrapper.c | 6 +-
drivers/usb/host/xhci-pci.c | 2 +-
include/linux/pci.h | 1 -
31 files changed, 208 insertions(+), 195 deletions(-)
Range-diff against v5:
-: ------------ > 1: c2b53ab26a6b PCI: Simplify pci_device_remove()
-: ------------ > 2: 2c733e1d5186 PCI: Drop useless check from pci_device_probe()
-: ------------ > 3: 547ca5a7aa16 xen/pci: Drop some checks that are always true
-: ------------ > 4: 40eb07353844 bcma: simplify reference to the driver's name
-: ------------ > 5: bab59c1dff6d powerpc/eeh: Don't use driver member of struct pci_dev and further cleanups
1: abd70de9782d ! 6: 92f4d61bbac3 ssb: Simplify determination of driver name
@@ Commit message
This has the upside of not requiring the driver member of struct pci_dev
which is about to be removed and being simpler.
+ Acked-by: Michael Büsch <m@bues.ch>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
## drivers/ssb/pcihost_wrapper.c ##
2: 735845bd26b9 ! 7: 6303f03ab2aa PCI: Replace pci_dev::driver usage that gets the driver name
@@ Commit message
driver name by dev_driver_string() which implicitly makes use of struct
pci_dev::dev->driver.
+ Acked-by: Simon Horman <simon.horman@corigine.com> (for NFP)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
## drivers/crypto/hisilicon/qm.c ##
3: 1e58019165b9 = 8: 658a6c00ec96 scsi: message: fusion: Remove unused parameter of mpt_pci driver's probe()
4: dea72a470141 = 9: aceaf5321603 crypto: qat - simplify adf_enable_aer()
5: b4165dda38ea ! 10: 80648d999985 PCI: Replace pci_dev::driver usage by pci_dev::dev.driver
@@ arch/x86/kernel/probe_roms.c: static struct resource video_rom_resource = {
static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
{
- struct pci_driver *drv = pdev->driver;
-+ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
const struct pci_device_id *id;
if (pdev->vendor == vendor && pdev->device == device)
+ return true;
+
+- for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
+- if (id->vendor == vendor && id->device == device)
+- break;
++ if (pdev->dev.driver) {
++ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
++ for (id = drv->id_table; id && id->vendor; id++)
++ if (id->vendor == vendor && id->device == device)
++ break;
++ }
+
+ return id && id->vendor;
+ }
## drivers/misc/cxl/guest.c ##
@@ drivers/misc/cxl/guest.c: static void pci_error_handlers(struct cxl_afu *afu,
@@ drivers/pci/iov.c: static ssize_t sriov_vf_total_msix_show(struct device *dev,
device_lock(dev);
- if (!pdev->driver || !pdev->driver->sriov_get_vf_total_msix)
-+ pdrv = to_pci_driver(dev->driver);
-+ if (!pdrv || !pdrv->sriov_get_vf_total_msix)
++ if (!dev->driver)
goto unlock;
- vf_total_msix = pdev->driver->sriov_get_vf_total_msix(pdev);
++ pdrv = to_pci_driver(dev->driver);
++ if (!pdrv->sriov_get_vf_total_msix)
++ goto unlock;
++
+ vf_total_msix = pdrv->sriov_get_vf_total_msix(pdev);
unlock:
device_unlock(dev);
@@ drivers/pci/iov.c: static ssize_t sriov_vf_msix_count_store(struct device *dev,
device_lock(&pdev->dev);
- if (!pdev->driver || !pdev->driver->sriov_set_msix_vec_count) {
++ if (!pdev->dev.driver) {
++ ret = -EOPNOTSUPP;
++ goto err_pdev;
++ }
++
+ pdrv = to_pci_driver(pdev->dev.driver);
-+ if (!pdrv || !pdrv->sriov_set_msix_vec_count) {
++ if (!pdrv->sriov_set_msix_vec_count) {
ret = -EOPNOTSUPP;
goto err_pdev;
}
@@ drivers/pci/pci-driver.c: static void pci_device_remove(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
-+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
pm_runtime_resume(dev);
+- if (drv && drv->shutdown)
+- drv->shutdown(pci_dev);
++ if (pci_dev->dev.driver) {
++ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
++
++ if (drv->shutdown)
++ drv->shutdown(pci_dev);
++ }
+
+ /*
+ * If this is a kexec reboot, turn off Bus Master bit on the
@@ drivers/pci/pci-driver.c: static int pci_pm_reenable_device(struct pci_dev *pci_dev)
static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
-+ struct pci_driver *drv = to_pci_driver(dev->driver);
- if (drv && drv->suspend) {
- pci_power_t prev = pci_dev->current_state;
+- if (drv && drv->suspend) {
+- pci_power_t prev = pci_dev->current_state;
+- int error;
++ if (dev->driver) {
++ struct pci_driver *drv = to_pci_driver(dev->driver);
+
+- error = drv->suspend(pci_dev, state);
+- suspend_report_result(drv->suspend, error);
+- if (error)
+- return error;
++ if (drv->suspend) {
++ pci_power_t prev = pci_dev->current_state;
++ int error;
+
+- if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
+- && pci_dev->current_state != PCI_UNKNOWN) {
+- pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
+- "PCI PM: Device state not saved by %pS\n",
+- drv->suspend);
++ error = drv->suspend(pci_dev, state);
++ suspend_report_result(drv->suspend, error);
++ if (error)
++ return error;
++
++ if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
++ && pci_dev->current_state != PCI_UNKNOWN) {
++ pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
++ "PCI PM: Device state not saved by %pS\n",
++ drv->suspend);
++ }
+ }
+ }
+
@@ drivers/pci/pci-driver.c: static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
-+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
pci_fixup_device(pci_fixup_resume, pci_dev);
+- return drv && drv->resume ?
+- drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
++ if (pci_dev->dev.driver) {
++ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
++
++ if (drv->resume)
++ return drv->resume(pci_dev);
++ }
++
++ return pci_pm_reenable_device(pci_dev);
+ }
+
+ /* Auxiliary functions used by the new power management framework */
@@ drivers/pci/pci-driver.c: static void pci_pm_default_suspend(struct pci_dev *pci_dev)
static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
{
- struct pci_driver *drv = pci_dev->driver;
-+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
- bool ret = drv && (drv->suspend || drv->resume);
+- bool ret = drv && (drv->suspend || drv->resume);
++ struct pci_driver *drv;
++ bool ret;
++
++ if (!pci_dev->dev.driver)
++ return false;
++
++ drv = to_pci_driver(pci_dev->dev.driver);
++ ret = drv && (drv->suspend || drv->resume);
/*
+ * Legacy PM support is used by default, so warn if the new framework is
@@ drivers/pci/pci-driver.c: static int pci_pm_runtime_suspend(struct device *dev)
int error;
6: d93a138bd7ab = 11: 2686d69bca17 PCI: Drop duplicated tracking of a pci_dev's bound driver
base-commit: 5816b3e6577eaa676ceb00a848f0fd65fe2adc29
--
2.30.2
^ permalink raw reply [flat|nested] 36+ messages in thread* [PATCH v6 01/11] PCI: Simplify pci_device_remove()
2021-10-04 12:59 ` Uwe Kleine-König
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, kernel, Christoph Hellwig
When the driver core calls pci_device_remove() there is a driver that bound
the device and so pci_dev->driver is never NULL.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/pci/pci-driver.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 2761ab86490d..8fb6418c93e8 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -459,16 +459,14 @@ static void pci_device_remove(struct device *dev)
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = pci_dev->driver;
- if (drv) {
- if (drv->remove) {
- pm_runtime_get_sync(dev);
- drv->remove(pci_dev);
- pm_runtime_put_noidle(dev);
- }
- pcibios_free_irq(pci_dev);
- pci_dev->driver = NULL;
- pci_iov_remove(pci_dev);
+ if (drv->remove) {
+ pm_runtime_get_sync(dev);
+ drv->remove(pci_dev);
+ pm_runtime_put_noidle(dev);
}
+ pcibios_free_irq(pci_dev);
+ pci_dev->driver = NULL;
+ pci_iov_remove(pci_dev);
/* Undo the runtime PM settings in local_pci_probe() */
pm_runtime_put_sync(dev);
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 02/11] PCI: Drop useless check from pci_device_probe()
2021-10-04 12:59 ` Uwe Kleine-König
(?)
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, kernel, Christoph Hellwig
When the device core calls the probe callback for a device the device is
never bound and so !pci_dev->driver is always true.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/pci/pci-driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 8fb6418c93e8..50449ec622a3 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -397,7 +397,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
const struct pci_device_id *id;
int error = 0;
- if (!pci_dev->driver && drv->probe) {
+ if (drv->probe) {
error = -ENODEV;
id = pci_match_device(drv, pci_dev);
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 03/11] xen/pci: Drop some checks that are always true
2021-10-04 12:59 ` Uwe Kleine-König
` (2 preceding siblings ...)
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas, Boris Ostrovsky, Juergen Gross
Cc: linux-pci, kernel, Stefano Stabellini, Konrad Rzeszutek Wilk,
xen-devel, Christoph Hellwig
pcifront_common_process() has a check at the start that exits early if
pcidev or pdidev->driver are NULL. So simplify the following code by not
checking these two again.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/pci/xen-pcifront.c | 57 +++++++++++++++++---------------------
1 file changed, 25 insertions(+), 32 deletions(-)
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 2156c632524d..f2d7f70a7a10 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -594,7 +594,6 @@ static pci_ers_result_t pcifront_common_process(int cmd,
int devfn = pdev->sh_info->aer_op.devfn;
int domain = pdev->sh_info->aer_op.domain;
struct pci_dev *pcidev;
- int flag = 0;
dev_dbg(&pdev->xdev->dev,
"pcifront AER process: cmd %x (bus:%x, devfn%x)",
@@ -609,40 +608,34 @@ static pci_ers_result_t pcifront_common_process(int cmd,
}
pdrv = pcidev->driver;
- if (pdrv) {
- if (pdrv->err_handler && pdrv->err_handler->error_detected) {
- pci_dbg(pcidev, "trying to call AER service\n");
- if (pcidev) {
- flag = 1;
- switch (cmd) {
- case XEN_PCI_OP_aer_detected:
- result = pdrv->err_handler->
- error_detected(pcidev, state);
- break;
- case XEN_PCI_OP_aer_mmio:
- result = pdrv->err_handler->
- mmio_enabled(pcidev);
- break;
- case XEN_PCI_OP_aer_slotreset:
- result = pdrv->err_handler->
- slot_reset(pcidev);
- break;
- case XEN_PCI_OP_aer_resume:
- pdrv->err_handler->resume(pcidev);
- break;
- default:
- dev_err(&pdev->xdev->dev,
- "bad request in aer recovery "
- "operation!\n");
-
- }
- }
+ if (pdrv->err_handler && pdrv->err_handler->error_detected) {
+ pci_dbg(pcidev, "trying to call AER service\n");
+ switch (cmd) {
+ case XEN_PCI_OP_aer_detected:
+ result = pdrv->err_handler->
+ error_detected(pcidev, state);
+ break;
+ case XEN_PCI_OP_aer_mmio:
+ result = pdrv->err_handler->
+ mmio_enabled(pcidev);
+ break;
+ case XEN_PCI_OP_aer_slotreset:
+ result = pdrv->err_handler->
+ slot_reset(pcidev);
+ break;
+ case XEN_PCI_OP_aer_resume:
+ pdrv->err_handler->resume(pcidev);
+ break;
+ default:
+ dev_err(&pdev->xdev->dev,
+ "bad request in aer recovery "
+ "operation!\n");
}
+
+ return result;
}
- if (!flag)
- result = PCI_ERS_RESULT_NONE;
- return result;
+ return PCI_ERS_RESULT_NONE;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 04/11] bcma: simplify reference to the driver's name
2021-10-04 12:59 ` Uwe Kleine-König
` (3 preceding siblings ...)
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas, Rafał Miłecki; +Cc: linux-pci, kernel, linux-wireless
When bcma_host_pci_probe() is called the pci driver core already
assigned the device's driver in local_pci_probe(). So dev->driver is
always true and here it points to bcma_pci_bridge_driver which has .name
set to "bcma-pci-bridge". Simplify accordingly.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/bcma/host_pci.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/bcma/host_pci.c b/drivers/bcma/host_pci.c
index 69c10a7b7c61..960632197b05 100644
--- a/drivers/bcma/host_pci.c
+++ b/drivers/bcma/host_pci.c
@@ -162,7 +162,6 @@ static int bcma_host_pci_probe(struct pci_dev *dev,
{
struct bcma_bus *bus;
int err = -ENOMEM;
- const char *name;
u32 val;
/* Alloc */
@@ -175,10 +174,7 @@ static int bcma_host_pci_probe(struct pci_dev *dev,
if (err)
goto err_kfree_bus;
- name = dev_name(&dev->dev);
- if (dev->driver && dev->driver->name)
- name = dev->driver->name;
- err = pci_request_regions(dev, name);
+ err = pci_request_regions(dev, "bcma-pci-bridge");
if (err)
goto err_pci_disable;
pci_set_master(dev);
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 05/11] powerpc/eeh: Don't use driver member of struct pci_dev and further cleanups
2021-10-04 12:59 ` Uwe Kleine-König
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas, Michael Ellerman
Cc: linux-pci, Oliver O'Halloran, kernel, Paul Mackerras, linuxppc-dev
The driver member of struct pci_dev is to be removed as it tracks
information already present by tracking of the driver core. So replace
pdev->driver->name by dev_driver_string() for the corresponding struct
device.
Also move the function nearer to its only user and instead of the ?:
operator use a normal if which is more readable.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/powerpc/include/asm/ppc-pci.h | 5 -----
arch/powerpc/kernel/eeh.c | 8 ++++++++
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index 2b9edbf6e929..f6cf0159024e 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -55,11 +55,6 @@ void eeh_pe_dev_mode_mark(struct eeh_pe *pe, int mode);
void eeh_sysfs_add_device(struct pci_dev *pdev);
void eeh_sysfs_remove_device(struct pci_dev *pdev);
-static inline const char *eeh_driver_name(struct pci_dev *pdev)
-{
- return (pdev && pdev->driver) ? pdev->driver->name : "<null>";
-}
-
#endif /* CONFIG_EEH */
#define PCI_BUSNO(bdfn) ((bdfn >> 8) & 0xff)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index e9b597ed423c..4b08881c4a1e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -399,6 +399,14 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
return ret;
}
+static inline const char *eeh_driver_name(struct pci_dev *pdev)
+{
+ if (pdev)
+ return dev_driver_string(&pdev->dev);
+
+ return "<null>";
+}
+
/**
* eeh_dev_check_failure - Check if all 1's data is due to EEH slot freeze
* @edev: eeh device
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 05/11] powerpc/eeh: Don't use driver member of struct pci_dev and further cleanups
@ 2021-10-04 12:59 ` Uwe Kleine-König
0 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas, Michael Ellerman
Cc: linux-pci, kernel, Benjamin Herrenschmidt, Paul Mackerras,
Russell Currey, Oliver O'Halloran, linuxppc-dev
The driver member of struct pci_dev is to be removed as it tracks
information already present by tracking of the driver core. So replace
pdev->driver->name by dev_driver_string() for the corresponding struct
device.
Also move the function nearer to its only user and instead of the ?:
operator use a normal if which is more readable.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/powerpc/include/asm/ppc-pci.h | 5 -----
arch/powerpc/kernel/eeh.c | 8 ++++++++
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index 2b9edbf6e929..f6cf0159024e 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -55,11 +55,6 @@ void eeh_pe_dev_mode_mark(struct eeh_pe *pe, int mode);
void eeh_sysfs_add_device(struct pci_dev *pdev);
void eeh_sysfs_remove_device(struct pci_dev *pdev);
-static inline const char *eeh_driver_name(struct pci_dev *pdev)
-{
- return (pdev && pdev->driver) ? pdev->driver->name : "<null>";
-}
-
#endif /* CONFIG_EEH */
#define PCI_BUSNO(bdfn) ((bdfn >> 8) & 0xff)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index e9b597ed423c..4b08881c4a1e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -399,6 +399,14 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
return ret;
}
+static inline const char *eeh_driver_name(struct pci_dev *pdev)
+{
+ if (pdev)
+ return dev_driver_string(&pdev->dev);
+
+ return "<null>";
+}
+
/**
* eeh_dev_check_failure - Check if all 1's data is due to EEH slot freeze
* @edev: eeh device
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v6 06/11] ssb: Simplify determination of driver name
2021-10-04 12:59 ` Uwe Kleine-König
` (5 preceding siblings ...)
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas, Michael Büsch; +Cc: linux-pci, kernel, linux-wireless
For all drivers that make use of ssb_pcihost_probe() (i.e.
b43_pci_bridge_driver and b44_pci_driver) the driver name is set.
As at the time for the function is called __pci_register_driver() already
assigned drv->driver.name to hold the same value, use
dev_driver_string() with the same result.
This has the upside of not requiring the driver member of struct pci_dev
which is about to be removed and being simpler.
Acked-by: Michael Büsch <m@bues.ch>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/ssb/pcihost_wrapper.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/ssb/pcihost_wrapper.c b/drivers/ssb/pcihost_wrapper.c
index 410215c16920..dd70fd41c77d 100644
--- a/drivers/ssb/pcihost_wrapper.c
+++ b/drivers/ssb/pcihost_wrapper.c
@@ -69,7 +69,6 @@ static int ssb_pcihost_probe(struct pci_dev *dev,
{
struct ssb_bus *ssb;
int err = -ENOMEM;
- const char *name;
u32 val;
ssb = kzalloc(sizeof(*ssb), GFP_KERNEL);
@@ -78,10 +77,7 @@ static int ssb_pcihost_probe(struct pci_dev *dev,
err = pci_enable_device(dev);
if (err)
goto err_kfree_ssb;
- name = dev_name(&dev->dev);
- if (dev->driver && dev->driver->name)
- name = dev->driver->name;
- err = pci_request_regions(dev, name);
+ err = pci_request_regions(dev, dev_driver_string(&dev->dev));
if (err)
goto err_pci_disable;
pci_set_master(dev);
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 07/11] PCI: Replace pci_dev::driver usage that gets the driver name
2021-10-04 12:59 ` Uwe Kleine-König
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, Alexander Duyck, oss-drivers, Paul Mackerras,
Herbert Xu, Rafał Miłecki, Jesse Brandeburg,
Ido Schimmel, Jakub Kicinski, Yisen Zhuang, Vadym Kochan,
Michael Buesch, Jiri Pirko, Salil Mehta, netdev, linux-wireless,
linux-kernel, Taras Chornyi, Zhou Wang, linux-crypto, kernel,
Simon Horman, Oliver O'Halloran, linuxppc-dev,
David S. Miller
struct pci_dev::driver holds (apart from a constant offset) the same
data as struct pci_dev::dev->driver. With the goal to remove struct
pci_dev::driver to get rid of data duplication replace getting the
driver name by dev_driver_string() which implicitly makes use of struct
pci_dev::dev->driver.
Acked-by: Simon Horman <simon.horman@corigine.com> (for NFP)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/crypto/hisilicon/qm.c | 2 +-
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 2 +-
drivers/net/ethernet/marvell/prestera/prestera_pci.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/pci.c | 2 +-
drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c | 3 ++-
5 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
index 369562d34d66..8f361e54e524 100644
--- a/drivers/crypto/hisilicon/qm.c
+++ b/drivers/crypto/hisilicon/qm.c
@@ -3085,7 +3085,7 @@ static int qm_alloc_uacce(struct hisi_qm *qm)
};
int ret;
- ret = strscpy(interface.name, pdev->driver->name,
+ ret = strscpy(interface.name, dev_driver_string(&pdev->dev),
sizeof(interface.name));
if (ret < 0)
return -ENAMETOOLONG;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 7ea511d59e91..f279edfce3f1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -606,7 +606,7 @@ static void hns3_get_drvinfo(struct net_device *netdev,
return;
}
- strncpy(drvinfo->driver, h->pdev->driver->name,
+ strncpy(drvinfo->driver, dev_driver_string(&h->pdev->dev),
sizeof(drvinfo->driver));
drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0';
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index a250d394da38..a8f007f6dad2 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -720,7 +720,7 @@ static int prestera_fw_load(struct prestera_fw *fw)
static int prestera_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
- const char *driver_name = pdev->driver->name;
+ const char *driver_name = dev_driver_string(&pdev->dev);
struct prestera_fw *fw;
int err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 13b0259f7ea6..8f306364f7bf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1876,7 +1876,7 @@ static void mlxsw_pci_cmd_fini(struct mlxsw_pci *mlxsw_pci)
static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
- const char *driver_name = pdev->driver->name;
+ const char *driver_name = dev_driver_string(&pdev->dev);
struct mlxsw_pci *mlxsw_pci;
int err;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 0685ece1f155..1de076f55740 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -202,7 +202,8 @@ nfp_get_drvinfo(struct nfp_app *app, struct pci_dev *pdev,
{
char nsp_version[ETHTOOL_FWVERS_LEN] = {};
- strlcpy(drvinfo->driver, pdev->driver->name, sizeof(drvinfo->driver));
+ strlcpy(drvinfo->driver, dev_driver_string(&pdev->dev),
+ sizeof(drvinfo->driver));
nfp_net_get_nspinfo(app, nsp_version);
snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
"%s %s %s %s", vnic_version, nsp_version,
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 07/11] PCI: Replace pci_dev::driver usage that gets the driver name
@ 2021-10-04 12:59 ` Uwe Kleine-König
0 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, kernel, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Rafał Miłecki, Zhou Wang, Herbert Xu,
David S. Miller, Yisen Zhuang, Salil Mehta, Jakub Kicinski,
Vadym Kochan, Taras Chornyi, Jiri Pirko, Ido Schimmel,
Simon Horman, Michael Buesch, Oliver O'Halloran,
Jesse Brandeburg, Alexander Duyck, linuxppc-dev, linux-kernel,
linux-wireless, linux-crypto, netdev, oss-drivers
struct pci_dev::driver holds (apart from a constant offset) the same
data as struct pci_dev::dev->driver. With the goal to remove struct
pci_dev::driver to get rid of data duplication replace getting the
driver name by dev_driver_string() which implicitly makes use of struct
pci_dev::dev->driver.
Acked-by: Simon Horman <simon.horman@corigine.com> (for NFP)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/crypto/hisilicon/qm.c | 2 +-
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 2 +-
drivers/net/ethernet/marvell/prestera/prestera_pci.c | 2 +-
drivers/net/ethernet/mellanox/mlxsw/pci.c | 2 +-
drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c | 3 ++-
5 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
index 369562d34d66..8f361e54e524 100644
--- a/drivers/crypto/hisilicon/qm.c
+++ b/drivers/crypto/hisilicon/qm.c
@@ -3085,7 +3085,7 @@ static int qm_alloc_uacce(struct hisi_qm *qm)
};
int ret;
- ret = strscpy(interface.name, pdev->driver->name,
+ ret = strscpy(interface.name, dev_driver_string(&pdev->dev),
sizeof(interface.name));
if (ret < 0)
return -ENAMETOOLONG;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 7ea511d59e91..f279edfce3f1 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -606,7 +606,7 @@ static void hns3_get_drvinfo(struct net_device *netdev,
return;
}
- strncpy(drvinfo->driver, h->pdev->driver->name,
+ strncpy(drvinfo->driver, dev_driver_string(&h->pdev->dev),
sizeof(drvinfo->driver));
drvinfo->driver[sizeof(drvinfo->driver) - 1] = '\0';
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_pci.c b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
index a250d394da38..a8f007f6dad2 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_pci.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_pci.c
@@ -720,7 +720,7 @@ static int prestera_fw_load(struct prestera_fw *fw)
static int prestera_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
{
- const char *driver_name = pdev->driver->name;
+ const char *driver_name = dev_driver_string(&pdev->dev);
struct prestera_fw *fw;
int err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 13b0259f7ea6..8f306364f7bf 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -1876,7 +1876,7 @@ static void mlxsw_pci_cmd_fini(struct mlxsw_pci *mlxsw_pci)
static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
- const char *driver_name = pdev->driver->name;
+ const char *driver_name = dev_driver_string(&pdev->dev);
struct mlxsw_pci *mlxsw_pci;
int err;
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
index 0685ece1f155..1de076f55740 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c
@@ -202,7 +202,8 @@ nfp_get_drvinfo(struct nfp_app *app, struct pci_dev *pdev,
{
char nsp_version[ETHTOOL_FWVERS_LEN] = {};
- strlcpy(drvinfo->driver, pdev->driver->name, sizeof(drvinfo->driver));
+ strlcpy(drvinfo->driver, dev_driver_string(&pdev->dev),
+ sizeof(drvinfo->driver));
nfp_net_get_nspinfo(app, nsp_version);
snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version),
"%s %s %s %s", vnic_version, nsp_version,
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v6 07/11] PCI: Replace pci_dev::driver usage that gets the driver name
2021-10-04 12:59 ` Uwe Kleine-König
@ 2021-10-04 16:06 ` Ido Schimmel
-1 siblings, 0 replies; 36+ messages in thread
From: Ido Schimmel @ 2021-10-04 16:06 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Bjorn Helgaas, linux-pci, kernel, Michael Ellerman,
Benjamin Herrenschmidt, Paul Mackerras, Rafał Miłecki,
Zhou Wang, Herbert Xu, David S. Miller, Yisen Zhuang,
Salil Mehta, Jakub Kicinski, Vadym Kochan, Taras Chornyi,
Jiri Pirko, Ido Schimmel, Simon Horman, Michael Buesch,
Oliver O'Halloran, Jesse Brandeburg, Alexander Duyck,
linuxppc-dev, linux-kernel, linux-wireless, linux-crypto, netdev,
oss-drivers
On Mon, Oct 04, 2021 at 02:59:31PM +0200, Uwe Kleine-König wrote:
> struct pci_dev::driver holds (apart from a constant offset) the same
> data as struct pci_dev::dev->driver. With the goal to remove struct
> pci_dev::driver to get rid of data duplication replace getting the
> driver name by dev_driver_string() which implicitly makes use of struct
> pci_dev::dev->driver.
>
> Acked-by: Simon Horman <simon.horman@corigine.com> (for NFP)
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
For mlxsw:
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Tested with the kexec flow that I mentioned last time. Works fine now.
Thanks
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v6 07/11] PCI: Replace pci_dev::driver usage that gets the driver name
@ 2021-10-04 16:06 ` Ido Schimmel
0 siblings, 0 replies; 36+ messages in thread
From: Ido Schimmel @ 2021-10-04 16:06 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-pci, Alexander Duyck, oss-drivers, Paul Mackerras,
Herbert Xu, Rafał Miłecki, Jesse Brandeburg,
Bjorn Helgaas, Ido Schimmel, Jakub Kicinski, Yisen Zhuang,
Vadym Kochan, Michael Buesch, Jiri Pirko, Salil Mehta, netdev,
linux-wireless, linux-kernel, Taras Chornyi, Zhou Wang,
linux-crypto, kernel, Simon Horman, Oliver O'Halloran,
linuxppc-dev, David S. Miller
On Mon, Oct 04, 2021 at 02:59:31PM +0200, Uwe Kleine-König wrote:
> struct pci_dev::driver holds (apart from a constant offset) the same
> data as struct pci_dev::dev->driver. With the goal to remove struct
> pci_dev::driver to get rid of data duplication replace getting the
> driver name by dev_driver_string() which implicitly makes use of struct
> pci_dev::dev->driver.
>
> Acked-by: Simon Horman <simon.horman@corigine.com> (for NFP)
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
For mlxsw:
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>
Tested with the kexec flow that I mentioned last time. Works fine now.
Thanks
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v6 08/11] scsi: message: fusion: Remove unused parameter of mpt_pci driver's probe()
2021-10-04 12:59 ` Uwe Kleine-König
` (7 preceding siblings ...)
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, kernel, Sathya Prakash, Sreekanth Reddy,
Suganath Prabu Subramani, MPT-FusionLinux.pdl, linux-scsi,
Martin K . Petersen, Christoph Hellwig
The only two drivers don't make use of the id parameter, so drop it.
This removes a usage of struct pci_dev::driver which is planned to be
removed as it tracks duplicate data.
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/message/fusion/mptbase.c | 7 ++-----
drivers/message/fusion/mptbase.h | 2 +-
drivers/message/fusion/mptctl.c | 4 ++--
drivers/message/fusion/mptlan.c | 2 +-
4 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 7f7abc9069f7..b94d5e4fdc23 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -829,7 +829,6 @@ int
mpt_device_driver_register(struct mpt_pci_driver * dd_cbfunc, u8 cb_idx)
{
MPT_ADAPTER *ioc;
- const struct pci_device_id *id;
if (!cb_idx || cb_idx >= MPT_MAX_PROTOCOL_DRIVERS)
return -EINVAL;
@@ -838,10 +837,8 @@ mpt_device_driver_register(struct mpt_pci_driver * dd_cbfunc, u8 cb_idx)
/* call per pci device probe entry point */
list_for_each_entry(ioc, &ioc_list, list) {
- id = ioc->pcidev->driver ?
- ioc->pcidev->driver->id_table : NULL;
if (dd_cbfunc->probe)
- dd_cbfunc->probe(ioc->pcidev, id);
+ dd_cbfunc->probe(ioc->pcidev);
}
return 0;
@@ -2032,7 +2029,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
for(cb_idx = 0; cb_idx < MPT_MAX_PROTOCOL_DRIVERS; cb_idx++) {
if(MptDeviceDriverHandlers[cb_idx] &&
MptDeviceDriverHandlers[cb_idx]->probe) {
- MptDeviceDriverHandlers[cb_idx]->probe(pdev,id);
+ MptDeviceDriverHandlers[cb_idx]->probe(pdev);
}
}
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index b9e0376be723..4bd0682c65d3 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -257,7 +257,7 @@ typedef enum {
} MPT_DRIVER_CLASS;
struct mpt_pci_driver{
- int (*probe) (struct pci_dev *dev, const struct pci_device_id *id);
+ int (*probe) (struct pci_dev *dev);
void (*remove) (struct pci_dev *dev);
};
diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
index 72025996cd70..ae433c150b37 100644
--- a/drivers/message/fusion/mptctl.c
+++ b/drivers/message/fusion/mptctl.c
@@ -114,7 +114,7 @@ static int mptctl_do_reset(MPT_ADAPTER *iocp, unsigned long arg);
static int mptctl_hp_hostinfo(MPT_ADAPTER *iocp, unsigned long arg, unsigned int cmd);
static int mptctl_hp_targetinfo(MPT_ADAPTER *iocp, unsigned long arg);
-static int mptctl_probe(struct pci_dev *, const struct pci_device_id *);
+static int mptctl_probe(struct pci_dev *);
static void mptctl_remove(struct pci_dev *);
#ifdef CONFIG_COMPAT
@@ -2838,7 +2838,7 @@ static long compat_mpctl_ioctl(struct file *f, unsigned int cmd, unsigned long a
*/
static int
-mptctl_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+mptctl_probe(struct pci_dev *pdev)
{
MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
index 3261cac762de..7c1af5e6eb0b 100644
--- a/drivers/message/fusion/mptlan.c
+++ b/drivers/message/fusion/mptlan.c
@@ -1377,7 +1377,7 @@ mpt_register_lan_device (MPT_ADAPTER *mpt_dev, int pnum)
}
static int
-mptlan_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+mptlan_probe(struct pci_dev *pdev)
{
MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
struct net_device *dev;
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 09/11] crypto: qat - simplify adf_enable_aer()
2021-10-04 12:59 ` Uwe Kleine-König
` (8 preceding siblings ...)
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, kernel, Giovanni Cabiddu, Herbert Xu, David S. Miller,
Tomaszx Kowalik, Fiona Trahe, Marco Chiappero, Andy Shevchenko,
Wojciech Ziemba, Jack Xu, qat-linux, linux-crypto
A struct pci_driver is supposed to be constant and assigning .err_handler
once per bound device isn't really sensible. Also as the function returns
zero unconditionally let it return no value instead and simplify the
callers accordingly.
As a side effect this removes one user of struct pci_dev::driver. This
member is planned to be removed.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/crypto/qat/qat_4xxx/adf_drv.c | 7 ++-----
drivers/crypto/qat/qat_c3xxx/adf_drv.c | 7 ++-----
drivers/crypto/qat/qat_c62x/adf_drv.c | 7 ++-----
drivers/crypto/qat/qat_common/adf_aer.c | 10 +++-------
drivers/crypto/qat/qat_common/adf_common_drv.h | 3 ++-
drivers/crypto/qat/qat_dh895xcc/adf_drv.c | 7 ++-----
6 files changed, 13 insertions(+), 28 deletions(-)
diff --git a/drivers/crypto/qat/qat_4xxx/adf_drv.c b/drivers/crypto/qat/qat_4xxx/adf_drv.c
index 359fb7989dfb..71ef065914b2 100644
--- a/drivers/crypto/qat/qat_4xxx/adf_drv.c
+++ b/drivers/crypto/qat/qat_4xxx/adf_drv.c
@@ -247,11 +247,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer.\n");
- ret = -EFAULT;
- goto out_err;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state.\n");
@@ -304,6 +300,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
module_pci_driver(adf_driver);
diff --git a/drivers/crypto/qat/qat_c3xxx/adf_drv.c b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
index cc6e75dc60de..2aef0bb791df 100644
--- a/drivers/crypto/qat/qat_c3xxx/adf_drv.c
+++ b/drivers/crypto/qat/qat_c3xxx/adf_drv.c
@@ -33,6 +33,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ -192,11 +193,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer\n");
- ret = -EFAULT;
- goto out_err_free_reg;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state\n");
diff --git a/drivers/crypto/qat/qat_c62x/adf_drv.c b/drivers/crypto/qat/qat_c62x/adf_drv.c
index bf251dfe74b3..56163083f161 100644
--- a/drivers/crypto/qat/qat_c62x/adf_drv.c
+++ b/drivers/crypto/qat/qat_c62x/adf_drv.c
@@ -33,6 +33,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ -192,11 +193,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer\n");
- ret = -EFAULT;
- goto out_err_free_reg;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state\n");
diff --git a/drivers/crypto/qat/qat_common/adf_aer.c b/drivers/crypto/qat/qat_common/adf_aer.c
index ed3e40bc56eb..fe9bb2f3536a 100644
--- a/drivers/crypto/qat/qat_common/adf_aer.c
+++ b/drivers/crypto/qat/qat_common/adf_aer.c
@@ -166,11 +166,12 @@ static void adf_resume(struct pci_dev *pdev)
dev_info(&pdev->dev, "Device is up and running\n");
}
-static const struct pci_error_handlers adf_err_handler = {
+const struct pci_error_handlers adf_err_handler = {
.error_detected = adf_error_detected,
.slot_reset = adf_slot_reset,
.resume = adf_resume,
};
+EXPORT_SYMBOL_GPL(adf_err_handler);
/**
* adf_enable_aer() - Enable Advance Error Reporting for acceleration device
@@ -179,17 +180,12 @@ static const struct pci_error_handlers adf_err_handler = {
* Function enables PCI Advance Error Reporting for the
* QAT acceleration device accel_dev.
* To be used by QAT device specific drivers.
- *
- * Return: 0 on success, error code otherwise.
*/
-int adf_enable_aer(struct adf_accel_dev *accel_dev)
+void adf_enable_aer(struct adf_accel_dev *accel_dev)
{
struct pci_dev *pdev = accel_to_pci_dev(accel_dev);
- struct pci_driver *pdrv = pdev->driver;
- pdrv->err_handler = &adf_err_handler;
pci_enable_pcie_error_reporting(pdev);
- return 0;
}
EXPORT_SYMBOL_GPL(adf_enable_aer);
diff --git a/drivers/crypto/qat/qat_common/adf_common_drv.h b/drivers/crypto/qat/qat_common/adf_common_drv.h
index 4261749fae8d..e4c24be212ff 100644
--- a/drivers/crypto/qat/qat_common/adf_common_drv.h
+++ b/drivers/crypto/qat/qat_common/adf_common_drv.h
@@ -95,7 +95,8 @@ void adf_ae_fw_release(struct adf_accel_dev *accel_dev);
int adf_ae_start(struct adf_accel_dev *accel_dev);
int adf_ae_stop(struct adf_accel_dev *accel_dev);
-int adf_enable_aer(struct adf_accel_dev *accel_dev);
+extern const struct pci_error_handlers adf_err_handler;
+void adf_enable_aer(struct adf_accel_dev *accel_dev);
void adf_disable_aer(struct adf_accel_dev *accel_dev);
void adf_reset_sbr(struct adf_accel_dev *accel_dev);
void adf_reset_flr(struct adf_accel_dev *accel_dev);
diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
index 3976a81bd99b..acca56752aa0 100644
--- a/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
+++ b/drivers/crypto/qat/qat_dh895xcc/adf_drv.c
@@ -33,6 +33,7 @@ static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
+ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ -192,11 +193,7 @@ static int adf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
}
pci_set_master(pdev);
- if (adf_enable_aer(accel_dev)) {
- dev_err(&pdev->dev, "Failed to enable aer\n");
- ret = -EFAULT;
- goto out_err_free_reg;
- }
+ adf_enable_aer(accel_dev);
if (pci_save_state(pdev)) {
dev_err(&pdev->dev, "Failed to save pci state\n");
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver
2021-10-04 12:59 ` Uwe Kleine-König
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Mark Rutland, Peter Zijlstra, linux-pci, Oliver O'Halloran,
H. Peter Anvin, Jiri Olsa, Boris Ostrovsky, Stefano Stabellini,
Mathias Nyman, x86, Alexander Shishkin, Ingo Molnar, xen-devel,
Andrew Donnellan, Arnd Bergmann, Konrad Rzeszutek Wilk,
Arnaldo Carvalho de Melo, Borislav Petkov, Bjorn Helgaas,
Namhyung Kim, Thomas Gleixner, Juergen Gross, Greg Kroah-Hartman,
linux-usb, linux-perf-users, kernel, Frederic Barrat,
Paul Mackerras, linuxppc-dev
struct pci_dev::driver contains (apart from a constant offset) the same
data as struct pci_dev::dev->driver. Replace all remaining users of the
former pointer by the latter to allow removing the former.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/powerpc/kernel/eeh_driver.c | 10 ++---
arch/x86/events/intel/uncore.c | 2 +-
arch/x86/kernel/probe_roms.c | 10 +++--
drivers/misc/cxl/guest.c | 24 +++++-----
drivers/misc/cxl/pci.c | 30 ++++++++-----
drivers/pci/iov.c | 33 ++++++++++----
drivers/pci/pci-driver.c | 76 +++++++++++++++++++-------------
drivers/pci/pci.c | 4 +-
drivers/pci/pcie/err.c | 36 ++++++++-------
drivers/pci/xen-pcifront.c | 4 +-
drivers/usb/host/xhci-pci.c | 2 +-
11 files changed, 140 insertions(+), 91 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 3eff6a4888e7..350dab18e137 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -104,13 +104,13 @@ static bool eeh_edev_actionable(struct eeh_dev *edev)
*/
static inline struct pci_driver *eeh_pcid_get(struct pci_dev *pdev)
{
- if (!pdev || !pdev->driver)
+ if (!pdev || !pdev->dev.driver)
return NULL;
- if (!try_module_get(pdev->driver->driver.owner))
+ if (!try_module_get(pdev->dev.driver->owner))
return NULL;
- return pdev->driver;
+ return to_pci_driver(pdev->dev.driver);
}
/**
@@ -122,10 +122,10 @@ static inline struct pci_driver *eeh_pcid_get(struct pci_dev *pdev)
*/
static inline void eeh_pcid_put(struct pci_dev *pdev)
{
- if (!pdev || !pdev->driver)
+ if (!pdev || !pdev->dev.driver)
return;
- module_put(pdev->driver->driver.owner);
+ module_put(pdev->dev.driver->owner);
}
/**
diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
index c72e368dd164..f1ba6ab2e97e 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -1187,7 +1187,7 @@ static int uncore_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id
* PCI slot and func to indicate the uncore box.
*/
if (id->driver_data & ~0xffff) {
- struct pci_driver *pci_drv = pdev->driver;
+ struct pci_driver *pci_drv = to_pci_driver(pdev->dev.driver);
pmu = uncore_pci_find_dev_pmu(pdev, pci_drv->id_table);
if (pmu == NULL)
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index 9e1def3744f2..deaaef6efe34 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -80,15 +80,17 @@ static struct resource video_rom_resource = {
*/
static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
{
- struct pci_driver *drv = pdev->driver;
const struct pci_device_id *id;
if (pdev->vendor == vendor && pdev->device == device)
return true;
- for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
- if (id->vendor == vendor && id->device == device)
- break;
+ if (pdev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
+ for (id = drv->id_table; id && id->vendor; id++)
+ if (id->vendor == vendor && id->device == device)
+ break;
+ }
return id && id->vendor;
}
diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
index 186308f1f8eb..d997c9c3ebb5 100644
--- a/drivers/misc/cxl/guest.c
+++ b/drivers/misc/cxl/guest.c
@@ -25,28 +25,32 @@ static void pci_error_handlers(struct cxl_afu *afu,
return;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- if (!afu_dev->driver)
+ struct pci_driver *afu_drv;
+
+ if (!afu_dev->dev.driver)
continue;
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+
switch (bus_error_event) {
case CXL_ERROR_DETECTED_EVENT:
afu_dev->error_state = state;
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->error_detected)
- afu_dev->driver->err_handler->error_detected(afu_dev, state);
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->error_detected)
+ afu_drv->err_handler->error_detected(afu_dev, state);
break;
case CXL_SLOT_RESET_EVENT:
afu_dev->error_state = state;
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->slot_reset)
- afu_dev->driver->err_handler->slot_reset(afu_dev);
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->slot_reset)
+ afu_drv->err_handler->slot_reset(afu_dev);
break;
case CXL_RESUME_EVENT:
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->resume)
- afu_dev->driver->err_handler->resume(afu_dev);
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->resume)
+ afu_drv->err_handler->resume(afu_dev);
break;
}
}
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 2ba899f5659f..7e7545d01e27 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1805,14 +1805,16 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
return result;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- if (!afu_dev->driver)
+ struct pci_driver *afu_drv;
+ if (!afu_dev->dev.driver)
continue;
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+
afu_dev->error_state = state;
- if (afu_dev->driver->err_handler)
- afu_result = afu_dev->driver->err_handler->error_detected(afu_dev,
- state);
+ if (afu_drv->err_handler)
+ afu_result = afu_drv->err_handler->error_detected(afu_dev, state);
/* Disconnect trumps all, NONE trumps NEED_RESET */
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -2003,6 +2005,8 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
+ struct pci_driver *afu_drv;
+
/* Reset the device context.
* TODO: make this less disruptive
*/
@@ -2028,12 +2032,14 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
* shouldn't start new work until we call
* their resume function.
*/
- if (!afu_dev->driver)
+ if (!afu_dev->dev.driver)
continue;
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->slot_reset)
- afu_result = afu_dev->driver->err_handler->slot_reset(afu_dev);
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->slot_reset)
+ afu_result = afu_drv->err_handler->slot_reset(afu_dev);
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -2074,9 +2080,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- if (afu_dev->driver && afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->resume)
- afu_dev->driver->err_handler->resume(afu_dev);
+ struct pci_driver *afu_drv;
+ if (afu_dev->dev.driver &&
+ (afu_drv = to_pci_driver(afu_dev->dev.driver))->err_handler &&
+ afu_drv->err_handler->resume)
+ afu_drv->err_handler->resume(afu_dev);
}
}
spin_unlock(&adapter->afu_list_lock);
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index dafdc652fcd0..0d0a34347868 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -164,13 +164,18 @@ static ssize_t sriov_vf_total_msix_show(struct device *dev,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_driver *pdrv;
u32 vf_total_msix = 0;
device_lock(dev);
- if (!pdev->driver || !pdev->driver->sriov_get_vf_total_msix)
+ if (!dev->driver)
goto unlock;
- vf_total_msix = pdev->driver->sriov_get_vf_total_msix(pdev);
+ pdrv = to_pci_driver(dev->driver);
+ if (!pdrv->sriov_get_vf_total_msix)
+ goto unlock;
+
+ vf_total_msix = pdrv->sriov_get_vf_total_msix(pdev);
unlock:
device_unlock(dev);
return sysfs_emit(buf, "%u\n", vf_total_msix);
@@ -183,6 +188,7 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
{
struct pci_dev *vf_dev = to_pci_dev(dev);
struct pci_dev *pdev = pci_physfn(vf_dev);
+ struct pci_driver *pdrv;
int val, ret;
ret = kstrtoint(buf, 0, &val);
@@ -193,13 +199,19 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
return -EINVAL;
device_lock(&pdev->dev);
- if (!pdev->driver || !pdev->driver->sriov_set_msix_vec_count) {
+ if (!pdev->dev.driver) {
+ ret = -EOPNOTSUPP;
+ goto err_pdev;
+ }
+
+ pdrv = to_pci_driver(pdev->dev.driver);
+ if (!pdrv->sriov_set_msix_vec_count) {
ret = -EOPNOTSUPP;
goto err_pdev;
}
device_lock(&vf_dev->dev);
- if (vf_dev->driver) {
+ if (vf_dev->dev.driver) {
/*
* A driver is already attached to this VF and has configured
* itself based on the current MSI-X vector count. Changing
@@ -209,7 +221,7 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
goto err_dev;
}
- ret = pdev->driver->sriov_set_msix_vec_count(vf_dev, val);
+ ret = pdrv->sriov_set_msix_vec_count(vf_dev, val);
err_dev:
device_unlock(&vf_dev->dev);
@@ -376,6 +388,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_driver *pdrv;
int ret;
u16 num_vfs;
@@ -392,14 +405,16 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
/* is PF driver loaded */
- if (!pdev->driver) {
+ if (!pdev->dev.driver) {
pci_info(pdev, "no driver bound to device; cannot configure SR-IOV\n");
ret = -ENOENT;
goto exit;
}
+ pdrv = to_pci_driver(pdev->dev.driver);
+
/* is PF driver loaded w/callback */
- if (!pdev->driver->sriov_configure) {
+ if (!pdrv->sriov_configure) {
pci_info(pdev, "driver does not support SR-IOV configuration via sysfs\n");
ret = -ENOENT;
goto exit;
@@ -407,7 +422,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
if (num_vfs == 0) {
/* disable VFs */
- ret = pdev->driver->sriov_configure(pdev, 0);
+ ret = pdrv->sriov_configure(pdev, 0);
goto exit;
}
@@ -419,7 +434,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
}
- ret = pdev->driver->sriov_configure(pdev, num_vfs);
+ ret = pdrv->sriov_configure(pdev, num_vfs);
if (ret < 0)
goto exit;
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 50449ec622a3..8654fe70cd66 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -457,7 +457,7 @@ static int pci_device_probe(struct device *dev)
static void pci_device_remove(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
if (drv->remove) {
pm_runtime_get_sync(dev);
@@ -493,12 +493,15 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
pm_runtime_resume(dev);
- if (drv && drv->shutdown)
- drv->shutdown(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->shutdown)
+ drv->shutdown(pci_dev);
+ }
/*
* If this is a kexec reboot, turn off Bus Master bit on the
@@ -589,22 +592,25 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
- if (drv && drv->suspend) {
- pci_power_t prev = pci_dev->current_state;
- int error;
+ if (dev->driver) {
+ struct pci_driver *drv = to_pci_driver(dev->driver);
- error = drv->suspend(pci_dev, state);
- suspend_report_result(drv->suspend, error);
- if (error)
- return error;
+ if (drv->suspend) {
+ pci_power_t prev = pci_dev->current_state;
+ int error;
- if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
- && pci_dev->current_state != PCI_UNKNOWN) {
- pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
- "PCI PM: Device state not saved by %pS\n",
- drv->suspend);
+ error = drv->suspend(pci_dev, state);
+ suspend_report_result(drv->suspend, error);
+ if (error)
+ return error;
+
+ if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
+ && pci_dev->current_state != PCI_UNKNOWN) {
+ pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
+ "PCI PM: Device state not saved by %pS\n",
+ drv->suspend);
+ }
}
}
@@ -630,12 +636,17 @@ static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
pci_fixup_device(pci_fixup_resume, pci_dev);
- return drv && drv->resume ?
- drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->resume)
+ return drv->resume(pci_dev);
+ }
+
+ return pci_pm_reenable_device(pci_dev);
}
/* Auxiliary functions used by the new power management framework */
@@ -649,8 +660,14 @@ static void pci_pm_default_suspend(struct pci_dev *pci_dev)
static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
{
- struct pci_driver *drv = pci_dev->driver;
- bool ret = drv && (drv->suspend || drv->resume);
+ struct pci_driver *drv;
+ bool ret;
+
+ if (!pci_dev->dev.driver)
+ return false;
+
+ drv = to_pci_driver(pci_dev->dev.driver);
+ ret = drv && (drv->suspend || drv->resume);
/*
* Legacy PM support is used by default, so warn if the new framework is
@@ -1242,11 +1259,11 @@ static int pci_pm_runtime_suspend(struct device *dev)
int error;
/*
- * If pci_dev->driver is not set (unbound), we leave the device in D0,
+ * If pci_dev->dev.driver is not set (unbound), we leave the device in D0,
* but it may go to D3cold when the bridge above it runtime suspends.
* Save its config space in case that happens.
*/
- if (!pci_dev->driver) {
+ if (!pci_dev->dev.driver) {
pci_save_state(pci_dev);
return 0;
}
@@ -1303,7 +1320,7 @@ static int pci_pm_runtime_resume(struct device *dev)
*/
pci_restore_standard_config(pci_dev);
- if (!pci_dev->driver)
+ if (!dev->driver)
return 0;
pci_fixup_device(pci_fixup_resume_early, pci_dev);
@@ -1322,14 +1339,13 @@ static int pci_pm_runtime_resume(struct device *dev)
static int pci_pm_runtime_idle(struct device *dev)
{
- struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
/*
- * If pci_dev->driver is not set (unbound), the device should
+ * If dev->driver is not set (unbound), the device should
* always remain in D0 regardless of the runtime PM status
*/
- if (!pci_dev->driver)
+ if (!dev->driver)
return 0;
if (!pm)
@@ -1436,8 +1452,8 @@ static struct pci_driver pci_compat_driver = {
*/
struct pci_driver *pci_dev_driver(const struct pci_dev *dev)
{
- if (dev->driver)
- return dev->driver;
+ if (dev->dev.driver)
+ return to_pci_driver(dev->dev.driver);
else {
int i;
for (i = 0; i <= PCI_ROM_RESOURCE; i++)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ce2ab62b64cf..ccecf740de59 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5089,7 +5089,7 @@ EXPORT_SYMBOL_GPL(pci_dev_unlock);
static void pci_dev_save_and_disable(struct pci_dev *dev)
{
const struct pci_error_handlers *err_handler =
- dev->driver ? dev->driver->err_handler : NULL;
+ dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
/*
* dev->driver->err_handler->reset_prepare() is protected against
@@ -5120,7 +5120,7 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
static void pci_dev_restore(struct pci_dev *dev)
{
const struct pci_error_handlers *err_handler =
- dev->driver ? dev->driver->err_handler : NULL;
+ dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
pci_restore_state(dev);
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index b576aa890c76..b314b54f7821 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -49,14 +49,15 @@ static int report_error_detected(struct pci_dev *dev,
pci_channel_state_t state,
enum pci_ers_result *result)
{
+ struct pci_driver *pdrv;
pci_ers_result_t vote;
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
if (!pci_dev_set_io_state(dev, state) ||
- !dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->error_detected) {
+ !dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->error_detected) {
/*
* If any device in the subtree does not have an error_detected
* callback, PCI_ERS_RESULT_NO_AER_DRIVER prevents subsequent
@@ -70,7 +71,7 @@ static int report_error_detected(struct pci_dev *dev,
vote = PCI_ERS_RESULT_NONE;
}
} else {
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
vote = err_handler->error_detected(dev, state);
}
pci_uevent_ers(dev, vote);
@@ -92,15 +93,16 @@ static int report_normal_detected(struct pci_dev *dev, void *data)
static int report_mmio_enabled(struct pci_dev *dev, void *data)
{
pci_ers_result_t vote, *result = data;
+ struct pci_driver *pdrv;
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->mmio_enabled)
+ if (!dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->mmio_enabled)
goto out;
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
vote = err_handler->mmio_enabled(dev);
*result = merge_result(*result, vote);
out:
@@ -112,14 +114,15 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
{
pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
+ struct pci_driver *pdrv;
device_lock(&dev->dev);
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->slot_reset)
+ if (!dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->slot_reset)
goto out;
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
vote = err_handler->slot_reset(dev);
*result = merge_result(*result, vote);
out:
@@ -130,15 +133,16 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
static int report_resume(struct pci_dev *dev, void *data)
{
const struct pci_error_handlers *err_handler;
+ struct pci_driver *pdrv;
device_lock(&dev->dev);
if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
- !dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->resume)
+ !dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->resume)
goto out;
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
err_handler->resume(dev);
out:
pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED);
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index f2d7f70a7a10..73831fb87a1e 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -601,12 +601,12 @@ static pci_ers_result_t pcifront_common_process(int cmd,
result = PCI_ERS_RESULT_NONE;
pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
- if (!pcidev || !pcidev->driver) {
+ if (!pcidev || !pcidev->dev.driver) {
dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
pci_dev_put(pcidev);
return result;
}
- pdrv = pcidev->driver;
+ pdrv = to_pci_driver(pcidev->dev.driver);
if (pdrv->err_handler && pdrv->err_handler->error_detected) {
pci_dbg(pcidev, "trying to call AER service\n");
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 2c9f25ca8edd..2f4729f4f1e0 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -103,7 +103,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
struct xhci_driver_data *driver_data;
const struct pci_device_id *id;
- id = pci_match_id(pdev->driver->id_table, pdev);
+ id = pci_match_id(to_pci_driver(pdev->dev.driver)->id_table, pdev);
if (id && id->driver_data) {
driver_data = (struct xhci_driver_data *)id->driver_data;
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v6 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver
@ 2021-10-04 12:59 ` Uwe Kleine-König
0 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, kernel, Russell Currey, Oliver O'Halloran,
Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin,
Frederic Barrat, Andrew Donnellan, Arnd Bergmann,
Greg Kroah-Hartman, Bjorn Helgaas, Boris Ostrovsky,
Juergen Gross, Stefano Stabellini, Konrad Rzeszutek Wilk,
Mathias Nyman, linuxppc-dev, linux-perf-users, xen-devel,
linux-usb
struct pci_dev::driver contains (apart from a constant offset) the same
data as struct pci_dev::dev->driver. Replace all remaining users of the
former pointer by the latter to allow removing the former.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
arch/powerpc/kernel/eeh_driver.c | 10 ++---
arch/x86/events/intel/uncore.c | 2 +-
arch/x86/kernel/probe_roms.c | 10 +++--
drivers/misc/cxl/guest.c | 24 +++++-----
drivers/misc/cxl/pci.c | 30 ++++++++-----
drivers/pci/iov.c | 33 ++++++++++----
drivers/pci/pci-driver.c | 76 +++++++++++++++++++-------------
drivers/pci/pci.c | 4 +-
drivers/pci/pcie/err.c | 36 ++++++++-------
drivers/pci/xen-pcifront.c | 4 +-
drivers/usb/host/xhci-pci.c | 2 +-
11 files changed, 140 insertions(+), 91 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 3eff6a4888e7..350dab18e137 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -104,13 +104,13 @@ static bool eeh_edev_actionable(struct eeh_dev *edev)
*/
static inline struct pci_driver *eeh_pcid_get(struct pci_dev *pdev)
{
- if (!pdev || !pdev->driver)
+ if (!pdev || !pdev->dev.driver)
return NULL;
- if (!try_module_get(pdev->driver->driver.owner))
+ if (!try_module_get(pdev->dev.driver->owner))
return NULL;
- return pdev->driver;
+ return to_pci_driver(pdev->dev.driver);
}
/**
@@ -122,10 +122,10 @@ static inline struct pci_driver *eeh_pcid_get(struct pci_dev *pdev)
*/
static inline void eeh_pcid_put(struct pci_dev *pdev)
{
- if (!pdev || !pdev->driver)
+ if (!pdev || !pdev->dev.driver)
return;
- module_put(pdev->driver->driver.owner);
+ module_put(pdev->dev.driver->owner);
}
/**
diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
index c72e368dd164..f1ba6ab2e97e 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -1187,7 +1187,7 @@ static int uncore_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id
* PCI slot and func to indicate the uncore box.
*/
if (id->driver_data & ~0xffff) {
- struct pci_driver *pci_drv = pdev->driver;
+ struct pci_driver *pci_drv = to_pci_driver(pdev->dev.driver);
pmu = uncore_pci_find_dev_pmu(pdev, pci_drv->id_table);
if (pmu == NULL)
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index 9e1def3744f2..deaaef6efe34 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -80,15 +80,17 @@ static struct resource video_rom_resource = {
*/
static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
{
- struct pci_driver *drv = pdev->driver;
const struct pci_device_id *id;
if (pdev->vendor == vendor && pdev->device == device)
return true;
- for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
- if (id->vendor == vendor && id->device == device)
- break;
+ if (pdev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
+ for (id = drv->id_table; id && id->vendor; id++)
+ if (id->vendor == vendor && id->device == device)
+ break;
+ }
return id && id->vendor;
}
diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
index 186308f1f8eb..d997c9c3ebb5 100644
--- a/drivers/misc/cxl/guest.c
+++ b/drivers/misc/cxl/guest.c
@@ -25,28 +25,32 @@ static void pci_error_handlers(struct cxl_afu *afu,
return;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- if (!afu_dev->driver)
+ struct pci_driver *afu_drv;
+
+ if (!afu_dev->dev.driver)
continue;
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+
switch (bus_error_event) {
case CXL_ERROR_DETECTED_EVENT:
afu_dev->error_state = state;
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->error_detected)
- afu_dev->driver->err_handler->error_detected(afu_dev, state);
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->error_detected)
+ afu_drv->err_handler->error_detected(afu_dev, state);
break;
case CXL_SLOT_RESET_EVENT:
afu_dev->error_state = state;
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->slot_reset)
- afu_dev->driver->err_handler->slot_reset(afu_dev);
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->slot_reset)
+ afu_drv->err_handler->slot_reset(afu_dev);
break;
case CXL_RESUME_EVENT:
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->resume)
- afu_dev->driver->err_handler->resume(afu_dev);
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->resume)
+ afu_drv->err_handler->resume(afu_dev);
break;
}
}
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 2ba899f5659f..7e7545d01e27 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1805,14 +1805,16 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
return result;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- if (!afu_dev->driver)
+ struct pci_driver *afu_drv;
+ if (!afu_dev->dev.driver)
continue;
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+
afu_dev->error_state = state;
- if (afu_dev->driver->err_handler)
- afu_result = afu_dev->driver->err_handler->error_detected(afu_dev,
- state);
+ if (afu_drv->err_handler)
+ afu_result = afu_drv->err_handler->error_detected(afu_dev, state);
/* Disconnect trumps all, NONE trumps NEED_RESET */
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -2003,6 +2005,8 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
+ struct pci_driver *afu_drv;
+
/* Reset the device context.
* TODO: make this less disruptive
*/
@@ -2028,12 +2032,14 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
* shouldn't start new work until we call
* their resume function.
*/
- if (!afu_dev->driver)
+ if (!afu_dev->dev.driver)
continue;
- if (afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->slot_reset)
- afu_result = afu_dev->driver->err_handler->slot_reset(afu_dev);
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+
+ if (afu_drv->err_handler &&
+ afu_drv->err_handler->slot_reset)
+ afu_result = afu_drv->err_handler->slot_reset(afu_dev);
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -2074,9 +2080,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- if (afu_dev->driver && afu_dev->driver->err_handler &&
- afu_dev->driver->err_handler->resume)
- afu_dev->driver->err_handler->resume(afu_dev);
+ struct pci_driver *afu_drv;
+ if (afu_dev->dev.driver &&
+ (afu_drv = to_pci_driver(afu_dev->dev.driver))->err_handler &&
+ afu_drv->err_handler->resume)
+ afu_drv->err_handler->resume(afu_dev);
}
}
spin_unlock(&adapter->afu_list_lock);
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index dafdc652fcd0..0d0a34347868 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -164,13 +164,18 @@ static ssize_t sriov_vf_total_msix_show(struct device *dev,
char *buf)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_driver *pdrv;
u32 vf_total_msix = 0;
device_lock(dev);
- if (!pdev->driver || !pdev->driver->sriov_get_vf_total_msix)
+ if (!dev->driver)
goto unlock;
- vf_total_msix = pdev->driver->sriov_get_vf_total_msix(pdev);
+ pdrv = to_pci_driver(dev->driver);
+ if (!pdrv->sriov_get_vf_total_msix)
+ goto unlock;
+
+ vf_total_msix = pdrv->sriov_get_vf_total_msix(pdev);
unlock:
device_unlock(dev);
return sysfs_emit(buf, "%u\n", vf_total_msix);
@@ -183,6 +188,7 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
{
struct pci_dev *vf_dev = to_pci_dev(dev);
struct pci_dev *pdev = pci_physfn(vf_dev);
+ struct pci_driver *pdrv;
int val, ret;
ret = kstrtoint(buf, 0, &val);
@@ -193,13 +199,19 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
return -EINVAL;
device_lock(&pdev->dev);
- if (!pdev->driver || !pdev->driver->sriov_set_msix_vec_count) {
+ if (!pdev->dev.driver) {
+ ret = -EOPNOTSUPP;
+ goto err_pdev;
+ }
+
+ pdrv = to_pci_driver(pdev->dev.driver);
+ if (!pdrv->sriov_set_msix_vec_count) {
ret = -EOPNOTSUPP;
goto err_pdev;
}
device_lock(&vf_dev->dev);
- if (vf_dev->driver) {
+ if (vf_dev->dev.driver) {
/*
* A driver is already attached to this VF and has configured
* itself based on the current MSI-X vector count. Changing
@@ -209,7 +221,7 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
goto err_dev;
}
- ret = pdev->driver->sriov_set_msix_vec_count(vf_dev, val);
+ ret = pdrv->sriov_set_msix_vec_count(vf_dev, val);
err_dev:
device_unlock(&vf_dev->dev);
@@ -376,6 +388,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
const char *buf, size_t count)
{
struct pci_dev *pdev = to_pci_dev(dev);
+ struct pci_driver *pdrv;
int ret;
u16 num_vfs;
@@ -392,14 +405,16 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
/* is PF driver loaded */
- if (!pdev->driver) {
+ if (!pdev->dev.driver) {
pci_info(pdev, "no driver bound to device; cannot configure SR-IOV\n");
ret = -ENOENT;
goto exit;
}
+ pdrv = to_pci_driver(pdev->dev.driver);
+
/* is PF driver loaded w/callback */
- if (!pdev->driver->sriov_configure) {
+ if (!pdrv->sriov_configure) {
pci_info(pdev, "driver does not support SR-IOV configuration via sysfs\n");
ret = -ENOENT;
goto exit;
@@ -407,7 +422,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
if (num_vfs == 0) {
/* disable VFs */
- ret = pdev->driver->sriov_configure(pdev, 0);
+ ret = pdrv->sriov_configure(pdev, 0);
goto exit;
}
@@ -419,7 +434,7 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
}
- ret = pdev->driver->sriov_configure(pdev, num_vfs);
+ ret = pdrv->sriov_configure(pdev, num_vfs);
if (ret < 0)
goto exit;
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 50449ec622a3..8654fe70cd66 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -457,7 +457,7 @@ static int pci_device_probe(struct device *dev)
static void pci_device_remove(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
if (drv->remove) {
pm_runtime_get_sync(dev);
@@ -493,12 +493,15 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
pm_runtime_resume(dev);
- if (drv && drv->shutdown)
- drv->shutdown(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->shutdown)
+ drv->shutdown(pci_dev);
+ }
/*
* If this is a kexec reboot, turn off Bus Master bit on the
@@ -589,22 +592,25 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
- if (drv && drv->suspend) {
- pci_power_t prev = pci_dev->current_state;
- int error;
+ if (dev->driver) {
+ struct pci_driver *drv = to_pci_driver(dev->driver);
- error = drv->suspend(pci_dev, state);
- suspend_report_result(drv->suspend, error);
- if (error)
- return error;
+ if (drv->suspend) {
+ pci_power_t prev = pci_dev->current_state;
+ int error;
- if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
- && pci_dev->current_state != PCI_UNKNOWN) {
- pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
- "PCI PM: Device state not saved by %pS\n",
- drv->suspend);
+ error = drv->suspend(pci_dev, state);
+ suspend_report_result(drv->suspend, error);
+ if (error)
+ return error;
+
+ if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
+ && pci_dev->current_state != PCI_UNKNOWN) {
+ pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
+ "PCI PM: Device state not saved by %pS\n",
+ drv->suspend);
+ }
}
}
@@ -630,12 +636,17 @@ static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
pci_fixup_device(pci_fixup_resume, pci_dev);
- return drv && drv->resume ?
- drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->resume)
+ return drv->resume(pci_dev);
+ }
+
+ return pci_pm_reenable_device(pci_dev);
}
/* Auxiliary functions used by the new power management framework */
@@ -649,8 +660,14 @@ static void pci_pm_default_suspend(struct pci_dev *pci_dev)
static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
{
- struct pci_driver *drv = pci_dev->driver;
- bool ret = drv && (drv->suspend || drv->resume);
+ struct pci_driver *drv;
+ bool ret;
+
+ if (!pci_dev->dev.driver)
+ return false;
+
+ drv = to_pci_driver(pci_dev->dev.driver);
+ ret = drv && (drv->suspend || drv->resume);
/*
* Legacy PM support is used by default, so warn if the new framework is
@@ -1242,11 +1259,11 @@ static int pci_pm_runtime_suspend(struct device *dev)
int error;
/*
- * If pci_dev->driver is not set (unbound), we leave the device in D0,
+ * If pci_dev->dev.driver is not set (unbound), we leave the device in D0,
* but it may go to D3cold when the bridge above it runtime suspends.
* Save its config space in case that happens.
*/
- if (!pci_dev->driver) {
+ if (!pci_dev->dev.driver) {
pci_save_state(pci_dev);
return 0;
}
@@ -1303,7 +1320,7 @@ static int pci_pm_runtime_resume(struct device *dev)
*/
pci_restore_standard_config(pci_dev);
- if (!pci_dev->driver)
+ if (!dev->driver)
return 0;
pci_fixup_device(pci_fixup_resume_early, pci_dev);
@@ -1322,14 +1339,13 @@ static int pci_pm_runtime_resume(struct device *dev)
static int pci_pm_runtime_idle(struct device *dev)
{
- struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
/*
- * If pci_dev->driver is not set (unbound), the device should
+ * If dev->driver is not set (unbound), the device should
* always remain in D0 regardless of the runtime PM status
*/
- if (!pci_dev->driver)
+ if (!dev->driver)
return 0;
if (!pm)
@@ -1436,8 +1452,8 @@ static struct pci_driver pci_compat_driver = {
*/
struct pci_driver *pci_dev_driver(const struct pci_dev *dev)
{
- if (dev->driver)
- return dev->driver;
+ if (dev->dev.driver)
+ return to_pci_driver(dev->dev.driver);
else {
int i;
for (i = 0; i <= PCI_ROM_RESOURCE; i++)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ce2ab62b64cf..ccecf740de59 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5089,7 +5089,7 @@ EXPORT_SYMBOL_GPL(pci_dev_unlock);
static void pci_dev_save_and_disable(struct pci_dev *dev)
{
const struct pci_error_handlers *err_handler =
- dev->driver ? dev->driver->err_handler : NULL;
+ dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
/*
* dev->driver->err_handler->reset_prepare() is protected against
@@ -5120,7 +5120,7 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
static void pci_dev_restore(struct pci_dev *dev)
{
const struct pci_error_handlers *err_handler =
- dev->driver ? dev->driver->err_handler : NULL;
+ dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
pci_restore_state(dev);
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index b576aa890c76..b314b54f7821 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -49,14 +49,15 @@ static int report_error_detected(struct pci_dev *dev,
pci_channel_state_t state,
enum pci_ers_result *result)
{
+ struct pci_driver *pdrv;
pci_ers_result_t vote;
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
if (!pci_dev_set_io_state(dev, state) ||
- !dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->error_detected) {
+ !dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->error_detected) {
/*
* If any device in the subtree does not have an error_detected
* callback, PCI_ERS_RESULT_NO_AER_DRIVER prevents subsequent
@@ -70,7 +71,7 @@ static int report_error_detected(struct pci_dev *dev,
vote = PCI_ERS_RESULT_NONE;
}
} else {
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
vote = err_handler->error_detected(dev, state);
}
pci_uevent_ers(dev, vote);
@@ -92,15 +93,16 @@ static int report_normal_detected(struct pci_dev *dev, void *data)
static int report_mmio_enabled(struct pci_dev *dev, void *data)
{
pci_ers_result_t vote, *result = data;
+ struct pci_driver *pdrv;
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->mmio_enabled)
+ if (!dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->mmio_enabled)
goto out;
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
vote = err_handler->mmio_enabled(dev);
*result = merge_result(*result, vote);
out:
@@ -112,14 +114,15 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
{
pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
+ struct pci_driver *pdrv;
device_lock(&dev->dev);
- if (!dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->slot_reset)
+ if (!dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->slot_reset)
goto out;
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
vote = err_handler->slot_reset(dev);
*result = merge_result(*result, vote);
out:
@@ -130,15 +133,16 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
static int report_resume(struct pci_dev *dev, void *data)
{
const struct pci_error_handlers *err_handler;
+ struct pci_driver *pdrv;
device_lock(&dev->dev);
if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
- !dev->driver ||
- !dev->driver->err_handler ||
- !dev->driver->err_handler->resume)
+ !dev->dev.driver ||
+ !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv->err_handler->resume)
goto out;
- err_handler = dev->driver->err_handler;
+ err_handler = pdrv->err_handler;
err_handler->resume(dev);
out:
pci_uevent_ers(dev, PCI_ERS_RESULT_RECOVERED);
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index f2d7f70a7a10..73831fb87a1e 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -601,12 +601,12 @@ static pci_ers_result_t pcifront_common_process(int cmd,
result = PCI_ERS_RESULT_NONE;
pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
- if (!pcidev || !pcidev->driver) {
+ if (!pcidev || !pcidev->dev.driver) {
dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
pci_dev_put(pcidev);
return result;
}
- pdrv = pcidev->driver;
+ pdrv = to_pci_driver(pcidev->dev.driver);
if (pdrv->err_handler && pdrv->err_handler->error_detected) {
pci_dbg(pcidev, "trying to call AER service\n");
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 2c9f25ca8edd..2f4729f4f1e0 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -103,7 +103,7 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
struct xhci_driver_data *driver_data;
const struct pci_device_id *id;
- id = pci_match_id(pdev->driver->id_table, pdev);
+ id = pci_match_id(to_pci_driver(pdev->dev.driver)->id_table, pdev);
if (id && id->driver_data) {
driver_data = (struct xhci_driver_data *)id->driver_data;
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v6 11/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-04 12:59 ` Uwe Kleine-König
` (10 preceding siblings ...)
(?)
@ 2021-10-04 12:59 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-04 12:59 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci, kernel
Currently it's tracked twice which driver is bound to a given pci
device. Now that all users of the pci specific one (struct
pci_dev::driver) are updated to use an access macro
(pci_driver_of_dev()), change the macro to use the information from the
driver core and remove the driver member from struct pci_dev.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/pci/pci-driver.c | 4 ----
include/linux/pci.h | 1 -
2 files changed, 5 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 8654fe70cd66..e94aa338bab4 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -319,12 +319,10 @@ static long local_pci_probe(void *_ddi)
* its remove routine.
*/
pm_runtime_get_sync(dev);
- pci_dev->driver = pci_drv;
rc = pci_drv->probe(pci_dev, ddi->id);
if (!rc)
return rc;
if (rc < 0) {
- pci_dev->driver = NULL;
pm_runtime_put_sync(dev);
return rc;
}
@@ -390,7 +388,6 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
* @pci_dev: PCI device being probed
*
* returns 0 on success, else error.
- * side-effect: pci_dev->driver is set to drv when drv claims pci_dev.
*/
static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
{
@@ -465,7 +462,6 @@ static void pci_device_remove(struct device *dev)
pm_runtime_put_noidle(dev);
}
pcibios_free_irq(pci_dev);
- pci_dev->driver = NULL;
pci_iov_remove(pci_dev);
/* Undo the runtime PM settings in local_pci_probe() */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index cd8aa6fce204..7c1ceb39035c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -342,7 +342,6 @@ struct pci_dev {
u16 pcie_flags_reg; /* Cached PCIe Capabilities Register */
unsigned long *dma_alias_mask;/* Mask of enabled devfn aliases */
- struct pci_driver *driver; /* Driver bound to this device */
u64 dma_mask; /* Mask of the bits of bus address this
device implements. Normally this is
0xffffffff. You only need to change
--
2.30.2
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-04 12:59 ` Uwe Kleine-König
@ 2021-10-12 23:32 ` Bjorn Helgaas
-1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-12 23:32 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-pci, kernel, Alexander Duyck, Alexander Shishkin,
Andrew Donnellan, Andy Shevchenko, Arnaldo Carvalho de Melo,
Arnd Bergmann, Benjamin Herrenschmidt, Bjorn Helgaas,
Borislav Petkov, Boris Ostrovsky, David S. Miller, Fiona Trahe,
Frederic Barrat, Giovanni Cabiddu, Greg Kroah-Hartman,
Herbert Xu, H. Peter Anvin, Ido Schimmel, Ingo Molnar, Jack Xu,
Jakub Kicinski, Jesse Brandeburg, Jiri Olsa, Jiri Pirko,
Juergen Gross, Konrad Rzeszutek Wilk, Marco Chiappero,
Mark Rutland, Mathias Nyman, Michael Buesch, Michael Ellerman,
Namhyung Kim, Oliver O'Halloran, Paul Mackerras,
Peter Zijlstra, Rafał Miłecki, Russell Currey,
Salil Mehta, Sathya Prakash, Simon Horman, Sreekanth Reddy,
Stefano Stabellini, Suganath Prabu Subramani, Taras Chornyi,
Thomas Gleixner, Tomaszx Kowalik, Vadym Kochan, Wojciech Ziemba,
Yisen Zhuang, Zhou Wang, linux-crypto, linux-kernel,
linux-perf-users, linuxppc-dev, linux-scsi, linux-usb,
linux-wireless, MPT-FusionLinux.pdl, netdev, oss-drivers,
qat-linux, x86, xen-devel
On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> Hello,
>
> this is v6 of the quest to drop the "driver" member from struct pci_dev
> which tracks the same data (apart from a constant offset) as dev.driver.
I like this a lot and applied it to pci/driver for v5.16, thanks!
I split some of the bigger patches apart so they only touched one
driver or subsystem at a time. I also updated to_pci_driver() so it
returns NULL when given NULL, which makes some of the validations
quite a bit simpler, especially in the PM code in pci-driver.c.
Full interdiff from this v6 series:
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index deaaef6efe34..36e84d904260 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -80,17 +80,15 @@ static struct resource video_rom_resource = {
*/
static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
{
+ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
const struct pci_device_id *id;
if (pdev->vendor == vendor && pdev->device == device)
return true;
- if (pdev->dev.driver) {
- struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
- for (id = drv->id_table; id && id->vendor; id++)
- if (id->vendor == vendor && id->device == device)
- break;
- }
+ for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
+ if (id->vendor == vendor && id->device == device)
+ break;
return id && id->vendor;
}
diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
index d997c9c3ebb5..7eb3706cf42d 100644
--- a/drivers/misc/cxl/guest.c
+++ b/drivers/misc/cxl/guest.c
@@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
pci_channel_state_t state)
{
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
if (afu->phb == NULL)
return;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
-
- if (!afu_dev->dev.driver)
- continue;
-
afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
+ err_handler = afu_drv->err_handler;
switch (bus_error_event) {
case CXL_ERROR_DETECTED_EVENT:
afu_dev->error_state = state;
- if (afu_drv->err_handler &&
- afu_drv->err_handler->error_detected)
- afu_drv->err_handler->error_detected(afu_dev, state);
- break;
+ if (err_handler &&
+ err_handler->error_detected)
+ err_handler->error_detected(afu_dev, state);
+ break;
case CXL_SLOT_RESET_EVENT:
afu_dev->error_state = state;
- if (afu_drv->err_handler &&
- afu_drv->err_handler->slot_reset)
- afu_drv->err_handler->slot_reset(afu_dev);
- break;
+ if (err_handler &&
+ err_handler->slot_reset)
+ err_handler->slot_reset(afu_dev);
+ break;
case CXL_RESUME_EVENT:
- if (afu_drv->err_handler &&
- afu_drv->err_handler->resume)
- afu_drv->err_handler->resume(afu_dev);
- break;
+ if (err_handler &&
+ err_handler->resume)
+ err_handler->resume(afu_dev);
+ break;
}
}
}
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 7e7545d01e27..08bd81854101 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1795,6 +1795,8 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
pci_channel_state_t state)
{
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
pci_ers_result_t afu_result = PCI_ERS_RESULT_NEED_RESET;
@@ -1805,16 +1807,16 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
return result;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
- if (!afu_dev->dev.driver)
- continue;
-
afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
afu_dev->error_state = state;
- if (afu_drv->err_handler)
- afu_result = afu_drv->err_handler->error_detected(afu_dev, state);
+ err_handler = afu_drv->err_handler;
+ if (err_handler)
+ afu_result = err_handler->error_detected(afu_dev,
+ state);
/* Disconnect trumps all, NONE trumps NEED_RESET */
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -1974,6 +1976,8 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
struct cxl_afu *afu;
struct cxl_context *ctx;
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
pci_ers_result_t afu_result = PCI_ERS_RESULT_RECOVERED;
pci_ers_result_t result = PCI_ERS_RESULT_RECOVERED;
int i;
@@ -2005,8 +2009,6 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
-
/* Reset the device context.
* TODO: make this less disruptive
*/
@@ -2032,14 +2034,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
* shouldn't start new work until we call
* their resume function.
*/
- if (!afu_dev->dev.driver)
- continue;
-
afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
- if (afu_drv->err_handler &&
- afu_drv->err_handler->slot_reset)
- afu_result = afu_drv->err_handler->slot_reset(afu_dev);
+ err_handler = afu_drv->err_handler;
+ if (err_handler && err_handler->slot_reset)
+ afu_result = err_handler->slot_reset(afu_dev);
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -2066,6 +2067,8 @@ static void cxl_pci_resume(struct pci_dev *pdev)
struct cxl *adapter = pci_get_drvdata(pdev);
struct cxl_afu *afu;
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
int i;
/* Everything is back now. Drivers should restart work now.
@@ -2080,11 +2083,13 @@ static void cxl_pci_resume(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
- if (afu_dev->dev.driver &&
- (afu_drv = to_pci_driver(afu_dev->dev.driver))->err_handler &&
- afu_drv->err_handler->resume)
- afu_drv->err_handler->resume(afu_dev);
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
+
+ err_handler = afu_drv->err_handler;
+ if (err_handler && err_handler->resume)
+ err_handler->resume(afu_dev);
}
}
spin_unlock(&adapter->afu_list_lock);
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 0d0a34347868..fa4b52bb1e05 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -168,11 +168,8 @@ static ssize_t sriov_vf_total_msix_show(struct device *dev,
u32 vf_total_msix = 0;
device_lock(dev);
- if (!dev->driver)
- goto unlock;
-
pdrv = to_pci_driver(dev->driver);
- if (!pdrv->sriov_get_vf_total_msix)
+ if (!pdrv || !pdrv->sriov_get_vf_total_msix)
goto unlock;
vf_total_msix = pdrv->sriov_get_vf_total_msix(pdev);
@@ -199,19 +196,14 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
return -EINVAL;
device_lock(&pdev->dev);
- if (!pdev->dev.driver) {
- ret = -EOPNOTSUPP;
- goto err_pdev;
- }
-
- pdrv = to_pci_driver(pdev->dev.driver);
- if (!pdrv->sriov_set_msix_vec_count) {
+ pdrv = to_pci_driver(dev->driver);
+ if (!pdrv || !pdrv->sriov_set_msix_vec_count) {
ret = -EOPNOTSUPP;
goto err_pdev;
}
device_lock(&vf_dev->dev);
- if (vf_dev->dev.driver) {
+ if (to_pci_driver(vf_dev->dev.driver)) {
/*
* A driver is already attached to this VF and has configured
* itself based on the current MSI-X vector count. Changing
@@ -405,14 +397,13 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
/* is PF driver loaded */
- if (!pdev->dev.driver) {
+ pdrv = to_pci_driver(dev->driver);
+ if (!pdrv) {
pci_info(pdev, "no driver bound to device; cannot configure SR-IOV\n");
ret = -ENOENT;
goto exit;
}
- pdrv = to_pci_driver(pdev->dev.driver);
-
/* is PF driver loaded w/callback */
if (!pdrv->sriov_configure) {
pci_info(pdev, "driver does not support SR-IOV configuration via sysfs\n");
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e94aa338bab4..3884a1542e86 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -454,7 +454,7 @@ static int pci_device_probe(struct device *dev)
static void pci_device_remove(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
if (drv->remove) {
pm_runtime_get_sync(dev);
@@ -489,15 +489,12 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
pm_runtime_resume(dev);
- if (pci_dev->dev.driver) {
- struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
-
- if (drv->shutdown)
- drv->shutdown(pci_dev);
- }
+ if (drv && drv->shutdown)
+ drv->shutdown(pci_dev);
/*
* If this is a kexec reboot, turn off Bus Master bit on the
@@ -588,25 +585,22 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
- if (dev->driver) {
- struct pci_driver *drv = to_pci_driver(dev->driver);
+ if (drv && drv->suspend) {
+ pci_power_t prev = pci_dev->current_state;
+ int error;
- if (drv->suspend) {
- pci_power_t prev = pci_dev->current_state;
- int error;
+ error = drv->suspend(pci_dev, state);
+ suspend_report_result(drv->suspend, error);
+ if (error)
+ return error;
- error = drv->suspend(pci_dev, state);
- suspend_report_result(drv->suspend, error);
- if (error)
- return error;
-
- if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
- && pci_dev->current_state != PCI_UNKNOWN) {
- pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
- "PCI PM: Device state not saved by %pS\n",
- drv->suspend);
- }
+ if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
+ && pci_dev->current_state != PCI_UNKNOWN) {
+ pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
+ "PCI PM: Device state not saved by %pS\n",
+ drv->suspend);
}
}
@@ -632,17 +626,12 @@ static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
pci_fixup_device(pci_fixup_resume, pci_dev);
- if (pci_dev->dev.driver) {
- struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
-
- if (drv->resume)
- return drv->resume(pci_dev);
- }
-
- return pci_pm_reenable_device(pci_dev);
+ return drv && drv->resume ?
+ drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
}
/* Auxiliary functions used by the new power management framework */
@@ -656,14 +645,8 @@ static void pci_pm_default_suspend(struct pci_dev *pci_dev)
static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
{
- struct pci_driver *drv;
- bool ret;
-
- if (!pci_dev->dev.driver)
- return false;
-
- drv = to_pci_driver(pci_dev->dev.driver);
- ret = drv && (drv->suspend || drv->resume);
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+ bool ret = drv && (drv->suspend || drv->resume);
/*
* Legacy PM support is used by default, so warn if the new framework is
@@ -1255,11 +1238,11 @@ static int pci_pm_runtime_suspend(struct device *dev)
int error;
/*
- * If pci_dev->dev.driver is not set (unbound), we leave the device in D0,
- * but it may go to D3cold when the bridge above it runtime suspends.
- * Save its config space in case that happens.
+ * If the device has no driver, we leave it in D0, but it may go to
+ * D3cold when the bridge above it runtime suspends. Save its
+ * config space in case that happens.
*/
- if (!pci_dev->dev.driver) {
+ if (!to_pci_driver(dev->driver)) {
pci_save_state(pci_dev);
return 0;
}
@@ -1316,7 +1299,7 @@ static int pci_pm_runtime_resume(struct device *dev)
*/
pci_restore_standard_config(pci_dev);
- if (!dev->driver)
+ if (!to_pci_driver(dev->driver))
return 0;
pci_fixup_device(pci_fixup_resume_early, pci_dev);
@@ -1335,13 +1318,14 @@ static int pci_pm_runtime_resume(struct device *dev)
static int pci_pm_runtime_idle(struct device *dev)
{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
/*
- * If dev->driver is not set (unbound), the device should
- * always remain in D0 regardless of the runtime PM status
+ * If the device has no driver, it should always remain in D0
+ * regardless of the runtime PM status
*/
- if (!dev->driver)
+ if (!to_pci_driver(dev->driver))
return 0;
if (!pm)
@@ -1448,8 +1432,10 @@ static struct pci_driver pci_compat_driver = {
*/
struct pci_driver *pci_dev_driver(const struct pci_dev *dev)
{
- if (dev->dev.driver)
- return to_pci_driver(dev->dev.driver);
+ struct pci_driver *drv = to_pci_driver(dev->dev.driver);
+
+ if (drv)
+ return drv;
else {
int i;
for (i = 0; i <= PCI_ROM_RESOURCE; i++)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ccecf740de59..5298ce131f8c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5088,13 +5088,14 @@ EXPORT_SYMBOL_GPL(pci_dev_unlock);
static void pci_dev_save_and_disable(struct pci_dev *dev)
{
+ struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
- dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
+ drv ? drv->err_handler : NULL;
/*
- * dev->driver->err_handler->reset_prepare() is protected against
- * races with ->remove() by the device lock, which must be held by
- * the caller.
+ * drv->err_handler->reset_prepare() is protected against races
+ * with ->remove() by the device lock, which must be held by the
+ * caller.
*/
if (err_handler && err_handler->reset_prepare)
err_handler->reset_prepare(dev);
@@ -5119,15 +5120,15 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
static void pci_dev_restore(struct pci_dev *dev)
{
+ struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
- dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
+ drv ? drv->err_handler : NULL;
pci_restore_state(dev);
/*
- * dev->driver->err_handler->reset_done() is protected against
- * races with ->remove() by the device lock, which must be held by
- * the caller.
+ * drv->err_handler->reset_done() is protected against races with
+ * ->remove() by the device lock, which must be held by the caller.
*/
if (err_handler && err_handler->reset_done)
err_handler->reset_done(dev);
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index b314b54f7821..42385fe6b7fa 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -54,9 +54,10 @@ static int report_error_detected(struct pci_dev *dev,
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
+ pdrv = to_pci_driver(dev->dev.driver);
if (!pci_dev_set_io_state(dev, state) ||
- !dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->error_detected) {
/*
* If any device in the subtree does not have an error_detected
@@ -92,13 +93,14 @@ static int report_normal_detected(struct pci_dev *dev, void *data)
static int report_mmio_enabled(struct pci_dev *dev, void *data)
{
- pci_ers_result_t vote, *result = data;
struct pci_driver *pdrv;
+ pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
- if (!dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ pdrv = to_pci_driver(dev->dev.driver);
+ if (!pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->mmio_enabled)
goto out;
@@ -112,13 +114,14 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data)
static int report_slot_reset(struct pci_dev *dev, void *data)
{
+ struct pci_driver *pdrv;
pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
- struct pci_driver *pdrv;
device_lock(&dev->dev);
- if (!dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ pdrv = to_pci_driver(dev->dev.driver);
+ if (!pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->slot_reset)
goto out;
@@ -132,13 +135,14 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
static int report_resume(struct pci_dev *dev, void *data)
{
- const struct pci_error_handlers *err_handler;
struct pci_driver *pdrv;
+ const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
+ pdrv = dev->driver;
if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
- !dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->resume)
goto out;
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 73831fb87a1e..0ec76b4af16f 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -588,7 +588,6 @@ static pci_ers_result_t pcifront_common_process(int cmd,
struct pcifront_device *pdev,
pci_channel_state_t state)
{
- pci_ers_result_t result;
struct pci_driver *pdrv;
int bus = pdev->sh_info->aer_op.bus;
int devfn = pdev->sh_info->aer_op.devfn;
@@ -598,13 +597,12 @@ static pci_ers_result_t pcifront_common_process(int cmd,
dev_dbg(&pdev->xdev->dev,
"pcifront AER process: cmd %x (bus:%x, devfn%x)",
cmd, bus, devfn);
- result = PCI_ERS_RESULT_NONE;
pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
if (!pcidev || !pcidev->dev.driver) {
dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
pci_dev_put(pcidev);
- return result;
+ return PCI_ERS_RESULT_NONE;
}
pdrv = to_pci_driver(pcidev->dev.driver);
@@ -612,27 +610,18 @@ static pci_ers_result_t pcifront_common_process(int cmd,
pci_dbg(pcidev, "trying to call AER service\n");
switch (cmd) {
case XEN_PCI_OP_aer_detected:
- result = pdrv->err_handler->
- error_detected(pcidev, state);
- break;
+ return pdrv->err_handler->error_detected(pcidev, state);
case XEN_PCI_OP_aer_mmio:
- result = pdrv->err_handler->
- mmio_enabled(pcidev);
- break;
+ return pdrv->err_handler->mmio_enabled(pcidev);
case XEN_PCI_OP_aer_slotreset:
- result = pdrv->err_handler->
- slot_reset(pcidev);
- break;
+ return pdrv->err_handler->slot_reset(pcidev);
case XEN_PCI_OP_aer_resume:
pdrv->err_handler->resume(pcidev);
- break;
+ return PCI_ERS_RESULT_NONE;
default:
dev_err(&pdev->xdev->dev,
- "bad request in aer recovery "
- "operation!\n");
+ "bad request in AER recovery operation!\n");
}
-
- return result;
}
return PCI_ERS_RESULT_NONE;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 7c1ceb39035c..03bfdb25a55c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -899,7 +899,10 @@ struct pci_driver {
struct pci_dynids dynids;
};
-#define to_pci_driver(drv) container_of(drv, struct pci_driver, driver)
+static inline struct pci_driver *to_pci_driver(struct device_driver *drv)
+{
+ return drv ? container_of(drv, struct pci_driver, driver) : NULL;
+}
/**
* PCI_DEVICE - macro used to describe a specific PCI device
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-12 23:32 ` Bjorn Helgaas
0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-12 23:32 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, linux-pci, Alexander Duyck, x86, qat-linux,
oss-drivers, Oliver O'Halloran, H. Peter Anvin, Jiri Olsa,
Thomas Gleixner, Marco Chiappero, Stefano Stabellini, Herbert Xu,
linux-scsi, Rafał Miłecki, Jesse Brandeburg,
Peter Zijlstra, Ingo Molnar, linux-wireless, Jakub Kicinski,
Yisen Zhuang, Suganath Prabu Subramani, Fiona Trahe,
Andrew Donnellan, Arnd Bergmann, Konrad Rzeszutek Wilk,
Ido Schimmel, Simon Horman, linuxppc-dev,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Andy Shevchenko, Juergen Gross, Salil Mehta,
Sreekanth Reddy, xen-devel, Vadym Kochan, MPT-FusionLinux.pdl,
Greg Kroah-Hartman, linux-usb, Wojciech Ziemba, linux-kernel,
Mathias Nyman, Zhou Wang, linux-crypto, kernel, netdev,
Frederic Barrat, Paul Mackerras, Tomaszx Kowalik, Taras Chornyi,
David S. Miller, linux-perf-users
On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> Hello,
>
> this is v6 of the quest to drop the "driver" member from struct pci_dev
> which tracks the same data (apart from a constant offset) as dev.driver.
I like this a lot and applied it to pci/driver for v5.16, thanks!
I split some of the bigger patches apart so they only touched one
driver or subsystem at a time. I also updated to_pci_driver() so it
returns NULL when given NULL, which makes some of the validations
quite a bit simpler, especially in the PM code in pci-driver.c.
Full interdiff from this v6 series:
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index deaaef6efe34..36e84d904260 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -80,17 +80,15 @@ static struct resource video_rom_resource = {
*/
static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
{
+ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
const struct pci_device_id *id;
if (pdev->vendor == vendor && pdev->device == device)
return true;
- if (pdev->dev.driver) {
- struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
- for (id = drv->id_table; id && id->vendor; id++)
- if (id->vendor == vendor && id->device == device)
- break;
- }
+ for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
+ if (id->vendor == vendor && id->device == device)
+ break;
return id && id->vendor;
}
diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
index d997c9c3ebb5..7eb3706cf42d 100644
--- a/drivers/misc/cxl/guest.c
+++ b/drivers/misc/cxl/guest.c
@@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
pci_channel_state_t state)
{
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
if (afu->phb == NULL)
return;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
-
- if (!afu_dev->dev.driver)
- continue;
-
afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
+ err_handler = afu_drv->err_handler;
switch (bus_error_event) {
case CXL_ERROR_DETECTED_EVENT:
afu_dev->error_state = state;
- if (afu_drv->err_handler &&
- afu_drv->err_handler->error_detected)
- afu_drv->err_handler->error_detected(afu_dev, state);
- break;
+ if (err_handler &&
+ err_handler->error_detected)
+ err_handler->error_detected(afu_dev, state);
+ break;
case CXL_SLOT_RESET_EVENT:
afu_dev->error_state = state;
- if (afu_drv->err_handler &&
- afu_drv->err_handler->slot_reset)
- afu_drv->err_handler->slot_reset(afu_dev);
- break;
+ if (err_handler &&
+ err_handler->slot_reset)
+ err_handler->slot_reset(afu_dev);
+ break;
case CXL_RESUME_EVENT:
- if (afu_drv->err_handler &&
- afu_drv->err_handler->resume)
- afu_drv->err_handler->resume(afu_dev);
- break;
+ if (err_handler &&
+ err_handler->resume)
+ err_handler->resume(afu_dev);
+ break;
}
}
}
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 7e7545d01e27..08bd81854101 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1795,6 +1795,8 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
pci_channel_state_t state)
{
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
pci_ers_result_t afu_result = PCI_ERS_RESULT_NEED_RESET;
@@ -1805,16 +1807,16 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
return result;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
- if (!afu_dev->dev.driver)
- continue;
-
afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
afu_dev->error_state = state;
- if (afu_drv->err_handler)
- afu_result = afu_drv->err_handler->error_detected(afu_dev, state);
+ err_handler = afu_drv->err_handler;
+ if (err_handler)
+ afu_result = err_handler->error_detected(afu_dev,
+ state);
/* Disconnect trumps all, NONE trumps NEED_RESET */
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -1974,6 +1976,8 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
struct cxl_afu *afu;
struct cxl_context *ctx;
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
pci_ers_result_t afu_result = PCI_ERS_RESULT_RECOVERED;
pci_ers_result_t result = PCI_ERS_RESULT_RECOVERED;
int i;
@@ -2005,8 +2009,6 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
-
/* Reset the device context.
* TODO: make this less disruptive
*/
@@ -2032,14 +2034,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
* shouldn't start new work until we call
* their resume function.
*/
- if (!afu_dev->dev.driver)
- continue;
-
afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
- if (afu_drv->err_handler &&
- afu_drv->err_handler->slot_reset)
- afu_result = afu_drv->err_handler->slot_reset(afu_dev);
+ err_handler = afu_drv->err_handler;
+ if (err_handler && err_handler->slot_reset)
+ afu_result = err_handler->slot_reset(afu_dev);
if (afu_result == PCI_ERS_RESULT_DISCONNECT)
result = PCI_ERS_RESULT_DISCONNECT;
@@ -2066,6 +2067,8 @@ static void cxl_pci_resume(struct pci_dev *pdev)
struct cxl *adapter = pci_get_drvdata(pdev);
struct cxl_afu *afu;
struct pci_dev *afu_dev;
+ struct pci_driver *afu_drv;
+ struct pci_error_handlers *err_handler;
int i;
/* Everything is back now. Drivers should restart work now.
@@ -2080,11 +2083,13 @@ static void cxl_pci_resume(struct pci_dev *pdev)
continue;
list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
- struct pci_driver *afu_drv;
- if (afu_dev->dev.driver &&
- (afu_drv = to_pci_driver(afu_dev->dev.driver))->err_handler &&
- afu_drv->err_handler->resume)
- afu_drv->err_handler->resume(afu_dev);
+ afu_drv = to_pci_driver(afu_dev->dev.driver);
+ if (!afu_drv)
+ continue;
+
+ err_handler = afu_drv->err_handler;
+ if (err_handler && err_handler->resume)
+ err_handler->resume(afu_dev);
}
}
spin_unlock(&adapter->afu_list_lock);
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 0d0a34347868..fa4b52bb1e05 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -168,11 +168,8 @@ static ssize_t sriov_vf_total_msix_show(struct device *dev,
u32 vf_total_msix = 0;
device_lock(dev);
- if (!dev->driver)
- goto unlock;
-
pdrv = to_pci_driver(dev->driver);
- if (!pdrv->sriov_get_vf_total_msix)
+ if (!pdrv || !pdrv->sriov_get_vf_total_msix)
goto unlock;
vf_total_msix = pdrv->sriov_get_vf_total_msix(pdev);
@@ -199,19 +196,14 @@ static ssize_t sriov_vf_msix_count_store(struct device *dev,
return -EINVAL;
device_lock(&pdev->dev);
- if (!pdev->dev.driver) {
- ret = -EOPNOTSUPP;
- goto err_pdev;
- }
-
- pdrv = to_pci_driver(pdev->dev.driver);
- if (!pdrv->sriov_set_msix_vec_count) {
+ pdrv = to_pci_driver(dev->driver);
+ if (!pdrv || !pdrv->sriov_set_msix_vec_count) {
ret = -EOPNOTSUPP;
goto err_pdev;
}
device_lock(&vf_dev->dev);
- if (vf_dev->dev.driver) {
+ if (to_pci_driver(vf_dev->dev.driver)) {
/*
* A driver is already attached to this VF and has configured
* itself based on the current MSI-X vector count. Changing
@@ -405,14 +397,13 @@ static ssize_t sriov_numvfs_store(struct device *dev,
goto exit;
/* is PF driver loaded */
- if (!pdev->dev.driver) {
+ pdrv = to_pci_driver(dev->driver);
+ if (!pdrv) {
pci_info(pdev, "no driver bound to device; cannot configure SR-IOV\n");
ret = -ENOENT;
goto exit;
}
- pdrv = to_pci_driver(pdev->dev.driver);
-
/* is PF driver loaded w/callback */
if (!pdrv->sriov_configure) {
pci_info(pdev, "driver does not support SR-IOV configuration via sysfs\n");
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e94aa338bab4..3884a1542e86 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -454,7 +454,7 @@ static int pci_device_probe(struct device *dev)
static void pci_device_remove(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
if (drv->remove) {
pm_runtime_get_sync(dev);
@@ -489,15 +489,12 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
pm_runtime_resume(dev);
- if (pci_dev->dev.driver) {
- struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
-
- if (drv->shutdown)
- drv->shutdown(pci_dev);
- }
+ if (drv && drv->shutdown)
+ drv->shutdown(pci_dev);
/*
* If this is a kexec reboot, turn off Bus Master bit on the
@@ -588,25 +585,22 @@ static int pci_pm_reenable_device(struct pci_dev *pci_dev)
static int pci_legacy_suspend(struct device *dev, pm_message_t state)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
- if (dev->driver) {
- struct pci_driver *drv = to_pci_driver(dev->driver);
+ if (drv && drv->suspend) {
+ pci_power_t prev = pci_dev->current_state;
+ int error;
- if (drv->suspend) {
- pci_power_t prev = pci_dev->current_state;
- int error;
+ error = drv->suspend(pci_dev, state);
+ suspend_report_result(drv->suspend, error);
+ if (error)
+ return error;
- error = drv->suspend(pci_dev, state);
- suspend_report_result(drv->suspend, error);
- if (error)
- return error;
-
- if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
- && pci_dev->current_state != PCI_UNKNOWN) {
- pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
- "PCI PM: Device state not saved by %pS\n",
- drv->suspend);
- }
+ if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
+ && pci_dev->current_state != PCI_UNKNOWN) {
+ pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
+ "PCI PM: Device state not saved by %pS\n",
+ drv->suspend);
}
}
@@ -632,17 +626,12 @@ static int pci_legacy_suspend_late(struct device *dev, pm_message_t state)
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
+ struct pci_driver *drv = to_pci_driver(dev->driver);
pci_fixup_device(pci_fixup_resume, pci_dev);
- if (pci_dev->dev.driver) {
- struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
-
- if (drv->resume)
- return drv->resume(pci_dev);
- }
-
- return pci_pm_reenable_device(pci_dev);
+ return drv && drv->resume ?
+ drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
}
/* Auxiliary functions used by the new power management framework */
@@ -656,14 +645,8 @@ static void pci_pm_default_suspend(struct pci_dev *pci_dev)
static bool pci_has_legacy_pm_support(struct pci_dev *pci_dev)
{
- struct pci_driver *drv;
- bool ret;
-
- if (!pci_dev->dev.driver)
- return false;
-
- drv = to_pci_driver(pci_dev->dev.driver);
- ret = drv && (drv->suspend || drv->resume);
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+ bool ret = drv && (drv->suspend || drv->resume);
/*
* Legacy PM support is used by default, so warn if the new framework is
@@ -1255,11 +1238,11 @@ static int pci_pm_runtime_suspend(struct device *dev)
int error;
/*
- * If pci_dev->dev.driver is not set (unbound), we leave the device in D0,
- * but it may go to D3cold when the bridge above it runtime suspends.
- * Save its config space in case that happens.
+ * If the device has no driver, we leave it in D0, but it may go to
+ * D3cold when the bridge above it runtime suspends. Save its
+ * config space in case that happens.
*/
- if (!pci_dev->dev.driver) {
+ if (!to_pci_driver(dev->driver)) {
pci_save_state(pci_dev);
return 0;
}
@@ -1316,7 +1299,7 @@ static int pci_pm_runtime_resume(struct device *dev)
*/
pci_restore_standard_config(pci_dev);
- if (!dev->driver)
+ if (!to_pci_driver(dev->driver))
return 0;
pci_fixup_device(pci_fixup_resume_early, pci_dev);
@@ -1335,13 +1318,14 @@ static int pci_pm_runtime_resume(struct device *dev)
static int pci_pm_runtime_idle(struct device *dev)
{
+ struct pci_dev *pci_dev = to_pci_dev(dev);
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
/*
- * If dev->driver is not set (unbound), the device should
- * always remain in D0 regardless of the runtime PM status
+ * If the device has no driver, it should always remain in D0
+ * regardless of the runtime PM status
*/
- if (!dev->driver)
+ if (!to_pci_driver(dev->driver))
return 0;
if (!pm)
@@ -1448,8 +1432,10 @@ static struct pci_driver pci_compat_driver = {
*/
struct pci_driver *pci_dev_driver(const struct pci_dev *dev)
{
- if (dev->dev.driver)
- return to_pci_driver(dev->dev.driver);
+ struct pci_driver *drv = to_pci_driver(dev->dev.driver);
+
+ if (drv)
+ return drv;
else {
int i;
for (i = 0; i <= PCI_ROM_RESOURCE; i++)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ccecf740de59..5298ce131f8c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5088,13 +5088,14 @@ EXPORT_SYMBOL_GPL(pci_dev_unlock);
static void pci_dev_save_and_disable(struct pci_dev *dev)
{
+ struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
- dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
+ drv ? drv->err_handler : NULL;
/*
- * dev->driver->err_handler->reset_prepare() is protected against
- * races with ->remove() by the device lock, which must be held by
- * the caller.
+ * drv->err_handler->reset_prepare() is protected against races
+ * with ->remove() by the device lock, which must be held by the
+ * caller.
*/
if (err_handler && err_handler->reset_prepare)
err_handler->reset_prepare(dev);
@@ -5119,15 +5120,15 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
static void pci_dev_restore(struct pci_dev *dev)
{
+ struct pci_driver *drv = to_pci_driver(dev->dev.driver);
const struct pci_error_handlers *err_handler =
- dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
+ drv ? drv->err_handler : NULL;
pci_restore_state(dev);
/*
- * dev->driver->err_handler->reset_done() is protected against
- * races with ->remove() by the device lock, which must be held by
- * the caller.
+ * drv->err_handler->reset_done() is protected against races with
+ * ->remove() by the device lock, which must be held by the caller.
*/
if (err_handler && err_handler->reset_done)
err_handler->reset_done(dev);
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
index b314b54f7821..42385fe6b7fa 100644
--- a/drivers/pci/pcie/err.c
+++ b/drivers/pci/pcie/err.c
@@ -54,9 +54,10 @@ static int report_error_detected(struct pci_dev *dev,
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
+ pdrv = to_pci_driver(dev->dev.driver);
if (!pci_dev_set_io_state(dev, state) ||
- !dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->error_detected) {
/*
* If any device in the subtree does not have an error_detected
@@ -92,13 +93,14 @@ static int report_normal_detected(struct pci_dev *dev, void *data)
static int report_mmio_enabled(struct pci_dev *dev, void *data)
{
- pci_ers_result_t vote, *result = data;
struct pci_driver *pdrv;
+ pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
- if (!dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ pdrv = to_pci_driver(dev->dev.driver);
+ if (!pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->mmio_enabled)
goto out;
@@ -112,13 +114,14 @@ static int report_mmio_enabled(struct pci_dev *dev, void *data)
static int report_slot_reset(struct pci_dev *dev, void *data)
{
+ struct pci_driver *pdrv;
pci_ers_result_t vote, *result = data;
const struct pci_error_handlers *err_handler;
- struct pci_driver *pdrv;
device_lock(&dev->dev);
- if (!dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ pdrv = to_pci_driver(dev->dev.driver);
+ if (!pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->slot_reset)
goto out;
@@ -132,13 +135,14 @@ static int report_slot_reset(struct pci_dev *dev, void *data)
static int report_resume(struct pci_dev *dev, void *data)
{
- const struct pci_error_handlers *err_handler;
struct pci_driver *pdrv;
+ const struct pci_error_handlers *err_handler;
device_lock(&dev->dev);
+ pdrv = dev->driver;
if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
- !dev->dev.driver ||
- !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
+ !pdrv ||
+ !pdrv->err_handler ||
!pdrv->err_handler->resume)
goto out;
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index 73831fb87a1e..0ec76b4af16f 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -588,7 +588,6 @@ static pci_ers_result_t pcifront_common_process(int cmd,
struct pcifront_device *pdev,
pci_channel_state_t state)
{
- pci_ers_result_t result;
struct pci_driver *pdrv;
int bus = pdev->sh_info->aer_op.bus;
int devfn = pdev->sh_info->aer_op.devfn;
@@ -598,13 +597,12 @@ static pci_ers_result_t pcifront_common_process(int cmd,
dev_dbg(&pdev->xdev->dev,
"pcifront AER process: cmd %x (bus:%x, devfn%x)",
cmd, bus, devfn);
- result = PCI_ERS_RESULT_NONE;
pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
if (!pcidev || !pcidev->dev.driver) {
dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
pci_dev_put(pcidev);
- return result;
+ return PCI_ERS_RESULT_NONE;
}
pdrv = to_pci_driver(pcidev->dev.driver);
@@ -612,27 +610,18 @@ static pci_ers_result_t pcifront_common_process(int cmd,
pci_dbg(pcidev, "trying to call AER service\n");
switch (cmd) {
case XEN_PCI_OP_aer_detected:
- result = pdrv->err_handler->
- error_detected(pcidev, state);
- break;
+ return pdrv->err_handler->error_detected(pcidev, state);
case XEN_PCI_OP_aer_mmio:
- result = pdrv->err_handler->
- mmio_enabled(pcidev);
- break;
+ return pdrv->err_handler->mmio_enabled(pcidev);
case XEN_PCI_OP_aer_slotreset:
- result = pdrv->err_handler->
- slot_reset(pcidev);
- break;
+ return pdrv->err_handler->slot_reset(pcidev);
case XEN_PCI_OP_aer_resume:
pdrv->err_handler->resume(pcidev);
- break;
+ return PCI_ERS_RESULT_NONE;
default:
dev_err(&pdev->xdev->dev,
- "bad request in aer recovery "
- "operation!\n");
+ "bad request in AER recovery operation!\n");
}
-
- return result;
}
return PCI_ERS_RESULT_NONE;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 7c1ceb39035c..03bfdb25a55c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -899,7 +899,10 @@ struct pci_driver {
struct pci_dynids dynids;
};
-#define to_pci_driver(drv) container_of(drv, struct pci_driver, driver)
+static inline struct pci_driver *to_pci_driver(struct device_driver *drv)
+{
+ return drv ? container_of(drv, struct pci_driver, driver) : NULL;
+}
/**
* PCI_DEVICE - macro used to describe a specific PCI device
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-12 23:32 ` Bjorn Helgaas
@ 2021-10-13 8:51 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-13 8:51 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, linux-pci, Alexander Duyck, Russell Currey,
x86, qat-linux, oss-drivers, Oliver O'Halloran,
H. Peter Anvin, Jiri Olsa, Thomas Gleixner, Marco Chiappero,
Stefano Stabellini, Herbert Xu, linux-scsi, Michael Ellerman,
Rafał Miłecki, Jesse Brandeburg, Peter Zijlstra,
Ingo Molnar, Benjamin Herrenschmidt, linux-wireless,
Jakub Kicinski, Yisen Zhuang, Suganath Prabu Subramani,
Fiona Trahe, Andrew Donnellan, Arnd Bergmann,
Konrad Rzeszutek Wilk, Ido Schimmel, Simon Horman, linuxppc-dev,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Andy Shevchenko, Juergen Gross, Salil Mehta,
Sreekanth Reddy, xen-devel, Vadym Kochan, MPT-FusionLinux.pdl,
Greg Kroah-Hartman, linux-usb, Wojciech Ziemba, linux-kernel,
Mathias Nyman, Zhou Wang, linux-crypto, kernel, netdev,
Frederic Barrat, Paul Mackerras, Tomaszx Kowalik, Taras Chornyi,
David S. Miller, linux-perf-users
[-- Attachment #1: Type: text/plain, Size: 2395 bytes --]
On Tue, Oct 12, 2021 at 06:32:12PM -0500, Bjorn Helgaas wrote:
> On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> > Hello,
> >
> > this is v6 of the quest to drop the "driver" member from struct pci_dev
> > which tracks the same data (apart from a constant offset) as dev.driver.
>
> I like this a lot and applied it to pci/driver for v5.16, thanks!
>
> I split some of the bigger patches apart so they only touched one
> driver or subsystem at a time. I also updated to_pci_driver() so it
> returns NULL when given NULL, which makes some of the validations
> quite a bit simpler, especially in the PM code in pci-driver.c.
OK.
> Full interdiff from this v6 series:
>
> diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
> index deaaef6efe34..36e84d904260 100644
> --- a/arch/x86/kernel/probe_roms.c
> +++ b/arch/x86/kernel/probe_roms.c
> @@ -80,17 +80,15 @@ static struct resource video_rom_resource = {
> */
> static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> {
> + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> const struct pci_device_id *id;
>
> if (pdev->vendor == vendor && pdev->device == device)
> return true;
>
> - if (pdev->dev.driver) {
> - struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> - for (id = drv->id_table; id && id->vendor; id++)
> - if (id->vendor == vendor && id->device == device)
> - break;
> - }
> + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> + if (id->vendor == vendor && id->device == device)
> + break;
>
> return id && id->vendor;
> }
> diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> index d997c9c3ebb5..7eb3706cf42d 100644
> --- a/drivers/misc/cxl/guest.c
> +++ b/drivers/misc/cxl/guest.c
> @@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
> pci_channel_state_t state)
> {
> struct pci_dev *afu_dev;
> + struct pci_driver *afu_drv;
> + struct pci_error_handlers *err_handler;
These two could be moved into the for loop (where afu_drv was with my
patch already). This is also possible in a few other drivers.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-13 8:51 ` Uwe Kleine-König
0 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-13 8:51 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, x86, Alexander Shishkin,
linux-pci, Alexander Duyck, Jesse Brandeburg, Sathya Prakash,
oss-drivers, netdev, Oliver O'Halloran, H. Peter Anvin,
Jiri Olsa, Boris Ostrovsky, Paul Mackerras, Marco Chiappero,
Stefano Stabellini, Herbert Xu, linux-scsi, Ido Schimmel,
Rafał Miłecki, qat-linux, Peter Zijlstra, Ingo Molnar,
Jakub Kicinski, Yisen Zhuang, Frederic Barrat, Fiona Trahe,
Andrew Donnellan, Arnd Bergmann, Konrad Rzeszutek Wilk,
Suganath Prabu Subramani, Arnaldo Carvalho de Melo, Jack Xu,
Borislav Petkov, Michael Buesch, Jiri Pirko, Bjorn Helgaas,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Juergen Gross,
Salil Mehta, Sreekanth Reddy, xen-devel, Vadym Kochan,
MPT-FusionLinux.pdl, linux-usb, linux-wireless, linux-kernel,
David S. Miller, Mathias Nyman, Zhou Wang, linux-crypto, kernel,
Greg Kroah-Hartman, Simon Horman, Wojciech Ziemba,
Tomaszx Kowalik, linuxppc-dev, Taras Chornyi, linux-perf-users
[-- Attachment #1: Type: text/plain, Size: 2395 bytes --]
On Tue, Oct 12, 2021 at 06:32:12PM -0500, Bjorn Helgaas wrote:
> On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> > Hello,
> >
> > this is v6 of the quest to drop the "driver" member from struct pci_dev
> > which tracks the same data (apart from a constant offset) as dev.driver.
>
> I like this a lot and applied it to pci/driver for v5.16, thanks!
>
> I split some of the bigger patches apart so they only touched one
> driver or subsystem at a time. I also updated to_pci_driver() so it
> returns NULL when given NULL, which makes some of the validations
> quite a bit simpler, especially in the PM code in pci-driver.c.
OK.
> Full interdiff from this v6 series:
>
> diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
> index deaaef6efe34..36e84d904260 100644
> --- a/arch/x86/kernel/probe_roms.c
> +++ b/arch/x86/kernel/probe_roms.c
> @@ -80,17 +80,15 @@ static struct resource video_rom_resource = {
> */
> static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> {
> + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> const struct pci_device_id *id;
>
> if (pdev->vendor == vendor && pdev->device == device)
> return true;
>
> - if (pdev->dev.driver) {
> - struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> - for (id = drv->id_table; id && id->vendor; id++)
> - if (id->vendor == vendor && id->device == device)
> - break;
> - }
> + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> + if (id->vendor == vendor && id->device == device)
> + break;
>
> return id && id->vendor;
> }
> diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> index d997c9c3ebb5..7eb3706cf42d 100644
> --- a/drivers/misc/cxl/guest.c
> +++ b/drivers/misc/cxl/guest.c
> @@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
> pci_channel_state_t state)
> {
> struct pci_dev *afu_dev;
> + struct pci_driver *afu_drv;
> + struct pci_error_handlers *err_handler;
These two could be moved into the for loop (where afu_drv was with my
patch already). This is also possible in a few other drivers.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-13 8:51 ` Uwe Kleine-König
@ 2021-10-13 10:54 ` Bjorn Helgaas
-1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-13 10:54 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, linux-pci, Alexander Duyck, Russell Currey,
x86, qat-linux, oss-drivers, Oliver O'Halloran,
H. Peter Anvin, Jiri Olsa, Thomas Gleixner, Marco Chiappero,
Stefano Stabellini, Herbert Xu, linux-scsi, Michael Ellerman,
Rafał Miłecki, Jesse Brandeburg, Peter Zijlstra,
Ingo Molnar, Benjamin Herrenschmidt, linux-wireless,
Jakub Kicinski, Yisen Zhuang, Suganath Prabu Subramani,
Fiona Trahe, Andrew Donnellan, Arnd Bergmann,
Konrad Rzeszutek Wilk, Ido Schimmel, Simon Horman, linuxppc-dev,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Andy Shevchenko, Juergen Gross, Salil Mehta,
Sreekanth Reddy, xen-devel, Vadym Kochan, MPT-FusionLinux.pdl,
Greg Kroah-Hartman, linux-usb, Wojciech Ziemba, linux-kernel,
Mathias Nyman, Zhou Wang, linux-crypto, kernel, netdev,
Frederic Barrat, Paul Mackerras, Tomaszx Kowalik, Taras Chornyi,
David S. Miller, linux-perf-users
On Wed, Oct 13, 2021 at 10:51:31AM +0200, Uwe Kleine-König wrote:
> On Tue, Oct 12, 2021 at 06:32:12PM -0500, Bjorn Helgaas wrote:
> > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > this is v6 of the quest to drop the "driver" member from struct pci_dev
> > > which tracks the same data (apart from a constant offset) as dev.driver.
> >
> > I like this a lot and applied it to pci/driver for v5.16, thanks!
> >
> > I split some of the bigger patches apart so they only touched one
> > driver or subsystem at a time. I also updated to_pci_driver() so it
> > returns NULL when given NULL, which makes some of the validations
> > quite a bit simpler, especially in the PM code in pci-driver.c.
>
> OK.
>
> > Full interdiff from this v6 series:
> >
> > diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
> > index deaaef6efe34..36e84d904260 100644
> > --- a/arch/x86/kernel/probe_roms.c
> > +++ b/arch/x86/kernel/probe_roms.c
> > @@ -80,17 +80,15 @@ static struct resource video_rom_resource = {
> > */
> > static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> > {
> > + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> > const struct pci_device_id *id;
> >
> > if (pdev->vendor == vendor && pdev->device == device)
> > return true;
> >
> > - if (pdev->dev.driver) {
> > - struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> > - for (id = drv->id_table; id && id->vendor; id++)
> > - if (id->vendor == vendor && id->device == device)
> > - break;
> > - }
> > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> > + if (id->vendor == vendor && id->device == device)
> > + break;
> >
> > return id && id->vendor;
> > }
> > diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> > index d997c9c3ebb5..7eb3706cf42d 100644
> > --- a/drivers/misc/cxl/guest.c
> > +++ b/drivers/misc/cxl/guest.c
> > @@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
> > pci_channel_state_t state)
> > {
> > struct pci_dev *afu_dev;
> > + struct pci_driver *afu_drv;
> > + struct pci_error_handlers *err_handler;
>
> These two could be moved into the for loop (where afu_drv was with my
> patch already). This is also possible in a few other drivers.
That's true, they could. I tried to follow the prevailing style in
the file. At least in cxl, I didn't see any other cases of
declarations being in the minimal scope like that.
Bjorn
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-13 10:54 ` Bjorn Helgaas
0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-13 10:54 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Giovanni Cabiddu, Mark Rutland, x86, Alexander Shishkin,
linux-pci, Alexander Duyck, Jesse Brandeburg, Sathya Prakash,
oss-drivers, netdev, Oliver O'Halloran, H. Peter Anvin,
Jiri Olsa, Boris Ostrovsky, Paul Mackerras, Marco Chiappero,
Stefano Stabellini, Herbert Xu, linux-scsi, Ido Schimmel,
Rafał Miłecki, qat-linux, Peter Zijlstra, Ingo Molnar,
Jakub Kicinski, Yisen Zhuang, Frederic Barrat, Fiona Trahe,
Andrew Donnellan, Arnd Bergmann, Konrad Rzeszutek Wilk,
Suganath Prabu Subramani, Arnaldo Carvalho de Melo, Jack Xu,
Borislav Petkov, Michael Buesch, Jiri Pirko, Bjorn Helgaas,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Juergen Gross,
Salil Mehta, Sreekanth Reddy, xen-devel, Vadym Kochan,
MPT-FusionLinux.pdl, linux-usb, linux-wireless, linux-kernel,
David S. Miller, Mathias Nyman, Zhou Wang, linux-crypto, kernel,
Greg Kroah-Hartman, Simon Horman, Wojciech Ziemba,
Tomaszx Kowalik, linuxppc-dev, Taras Chornyi, linux-perf-users
On Wed, Oct 13, 2021 at 10:51:31AM +0200, Uwe Kleine-König wrote:
> On Tue, Oct 12, 2021 at 06:32:12PM -0500, Bjorn Helgaas wrote:
> > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > this is v6 of the quest to drop the "driver" member from struct pci_dev
> > > which tracks the same data (apart from a constant offset) as dev.driver.
> >
> > I like this a lot and applied it to pci/driver for v5.16, thanks!
> >
> > I split some of the bigger patches apart so they only touched one
> > driver or subsystem at a time. I also updated to_pci_driver() so it
> > returns NULL when given NULL, which makes some of the validations
> > quite a bit simpler, especially in the PM code in pci-driver.c.
>
> OK.
>
> > Full interdiff from this v6 series:
> >
> > diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
> > index deaaef6efe34..36e84d904260 100644
> > --- a/arch/x86/kernel/probe_roms.c
> > +++ b/arch/x86/kernel/probe_roms.c
> > @@ -80,17 +80,15 @@ static struct resource video_rom_resource = {
> > */
> > static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> > {
> > + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> > const struct pci_device_id *id;
> >
> > if (pdev->vendor == vendor && pdev->device == device)
> > return true;
> >
> > - if (pdev->dev.driver) {
> > - struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> > - for (id = drv->id_table; id && id->vendor; id++)
> > - if (id->vendor == vendor && id->device == device)
> > - break;
> > - }
> > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> > + if (id->vendor == vendor && id->device == device)
> > + break;
> >
> > return id && id->vendor;
> > }
> > diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> > index d997c9c3ebb5..7eb3706cf42d 100644
> > --- a/drivers/misc/cxl/guest.c
> > +++ b/drivers/misc/cxl/guest.c
> > @@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
> > pci_channel_state_t state)
> > {
> > struct pci_dev *afu_dev;
> > + struct pci_driver *afu_drv;
> > + struct pci_error_handlers *err_handler;
>
> These two could be moved into the for loop (where afu_drv was with my
> patch already). This is also possible in a few other drivers.
That's true, they could. I tried to follow the prevailing style in
the file. At least in cxl, I didn't see any other cases of
declarations being in the minimal scope like that.
Bjorn
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-13 10:54 ` Bjorn Helgaas
@ 2021-10-13 10:58 ` Uwe Kleine-König
-1 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-13 10:58 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, linux-pci, Alexander Duyck, Russell Currey,
Jesse Brandeburg, oss-drivers, netdev, Oliver O'Halloran,
H. Peter Anvin, Jiri Olsa, Boris Ostrovsky, Paul Mackerras,
Marco Chiappero, Stefano Stabellini, Herbert Xu, linux-scsi,
Michael Ellerman, Ido Schimmel, x86, qat-linux, Peter Zijlstra,
Ingo Molnar, Rafał Miłecki, Benjamin Herrenschmidt,
Jakub Kicinski, Yisen Zhuang, Frederic Barrat, Fiona Trahe,
Andrew Donnellan, Arnd Bergmann, Konrad Rzeszutek Wilk,
Suganath Prabu Subramani, Arnaldo Carvalho de Melo, Jack Xu,
Borislav Petkov, Michael Buesch, Jiri Pirko, Bjorn Helgaas,
Namhyung Kim, Thomas Gleixner, Andy Shevchenko, Juergen Gross,
Salil Mehta, Sreekanth Reddy, xen-devel, Vadym Kochan,
MPT-FusionLinux.pdl, linux-usb, linux-wireless, linux-kernel,
David S. Miller, Mathias Nyman, Zhou Wang, linux-crypto, kernel,
Greg Kroah-Hartman, Simon Horman, Wojciech Ziemba,
Tomaszx Kowalik, linuxppc-dev, Taras Chornyi, linux-perf-users
[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]
Hello,
On Wed, Oct 13, 2021 at 05:54:28AM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 13, 2021 at 10:51:31AM +0200, Uwe Kleine-König wrote:
> > On Tue, Oct 12, 2021 at 06:32:12PM -0500, Bjorn Helgaas wrote:
> > > diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> > > index d997c9c3ebb5..7eb3706cf42d 100644
> > > --- a/drivers/misc/cxl/guest.c
> > > +++ b/drivers/misc/cxl/guest.c
> > > @@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
> > > pci_channel_state_t state)
> > > {
> > > struct pci_dev *afu_dev;
> > > + struct pci_driver *afu_drv;
> > > + struct pci_error_handlers *err_handler;
> >
> > These two could be moved into the for loop (where afu_drv was with my
> > patch already). This is also possible in a few other drivers.
>
> That's true, they could. I tried to follow the prevailing style in
> the file. At least in cxl, I didn't see any other cases of
> declarations being in the minimal scope like that.
I don't care much, do whatever you consider nice. I'm happy you liked
the cleanup and that you took it.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-13 10:58 ` Uwe Kleine-König
0 siblings, 0 replies; 36+ messages in thread
From: Uwe Kleine-König @ 2021-10-13 10:58 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, x86, Alexander Shishkin,
linux-pci, Alexander Duyck, qat-linux, oss-drivers,
Oliver O'Halloran, H. Peter Anvin, Jiri Olsa,
Thomas Gleixner, Marco Chiappero, Stefano Stabellini, Herbert Xu,
linux-scsi, Sathya Prakash, Jesse Brandeburg, Peter Zijlstra,
Ingo Molnar, Rafał Miłecki, Jakub Kicinski,
Yisen Zhuang, Suganath Prabu Subramani, Fiona Trahe,
Andrew Donnellan, Arnd Bergmann, Konrad Rzeszutek Wilk,
Ido Schimmel, Simon Horman, Arnaldo Carvalho de Melo,
Wojciech Ziemba, Jack Xu, Borislav Petkov, Michael Buesch,
Jiri Pirko, Bjorn Helgaas, Namhyung Kim, Boris Ostrovsky,
Andy Shevchenko, Juergen Gross, Salil Mehta, Sreekanth Reddy,
xen-devel, Vadym Kochan, MPT-FusionLinux.pdl, Taras Chornyi,
netdev, linux-usb, linux-wireless, linux-kernel, Mathias Nyman,
Zhou Wang, linux-crypto, kernel, Greg Kroah-Hartman,
Frederic Barrat, Paul Mackerras, Tomaszx Kowalik, linuxppc-dev,
David S. Miller, linux-perf-users
[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]
Hello,
On Wed, Oct 13, 2021 at 05:54:28AM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 13, 2021 at 10:51:31AM +0200, Uwe Kleine-König wrote:
> > On Tue, Oct 12, 2021 at 06:32:12PM -0500, Bjorn Helgaas wrote:
> > > diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
> > > index d997c9c3ebb5..7eb3706cf42d 100644
> > > --- a/drivers/misc/cxl/guest.c
> > > +++ b/drivers/misc/cxl/guest.c
> > > @@ -20,38 +20,38 @@ static void pci_error_handlers(struct cxl_afu *afu,
> > > pci_channel_state_t state)
> > > {
> > > struct pci_dev *afu_dev;
> > > + struct pci_driver *afu_drv;
> > > + struct pci_error_handlers *err_handler;
> >
> > These two could be moved into the for loop (where afu_drv was with my
> > patch already). This is also possible in a few other drivers.
>
> That's true, they could. I tried to follow the prevailing style in
> the file. At least in cxl, I didn't see any other cases of
> declarations being in the minimal scope like that.
I don't care much, do whatever you consider nice. I'm happy you liked
the cleanup and that you took it.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-12 23:32 ` Bjorn Helgaas
@ 2021-10-13 9:26 ` Andy Shevchenko
-1 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2021-10-13 9:26 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Uwe Kleine-König, linux-pci, Sascha Hauer, Alexander Duyck,
Alexander Shishkin, Andrew Donnellan, Andy Shevchenko,
Arnaldo Carvalho de Melo, Arnd Bergmann, Benjamin Herrenschmidt,
Bjorn Helgaas, Borislav Petkov, Boris Ostrovsky, David S. Miller,
Fiona Trahe, Frederic Barrat, Giovanni Cabiddu,
Greg Kroah-Hartman, Herbert Xu, H. Peter Anvin, Ido Schimmel,
Ingo Molnar, Jack Xu, Jakub Kicinski, Jesse Brandeburg,
Jiri Olsa, Jiri Pirko, Juergen Gross, Konrad Rzeszutek Wilk,
Marco Chiappero, Mark Rutland, Mathias Nyman, Michael Buesch,
Michael Ellerman, Namhyung Kim, Oliver O'Halloran,
Paul Mackerras, Peter Zijlstra, Rafał Miłecki,
Russell Currey, Salil Mehta, Sathya Prakash, Simon Horman,
Sreekanth Reddy, Stefano Stabellini, Suganath Prabu Subramani,
Taras Chornyi, Thomas Gleixner, Tomaszx Kowalik, Vadym Kochan,
Wojciech Ziemba, Yisen Zhuang, Zhou Wang, linux-crypto,
Linux Kernel Mailing List, linux-perf-users,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, linux-scsi, USB,
open list:TI WILINK WIRELES...,
MPT-FusionLinux.pdl, netdev, oss-drivers, qat-linux,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
xen-devel
On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> I split some of the bigger patches apart so they only touched one
> driver or subsystem at a time. I also updated to_pci_driver() so it
> returns NULL when given NULL, which makes some of the validations
> quite a bit simpler, especially in the PM code in pci-driver.c.
It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.
Below are some comments as well.
...
> static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> {
> + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> const struct pci_device_id *id;
>
> if (pdev->vendor == vendor && pdev->device == device)
> return true;
> + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> + if (id->vendor == vendor && id->device == device)
> + break;
return true;
> return id && id->vendor;
return false;
> }
...
> + afu_result = err_handler->error_detected(afu_dev,
> + state);
One line?
...
> device_lock(&vf_dev->dev);
> - if (vf_dev->dev.driver) {
> + if (to_pci_driver(vf_dev->dev.driver)) {
Hmm...
...
> + if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
> + && pci_dev->current_state != PCI_UNKNOWN) {
Can we keep && on the previous line?
> + pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
> + "PCI PM: Device state not saved by %pS\n",
> + drv->suspend);
> }
...
> + return drv && drv->resume ?
> + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
One line?
...
> + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> const struct pci_error_handlers *err_handler =
> - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> + drv ? drv->err_handler : NULL;
Isn't dev->driver == to_pci_driver(dev->dev.driver)?
...
> + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> const struct pci_error_handlers *err_handler =
> - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> + drv ? drv->err_handler : NULL;
Ditto.
...
> device_lock(&dev->dev);
> + pdrv = to_pci_driver(dev->dev.driver);
> if (!pci_dev_set_io_state(dev, state) ||
> - !dev->dev.driver ||
> - !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
> + !pdrv ||
> + !pdrv->err_handler ||
One line now?
> !pdrv->err_handler->error_detected) {
Or this and the previous line?
...
> + pdrv = to_pci_driver(dev->dev.driver);
> + if (!pdrv ||
> + !pdrv->err_handler ||
> !pdrv->err_handler->mmio_enabled)
> goto out;
Ditto.
...
> + pdrv = to_pci_driver(dev->dev.driver);
> + if (!pdrv ||
> + !pdrv->err_handler ||
> !pdrv->err_handler->slot_reset)
> goto out;
Ditto.
...
> if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
> - !dev->dev.driver ||
> - !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
> + !pdrv ||
> + !pdrv->err_handler ||
> !pdrv->err_handler->resume)
> goto out;
Ditto.
> - result = PCI_ERS_RESULT_NONE;
>
> pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> if (!pcidev || !pcidev->dev.driver) {
> dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
> pci_dev_put(pcidev);
> - return result;
> + return PCI_ERS_RESULT_NONE;
> }
> pdrv = to_pci_driver(pcidev->dev.driver);
What about splitting the conditional to two with clear error message
in each and use pci_err() in the second one?
...
> default:
> dev_err(&pdev->xdev->dev,
> - "bad request in aer recovery "
> - "operation!\n");
> + "bad request in AER recovery operation!\n");
Stray change? Or is it in a separate patch in your tree?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-13 9:26 ` Andy Shevchenko
0 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2021-10-13 9:26 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, Alexander Duyck,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
qat-linux, oss-drivers, Oliver O'Halloran, H. Peter Anvin,
Jiri Olsa, Thomas Gleixner, Marco Chiappero, Stefano Stabellini,
Herbert Xu, linux-scsi, Rafał Miłecki,
Jesse Brandeburg, Peter Zijlstra, Ingo Molnar, linux-pci,
open list:TI WILINK WIRELES...,
Jakub Kicinski, Yisen Zhuang, Suganath Prabu Subramani,
Fiona Trahe, Andrew Donnellan, Arnd Bergmann,
Konrad Rzeszutek Wilk, Ido Schimmel, Uwe Kleine-König,
Simon Horman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Andy Shevchenko, Juergen Gross, Salil Mehta,
Sreekanth Reddy, xen-devel, Vadym Kochan, MPT-FusionLinux.pdl,
Greg Kroah-Hartman, USB, Wojciech Ziemba,
Linux Kernel Mailing List, Mathias Nyman, Zhou Wang,
linux-crypto, Sascha Hauer, netdev, Frederic Barrat,
Paul Mackerras, Tomaszx Kowalik, Taras Chornyi, David S. Miller,
linux-perf-users
On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> I split some of the bigger patches apart so they only touched one
> driver or subsystem at a time. I also updated to_pci_driver() so it
> returns NULL when given NULL, which makes some of the validations
> quite a bit simpler, especially in the PM code in pci-driver.c.
It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.
Below are some comments as well.
...
> static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> {
> + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> const struct pci_device_id *id;
>
> if (pdev->vendor == vendor && pdev->device == device)
> return true;
> + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> + if (id->vendor == vendor && id->device == device)
> + break;
return true;
> return id && id->vendor;
return false;
> }
...
> + afu_result = err_handler->error_detected(afu_dev,
> + state);
One line?
...
> device_lock(&vf_dev->dev);
> - if (vf_dev->dev.driver) {
> + if (to_pci_driver(vf_dev->dev.driver)) {
Hmm...
...
> + if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
> + && pci_dev->current_state != PCI_UNKNOWN) {
Can we keep && on the previous line?
> + pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
> + "PCI PM: Device state not saved by %pS\n",
> + drv->suspend);
> }
...
> + return drv && drv->resume ?
> + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
One line?
...
> + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> const struct pci_error_handlers *err_handler =
> - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> + drv ? drv->err_handler : NULL;
Isn't dev->driver == to_pci_driver(dev->dev.driver)?
...
> + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> const struct pci_error_handlers *err_handler =
> - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> + drv ? drv->err_handler : NULL;
Ditto.
...
> device_lock(&dev->dev);
> + pdrv = to_pci_driver(dev->dev.driver);
> if (!pci_dev_set_io_state(dev, state) ||
> - !dev->dev.driver ||
> - !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
> + !pdrv ||
> + !pdrv->err_handler ||
One line now?
> !pdrv->err_handler->error_detected) {
Or this and the previous line?
...
> + pdrv = to_pci_driver(dev->dev.driver);
> + if (!pdrv ||
> + !pdrv->err_handler ||
> !pdrv->err_handler->mmio_enabled)
> goto out;
Ditto.
...
> + pdrv = to_pci_driver(dev->dev.driver);
> + if (!pdrv ||
> + !pdrv->err_handler ||
> !pdrv->err_handler->slot_reset)
> goto out;
Ditto.
...
> if (!pci_dev_set_io_state(dev, pci_channel_io_normal) ||
> - !dev->dev.driver ||
> - !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
> + !pdrv ||
> + !pdrv->err_handler ||
> !pdrv->err_handler->resume)
> goto out;
Ditto.
> - result = PCI_ERS_RESULT_NONE;
>
> pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> if (!pcidev || !pcidev->dev.driver) {
> dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
> pci_dev_put(pcidev);
> - return result;
> + return PCI_ERS_RESULT_NONE;
> }
> pdrv = to_pci_driver(pcidev->dev.driver);
What about splitting the conditional to two with clear error message
in each and use pci_err() in the second one?
...
> default:
> dev_err(&pdev->xdev->dev,
> - "bad request in aer recovery "
> - "operation!\n");
> + "bad request in AER recovery operation!\n");
Stray change? Or is it in a separate patch in your tree?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-13 9:26 ` Andy Shevchenko
@ 2021-10-13 11:33 ` Bjorn Helgaas
-1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-13 11:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Uwe Kleine-König, linux-pci, Sascha Hauer, Alexander Duyck,
Alexander Shishkin, Andrew Donnellan, Andy Shevchenko,
Arnaldo Carvalho de Melo, Arnd Bergmann, Benjamin Herrenschmidt,
Bjorn Helgaas, Borislav Petkov, Boris Ostrovsky, David S. Miller,
Fiona Trahe, Frederic Barrat, Giovanni Cabiddu,
Greg Kroah-Hartman, Herbert Xu, H. Peter Anvin, Ido Schimmel,
Ingo Molnar, Jack Xu, Jakub Kicinski, Jesse Brandeburg,
Jiri Olsa, Jiri Pirko, Juergen Gross, Konrad Rzeszutek Wilk,
Marco Chiappero, Mark Rutland, Mathias Nyman, Michael Buesch,
Michael Ellerman, Namhyung Kim, Oliver O'Halloran,
Paul Mackerras, Peter Zijlstra, Rafał Miłecki,
Russell Currey, Salil Mehta, Sathya Prakash, Simon Horman,
Sreekanth Reddy, Stefano Stabellini, Suganath Prabu Subramani,
Taras Chornyi, Thomas Gleixner, Tomaszx Kowalik, Vadym Kochan,
Wojciech Ziemba, Yisen Zhuang, Zhou Wang, linux-crypto,
Linux Kernel Mailing List, linux-perf-users,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, linux-scsi, USB,
open list:TI WILINK WIRELES...,
MPT-FusionLinux.pdl, netdev, oss-drivers, qat-linux,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
xen-devel
On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
>
> > I split some of the bigger patches apart so they only touched one
> > driver or subsystem at a time. I also updated to_pci_driver() so it
> > returns NULL when given NULL, which makes some of the validations
> > quite a bit simpler, especially in the PM code in pci-driver.c.
>
> It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.
It is a little unusual. I only found three of 77 that are NULL-aware:
to_moxtet_driver()
to_siox_driver()
to_spi_driver()
It seems worthwhile to me because it makes the patch and the resulting
code significantly cleaner. Here's one example without the NULL
check:
@@ -493,12 +493,15 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
pm_runtime_resume(dev);
- if (drv && drv->shutdown)
- drv->shutdown(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->shutdown)
+ drv->shutdown(pci_dev);
+ }
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
pm_runtime_resume(dev);
if (pci_dev->dev.driver) {
struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
if (drv->shutdown)
drv->shutdown(pci_dev);
}
and here's the same thing with the NULL check:
@@ -493,7 +493,7 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
+ struct pci_driver *drv = to_pci_driver(dev->driver);
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
pm_runtime_resume(dev);
if (drv && drv->shutdown)
drv->shutdown(pci_dev);
> > static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> > {
> > + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> > const struct pci_device_id *id;
> >
> > if (pdev->vendor == vendor && pdev->device == device)
> > return true;
>
> > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> > + if (id->vendor == vendor && id->device == device)
>
> > + break;
>
> return true;
>
> > return id && id->vendor;
>
> return false;
Good cleanup for a follow-up patch, but doesn't seem directly related
to the objective here. The current patch is:
@@ -80,7 +80,7 @@ static struct resource video_rom_resource = {
*/
static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
{
- struct pci_driver *drv = pdev->driver;
+ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
const struct pci_device_id *id;
if (pdev->vendor == vendor && pdev->device == device)
> > device_lock(&vf_dev->dev);
> > - if (vf_dev->dev.driver) {
> > + if (to_pci_driver(vf_dev->dev.driver)) {
>
> Hmm...
Yeah, it could be either of:
if (to_pci_driver(vf_dev->dev.driver))
if (vf_dev->dev.driver)
I went back and forth on that and went with to_pci_driver() on the
theory that we were testing the pci_driver * before and the patch is
more of a mechanical change and easier to review if we test the
pci_driver * after.
> > + if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
>
> > + && pci_dev->current_state != PCI_UNKNOWN) {
>
> Can we keep && on the previous line?
I think this is in pci_legacy_suspend(), and I didn't touch that line.
It shows up in the interdiff because without the NULL check in
to_pci_driver(), we had to indent this code another level. With the
NULL check, we don't need that extra indentation.
> > + pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
> > + "PCI PM: Device state not saved by %pS\n",
> > + drv->suspend);
> > }
>
> ...
>
> > + return drv && drv->resume ?
> > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
>
> One line?
I don't think I touched that line.
> > + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> > const struct pci_error_handlers *err_handler =
> > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> > + drv ? drv->err_handler : NULL;
>
> Isn't dev->driver == to_pci_driver(dev->dev.driver)?
Yes, I think so, but not sure what you're getting at here, can you
elaborate?
> > device_lock(&dev->dev);
> > + pdrv = to_pci_driver(dev->dev.driver);
> > if (!pci_dev_set_io_state(dev, state) ||
> > - !dev->dev.driver ||
> > - !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
>
> > + !pdrv ||
> > + !pdrv->err_handler ||
>
> One line now?
>
> > !pdrv->err_handler->error_detected) {
>
> Or this and the previous line?
Could, but the "dev->driver" to "to_pci_driver(dev->dev.driver)"
changes are the heart of this patch, and I don't like to clutter it
with unrelated changes.
> > - result = PCI_ERS_RESULT_NONE;
> >
> > pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> > if (!pcidev || !pcidev->dev.driver) {
> > dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
> > pci_dev_put(pcidev);
> > - return result;
> > + return PCI_ERS_RESULT_NONE;
> > }
> > pdrv = to_pci_driver(pcidev->dev.driver);
>
> What about splitting the conditional to two with clear error message
> in each and use pci_err() in the second one?
Could possibly be cleaned up. Felt like feature creep so I didn't.
> > default:
> > dev_err(&pdev->xdev->dev,
> > - "bad request in aer recovery "
> > - "operation!\n");
> > + "bad request in AER recovery operation!\n");
>
> Stray change? Or is it in a separate patch in your tree?
Could be skipped. The string now fits on one line so I combined it to
make it more greppable.
Bjorn
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-13 11:33 ` Bjorn Helgaas
0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-13 11:33 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, Alexander Duyck,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
qat-linux, oss-drivers, Oliver O'Halloran, H. Peter Anvin,
Jiri Olsa, Thomas Gleixner, Marco Chiappero, Stefano Stabellini,
Herbert Xu, linux-scsi, Rafał Miłecki,
Jesse Brandeburg, Peter Zijlstra, Ingo Molnar, linux-pci,
open list:TI WILINK WIRELES...,
Jakub Kicinski, Yisen Zhuang, Suganath Prabu Subramani,
Fiona Trahe, Andrew Donnellan, Arnd Bergmann,
Konrad Rzeszutek Wilk, Ido Schimmel, Uwe Kleine-König,
Simon Horman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Andy Shevchenko, Juergen Gross, Salil Mehta,
Sreekanth Reddy, xen-devel, Vadym Kochan, MPT-FusionLinux.pdl,
Greg Kroah-Hartman, USB, Wojciech Ziemba,
Linux Kernel Mailing List, Mathias Nyman, Zhou Wang,
linux-crypto, Sascha Hauer, netdev, Frederic Barrat,
Paul Mackerras, Tomaszx Kowalik, Taras Chornyi, David S. Miller,
linux-perf-users
On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
>
> > I split some of the bigger patches apart so they only touched one
> > driver or subsystem at a time. I also updated to_pci_driver() so it
> > returns NULL when given NULL, which makes some of the validations
> > quite a bit simpler, especially in the PM code in pci-driver.c.
>
> It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.
It is a little unusual. I only found three of 77 that are NULL-aware:
to_moxtet_driver()
to_siox_driver()
to_spi_driver()
It seems worthwhile to me because it makes the patch and the resulting
code significantly cleaner. Here's one example without the NULL
check:
@@ -493,12 +493,15 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
pm_runtime_resume(dev);
- if (drv && drv->shutdown)
- drv->shutdown(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->shutdown)
+ drv->shutdown(pci_dev);
+ }
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
pm_runtime_resume(dev);
if (pci_dev->dev.driver) {
struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
if (drv->shutdown)
drv->shutdown(pci_dev);
}
and here's the same thing with the NULL check:
@@ -493,7 +493,7 @@ static void pci_device_remove(struct device *dev)
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
+ struct pci_driver *drv = to_pci_driver(dev->driver);
static void pci_device_shutdown(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
pm_runtime_resume(dev);
if (drv && drv->shutdown)
drv->shutdown(pci_dev);
> > static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
> > {
> > + struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
> > const struct pci_device_id *id;
> >
> > if (pdev->vendor == vendor && pdev->device == device)
> > return true;
>
> > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> > + if (id->vendor == vendor && id->device == device)
>
> > + break;
>
> return true;
>
> > return id && id->vendor;
>
> return false;
Good cleanup for a follow-up patch, but doesn't seem directly related
to the objective here. The current patch is:
@@ -80,7 +80,7 @@ static struct resource video_rom_resource = {
*/
static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short device)
{
- struct pci_driver *drv = pdev->driver;
+ struct pci_driver *drv = to_pci_driver(pdev->dev.driver);
const struct pci_device_id *id;
if (pdev->vendor == vendor && pdev->device == device)
> > device_lock(&vf_dev->dev);
> > - if (vf_dev->dev.driver) {
> > + if (to_pci_driver(vf_dev->dev.driver)) {
>
> Hmm...
Yeah, it could be either of:
if (to_pci_driver(vf_dev->dev.driver))
if (vf_dev->dev.driver)
I went back and forth on that and went with to_pci_driver() on the
theory that we were testing the pci_driver * before and the patch is
more of a mechanical change and easier to review if we test the
pci_driver * after.
> > + if (!pci_dev->state_saved && pci_dev->current_state != PCI_D0
>
> > + && pci_dev->current_state != PCI_UNKNOWN) {
>
> Can we keep && on the previous line?
I think this is in pci_legacy_suspend(), and I didn't touch that line.
It shows up in the interdiff because without the NULL check in
to_pci_driver(), we had to indent this code another level. With the
NULL check, we don't need that extra indentation.
> > + pci_WARN_ONCE(pci_dev, pci_dev->current_state != prev,
> > + "PCI PM: Device state not saved by %pS\n",
> > + drv->suspend);
> > }
>
> ...
>
> > + return drv && drv->resume ?
> > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
>
> One line?
I don't think I touched that line.
> > + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> > const struct pci_error_handlers *err_handler =
> > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> > + drv ? drv->err_handler : NULL;
>
> Isn't dev->driver == to_pci_driver(dev->dev.driver)?
Yes, I think so, but not sure what you're getting at here, can you
elaborate?
> > device_lock(&dev->dev);
> > + pdrv = to_pci_driver(dev->dev.driver);
> > if (!pci_dev_set_io_state(dev, state) ||
> > - !dev->dev.driver ||
> > - !(pdrv = to_pci_driver(dev->dev.driver))->err_handler ||
>
> > + !pdrv ||
> > + !pdrv->err_handler ||
>
> One line now?
>
> > !pdrv->err_handler->error_detected) {
>
> Or this and the previous line?
Could, but the "dev->driver" to "to_pci_driver(dev->dev.driver)"
changes are the heart of this patch, and I don't like to clutter it
with unrelated changes.
> > - result = PCI_ERS_RESULT_NONE;
> >
> > pcidev = pci_get_domain_bus_and_slot(domain, bus, devfn);
> > if (!pcidev || !pcidev->dev.driver) {
> > dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
> > pci_dev_put(pcidev);
> > - return result;
> > + return PCI_ERS_RESULT_NONE;
> > }
> > pdrv = to_pci_driver(pcidev->dev.driver);
>
> What about splitting the conditional to two with clear error message
> in each and use pci_err() in the second one?
Could possibly be cleaned up. Felt like feature creep so I didn't.
> > default:
> > dev_err(&pdev->xdev->dev,
> > - "bad request in aer recovery "
> > - "operation!\n");
> > + "bad request in AER recovery operation!\n");
>
> Stray change? Or is it in a separate patch in your tree?
Could be skipped. The string now fits on one line so I combined it to
make it more greppable.
Bjorn
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-13 11:33 ` Bjorn Helgaas
@ 2021-10-13 13:23 ` Andy Shevchenko
-1 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2021-10-13 13:23 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Uwe Kleine-König, linux-pci, Sascha Hauer, Alexander Duyck,
Alexander Shishkin, Andrew Donnellan, Arnaldo Carvalho de Melo,
Arnd Bergmann, Benjamin Herrenschmidt, Bjorn Helgaas,
Borislav Petkov, Boris Ostrovsky, David S. Miller, Fiona Trahe,
Frederic Barrat, Giovanni Cabiddu, Greg Kroah-Hartman,
Herbert Xu, H. Peter Anvin, Ido Schimmel, Ingo Molnar, Jack Xu,
Jakub Kicinski, Jesse Brandeburg, Jiri Olsa, Jiri Pirko,
Juergen Gross, Konrad Rzeszutek Wilk, Marco Chiappero,
Mark Rutland, Mathias Nyman, Michael Buesch, Michael Ellerman,
Namhyung Kim, Oliver O'Halloran, Paul Mackerras,
Peter Zijlstra, Rafał Miłecki, Russell Currey,
Salil Mehta, Sathya Prakash, Simon Horman, Sreekanth Reddy,
Stefano Stabellini, Suganath Prabu Subramani, Taras Chornyi,
Thomas Gleixner, Tomaszx Kowalik, Vadym Kochan, Wojciech Ziemba,
Yisen Zhuang, Zhou Wang, linux-crypto, Linux Kernel Mailing List,
linux-perf-users, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
linux-scsi, USB, open list:TI WILINK WIRELES...,
MPT-FusionLinux.pdl, netdev, oss-drivers, qat-linux,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
xen-devel
On Wed, Oct 13, 2021 at 06:33:56AM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
...
> > It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.
>
> It is a little unusual. I only found three of 77 that are NULL-aware:
>
> to_moxtet_driver()
> to_siox_driver()
> to_spi_driver()
>
> It seems worthwhile to me because it makes the patch and the resulting
> code significantly cleaner.
I'm not objecting the change, just a remark.
...
> > > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> > > + if (id->vendor == vendor && id->device == device)
> >
> > > + break;
> >
> > return true;
> >
> > > return id && id->vendor;
> >
> > return false;
>
> Good cleanup for a follow-up patch, but doesn't seem directly related
> to the objective here.
True. Maybe you can bake one while not forgotten?
...
> > > + return drv && drv->resume ?
> > > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
> >
> > One line?
>
> I don't think I touched that line.
Then why they are both in + section?
...
> > > + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> > > const struct pci_error_handlers *err_handler =
> > > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> > > + drv ? drv->err_handler : NULL;
> >
> > Isn't dev->driver == to_pci_driver(dev->dev.driver)?
>
> Yes, I think so, but not sure what you're getting at here, can you
> elaborate?
Getting pointer from another pointer seems waste of resources, why we
can't simply
struct pci_driver *drv = dev->driver;
?
...
> > Stray change? Or is it in a separate patch in your tree?
>
> Could be skipped. The string now fits on one line so I combined it to
> make it more greppable.
This is inconsistency in your changes, in one case you are objecting of
doing something close to the changed lines, in the other you are doing
unrelated change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-13 13:23 ` Andy Shevchenko
0 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2021-10-13 13:23 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, Alexander Duyck,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
qat-linux, oss-drivers, Oliver O'Halloran, H. Peter Anvin,
Jiri Olsa, Thomas Gleixner, Marco Chiappero, Stefano Stabellini,
Herbert Xu, linux-scsi, Rafał Miłecki,
Jesse Brandeburg, Peter Zijlstra, Ingo Molnar, linux-pci,
open list:TI WILINK WIRELES...,
Jakub Kicinski, Yisen Zhuang, Suganath Prabu Subramani,
Fiona Trahe, Andrew Donnellan, Arnd Bergmann,
Konrad Rzeszutek Wilk, Ido Schimmel, Uwe Kleine-König,
Simon Horman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Juergen Gross, Salil Mehta, Sreekanth Reddy,
xen-devel, Vadym Kochan, MPT-FusionLinux.pdl, Greg Kroah-Hartman,
USB, Wojciech Ziemba, Linux Kernel Mailing List, Mathias Nyman,
Zhou Wang, linux-crypto, Sascha Hauer, netdev, Frederic Barrat,
Paul Mackerras, Tomaszx Kowalik, Taras Chornyi, David S. Miller,
linux-perf-users
On Wed, Oct 13, 2021 at 06:33:56AM -0500, Bjorn Helgaas wrote:
> On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
...
> > It's a bit unusual. Other to_*_dev() are not NULL-aware IIRC.
>
> It is a little unusual. I only found three of 77 that are NULL-aware:
>
> to_moxtet_driver()
> to_siox_driver()
> to_spi_driver()
>
> It seems worthwhile to me because it makes the patch and the resulting
> code significantly cleaner.
I'm not objecting the change, just a remark.
...
> > > + for (id = drv ? drv->id_table : NULL; id && id->vendor; id++)
> > > + if (id->vendor == vendor && id->device == device)
> >
> > > + break;
> >
> > return true;
> >
> > > return id && id->vendor;
> >
> > return false;
>
> Good cleanup for a follow-up patch, but doesn't seem directly related
> to the objective here.
True. Maybe you can bake one while not forgotten?
...
> > > + return drv && drv->resume ?
> > > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
> >
> > One line?
>
> I don't think I touched that line.
Then why they are both in + section?
...
> > > + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> > > const struct pci_error_handlers *err_handler =
> > > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> > > + drv ? drv->err_handler : NULL;
> >
> > Isn't dev->driver == to_pci_driver(dev->dev.driver)?
>
> Yes, I think so, but not sure what you're getting at here, can you
> elaborate?
Getting pointer from another pointer seems waste of resources, why we
can't simply
struct pci_driver *drv = dev->driver;
?
...
> > Stray change? Or is it in a separate patch in your tree?
>
> Could be skipped. The string now fits on one line so I combined it to
> make it more greppable.
This is inconsistency in your changes, in one case you are objecting of
doing something close to the changed lines, in the other you are doing
unrelated change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-13 13:23 ` Andy Shevchenko
@ 2021-10-15 16:46 ` Bjorn Helgaas
-1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-15 16:46 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Uwe Kleine-König, linux-pci, Sascha Hauer, Alexander Duyck,
Alexander Shishkin, Andrew Donnellan, Arnaldo Carvalho de Melo,
Arnd Bergmann, Benjamin Herrenschmidt, Bjorn Helgaas,
Borislav Petkov, Boris Ostrovsky, David S. Miller, Fiona Trahe,
Frederic Barrat, Giovanni Cabiddu, Greg Kroah-Hartman,
Herbert Xu, H. Peter Anvin, Ido Schimmel, Ingo Molnar, Jack Xu,
Jakub Kicinski, Jesse Brandeburg, Jiri Olsa, Jiri Pirko,
Juergen Gross, Konrad Rzeszutek Wilk, Marco Chiappero,
Mark Rutland, Mathias Nyman, Michael Buesch, Michael Ellerman,
Namhyung Kim, Oliver O'Halloran, Paul Mackerras,
Peter Zijlstra, Rafał Miłecki, Russell Currey,
Salil Mehta, Sathya Prakash, Simon Horman, Sreekanth Reddy,
Stefano Stabellini, Suganath Prabu Subramani, Taras Chornyi,
Thomas Gleixner, Tomaszx Kowalik, Vadym Kochan, Wojciech Ziemba,
Yisen Zhuang, Zhou Wang, linux-crypto, Linux Kernel Mailing List,
linux-perf-users, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
linux-scsi, USB, open list:TI WILINK WIRELES...,
MPT-FusionLinux.pdl, netdev, oss-drivers, qat-linux,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
xen-devel
On Wed, Oct 13, 2021 at 04:23:09PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 13, 2021 at 06:33:56AM -0500, Bjorn Helgaas wrote:
> > On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:
> > > On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> > > > + return drv && drv->resume ?
> > > > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
> > >
> > > One line?
> >
> > I don't think I touched that line.
>
> Then why they are both in + section?
They're both in the + section of the interdiff because Uwe's v6 patch
looks like this:
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return drv && drv->resume ?
- drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->resume)
+ return drv->resume(pci_dev);
+ }
+
+ return pci_pm_reenable_device(pci_dev);
and my revision looks like this:
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
+ struct pci_driver *drv = to_pci_driver(dev->driver);
so compared to Uwe's v6, I restored that section to the original code.
My goal here was to make the patch as simple and easy to review as
possible.
> > > > + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> > > > const struct pci_error_handlers *err_handler =
> > > > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> > > > + drv ? drv->err_handler : NULL;
> > >
> > > Isn't dev->driver == to_pci_driver(dev->dev.driver)?
> >
> > Yes, I think so, but not sure what you're getting at here, can you
> > elaborate?
>
> Getting pointer from another pointer seems waste of resources, why we
> can't simply
>
> struct pci_driver *drv = dev->driver;
I think this is in pci_dev_save_and_disable(), and "dev" here is a
struct pci_dev *. We're removing the dev->driver member. Let me know
if I'm still missing something.
> > > > - "bad request in aer recovery "
> > > > - "operation!\n");
> > > > + "bad request in AER recovery operation!\n");
> > > Stray change? Or is it in a separate patch in your tree?
> >
> > Could be skipped. The string now fits on one line so I combined it to
> > make it more greppable.
>
> This is inconsistency in your changes, in one case you are objecting of
> doing something close to the changed lines, in the other you are doing
> unrelated change.
You're right, this didn't make much sense in that patch. I moved the
line join to the previous patch, which unindented this section and
made space for this to fit on one line. Here's the revised commit:
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=34ab316d7287
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-15 16:46 ` Bjorn Helgaas
0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2021-10-15 16:46 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, Alexander Duyck,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
qat-linux, oss-drivers, Oliver O'Halloran, H. Peter Anvin,
Jiri Olsa, Thomas Gleixner, Marco Chiappero, Stefano Stabellini,
Herbert Xu, linux-scsi, Rafał Miłecki,
Jesse Brandeburg, Peter Zijlstra, Ingo Molnar, linux-pci,
open list:TI WILINK WIRELES...,
Jakub Kicinski, Yisen Zhuang, Suganath Prabu Subramani,
Fiona Trahe, Andrew Donnellan, Arnd Bergmann,
Konrad Rzeszutek Wilk, Ido Schimmel, Uwe Kleine-König,
Simon Horman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Juergen Gross, Salil Mehta, Sreekanth Reddy,
xen-devel, Vadym Kochan, MPT-FusionLinux.pdl, Greg Kroah-Hartman,
USB, Wojciech Ziemba, Linux Kernel Mailing List, Mathias Nyman,
Zhou Wang, linux-crypto, Sascha Hauer, netdev, Frederic Barrat,
Paul Mackerras, Tomaszx Kowalik, Taras Chornyi, David S. Miller,
linux-perf-users
On Wed, Oct 13, 2021 at 04:23:09PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 13, 2021 at 06:33:56AM -0500, Bjorn Helgaas wrote:
> > On Wed, Oct 13, 2021 at 12:26:42PM +0300, Andy Shevchenko wrote:
> > > On Wed, Oct 13, 2021 at 2:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > > On Mon, Oct 04, 2021 at 02:59:24PM +0200, Uwe Kleine-König wrote:
> > > > + return drv && drv->resume ?
> > > > + drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
> > >
> > > One line?
> >
> > I don't think I touched that line.
>
> Then why they are both in + section?
They're both in the + section of the interdiff because Uwe's v6 patch
looks like this:
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return drv && drv->resume ?
- drv->resume(pci_dev) : pci_pm_reenable_device(pci_dev);
+ if (pci_dev->dev.driver) {
+ struct pci_driver *drv = to_pci_driver(pci_dev->dev.driver);
+
+ if (drv->resume)
+ return drv->resume(pci_dev);
+ }
+
+ return pci_pm_reenable_device(pci_dev);
and my revision looks like this:
static int pci_legacy_resume(struct device *dev)
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- struct pci_driver *drv = pci_dev->driver;
+ struct pci_driver *drv = to_pci_driver(dev->driver);
so compared to Uwe's v6, I restored that section to the original code.
My goal here was to make the patch as simple and easy to review as
possible.
> > > > + struct pci_driver *drv = to_pci_driver(dev->dev.driver);
> > > > const struct pci_error_handlers *err_handler =
> > > > - dev->dev.driver ? to_pci_driver(dev->dev.driver)->err_handler : NULL;
> > > > + drv ? drv->err_handler : NULL;
> > >
> > > Isn't dev->driver == to_pci_driver(dev->dev.driver)?
> >
> > Yes, I think so, but not sure what you're getting at here, can you
> > elaborate?
>
> Getting pointer from another pointer seems waste of resources, why we
> can't simply
>
> struct pci_driver *drv = dev->driver;
I think this is in pci_dev_save_and_disable(), and "dev" here is a
struct pci_dev *. We're removing the dev->driver member. Let me know
if I'm still missing something.
> > > > - "bad request in aer recovery "
> > > > - "operation!\n");
> > > > + "bad request in AER recovery operation!\n");
> > > Stray change? Or is it in a separate patch in your tree?
> >
> > Could be skipped. The string now fits on one line so I combined it to
> > make it more greppable.
>
> This is inconsistency in your changes, in one case you are objecting of
> doing something close to the changed lines, in the other you are doing
> unrelated change.
You're right, this didn't make much sense in that patch. I moved the
line join to the previous patch, which unindented this section and
made space for this to fit on one line. Here's the revised commit:
https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=34ab316d7287
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
2021-10-15 16:46 ` Bjorn Helgaas
@ 2021-10-15 19:52 ` Andy Shevchenko
-1 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2021-10-15 19:52 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Uwe Kleine-König, linux-pci, Sascha Hauer, Alexander Duyck,
Alexander Shishkin, Andrew Donnellan, Arnaldo Carvalho de Melo,
Arnd Bergmann, Benjamin Herrenschmidt, Bjorn Helgaas,
Borislav Petkov, Boris Ostrovsky, David S. Miller, Fiona Trahe,
Frederic Barrat, Giovanni Cabiddu, Greg Kroah-Hartman,
Herbert Xu, H. Peter Anvin, Ido Schimmel, Ingo Molnar, Jack Xu,
Jakub Kicinski, Jesse Brandeburg, Jiri Olsa, Jiri Pirko,
Juergen Gross, Konrad Rzeszutek Wilk, Marco Chiappero,
Mark Rutland, Mathias Nyman, Michael Buesch, Michael Ellerman,
Namhyung Kim, Oliver O'Halloran, Paul Mackerras,
Peter Zijlstra, Rafał Miłecki, Russell Currey,
Salil Mehta, Sathya Prakash, Simon Horman, Sreekanth Reddy,
Stefano Stabellini, Suganath Prabu Subramani, Taras Chornyi,
Thomas Gleixner, Tomaszx Kowalik, Vadym Kochan, Wojciech Ziemba,
Yisen Zhuang, Zhou Wang, linux-crypto, Linux Kernel Mailing List,
linux-perf-users, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
linux-scsi, USB, open list:TI WILINK WIRELES...,
MPT-FusionLinux.pdl, netdev, oss-drivers, qat-linux,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
xen-devel
On Fri, Oct 15, 2021 at 7:46 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Oct 13, 2021 at 04:23:09PM +0300, Andy Shevchenko wrote:
...
> so compared to Uwe's v6, I restored that section to the original code.
> My goal here was to make the patch as simple and easy to review as
> possible.
Thanks for elaboration. I have got it.
...
> You're right, this didn't make much sense in that patch. I moved the
> line join to the previous patch, which unindented this section and
> made space for this to fit on one line. Here's the revised commit:
>
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=34ab316d7287
Side remark: default without break or return is error prone (okay, to
some extent). Perhaps adding the return statement there will make
things robust and clean.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v6 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-10-15 19:52 ` Andy Shevchenko
0 siblings, 0 replies; 36+ messages in thread
From: Andy Shevchenko @ 2021-10-15 19:52 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Giovanni Cabiddu, Mark Rutland, Sathya Prakash,
Alexander Shishkin, Alexander Duyck,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
qat-linux, oss-drivers, Oliver O'Halloran, H. Peter Anvin,
Jiri Olsa, Thomas Gleixner, Marco Chiappero, Stefano Stabellini,
Herbert Xu, linux-scsi, Rafał Miłecki,
Jesse Brandeburg, Peter Zijlstra, Ingo Molnar, linux-pci,
open list:TI WILINK WIRELES...,
Jakub Kicinski, Yisen Zhuang, Suganath Prabu Subramani,
Fiona Trahe, Andrew Donnellan, Arnd Bergmann,
Konrad Rzeszutek Wilk, Ido Schimmel, Uwe Kleine-König,
Simon Horman, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
Arnaldo Carvalho de Melo, Jack Xu, Borislav Petkov,
Michael Buesch, Jiri Pirko, Bjorn Helgaas, Namhyung Kim,
Boris Ostrovsky, Juergen Gross, Salil Mehta, Sreekanth Reddy,
xen-devel, Vadym Kochan, MPT-FusionLinux.pdl, Greg Kroah-Hartman,
USB, Wojciech Ziemba, Linux Kernel Mailing List, Mathias Nyman,
Zhou Wang, linux-crypto, Sascha Hauer, netdev, Frederic Barrat,
Paul Mackerras, Tomaszx Kowalik, Taras Chornyi, David S. Miller,
linux-perf-users
On Fri, Oct 15, 2021 at 7:46 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, Oct 13, 2021 at 04:23:09PM +0300, Andy Shevchenko wrote:
...
> so compared to Uwe's v6, I restored that section to the original code.
> My goal here was to make the patch as simple and easy to review as
> possible.
Thanks for elaboration. I have got it.
...
> You're right, this didn't make much sense in that patch. I moved the
> line join to the previous patch, which unindented this section and
> made space for this to fit on one line. Here's the revised commit:
>
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=34ab316d7287
Side remark: default without break or return is error prone (okay, to
some extent). Perhaps adding the return statement there will make
things robust and clean.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 36+ messages in thread