* [PATCH v5 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver
@ 2021-09-29 8:52 Uwe Kleine-König
2021-09-29 8:53 ` [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver Uwe Kleine-König
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2021-09-29 8:52 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Christoph Hellwig, 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
Hello,
this is v5 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 v4:
- split some changes out of "PCI: replace pci_dev::driver usage that
gets the driver name" and reworked them a bit as suggested by Bjorn.
- Add a line break in
drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c to please
checkpatch --strict and Simon Horman.
- Fixed a build problem in "crypto: qat - simplify adf_enable_aer()",
thanks to Giovanni Cabiddu for spotting and a suggested fix.
I didn't replace :: by . as suggested by Bjorn as I'm unsure if his
preference is stronger than mine.
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 | 2 +-
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 | 25 +++++---
drivers/pci/pci-driver.c | 45 ++++++-------
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, 161 insertions(+), 175 deletions(-)
Range-diff against v4:
1: 8d064bbc74b0 = 1: c2b53ab26a6b PCI: Simplify pci_device_remove()
2: 966b49c308b4 = 2: 2c733e1d5186 PCI: Drop useless check from pci_device_probe()
3: ce710d6e8a1b = 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
-: ------------ > 6: abd70de9782d ssb: Simplify determination of driver name
4: 3e4e6994a59d ! 7: 735845bd26b9 PCI: replace pci_dev::driver usage that gets the driver name
@@ Metadata
Author: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
## Commit message ##
- PCI: replace pci_dev::driver usage that gets the driver name
+ PCI: Replace pci_dev::driver usage that gets the driver name
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
@@ Commit message
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
- ## arch/powerpc/include/asm/ppc-pci.h ##
-@@ arch/powerpc/include/asm/ppc-pci.h: 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>";
-+ if (pdev) {
-+ const char *drvstr = dev_driver_string(&pdev->dev);
-+
-+ if (strcmp(drvstr, ""))
-+ return drvstr;
-+ }
-+
-+ return "<null>";
- }
-
- #endif /* CONFIG_EEH */
-
- ## drivers/bcma/host_pci.c ##
-@@ drivers/bcma/host_pci.c: 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;
-+ name = dev_driver_string(&dev->dev);
-+ if (!strcmp(name, ""))
-+ name = dev_name(&dev->dev);
-+
- err = pci_request_regions(dev, name);
- if (err)
- goto err_pci_disable;
-
## drivers/crypto/hisilicon/qm.c ##
@@ drivers/crypto/hisilicon/qm.c: static int qm_alloc_uacce(struct hisi_qm *qm)
};
@@ drivers/net/ethernet/netronome/nfp/nfp_net_ethtool.c: nfp_get_drvinfo(struct nfp
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));
++ 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,
-
- ## drivers/ssb/pcihost_wrapper.c ##
-@@ drivers/ssb/pcihost_wrapper.c: 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;
-+
-+ name = dev_driver_string(&dev->dev);
-+ if (*name == '\0')
-+ name = dev_name(&dev->dev);
-+
- err = pci_request_regions(dev, name);
- if (err)
- goto err_pci_disable;
5: 574b743327b8 = 8: 1e58019165b9 scsi: message: fusion: Remove unused parameter of mpt_pci driver's probe()
6: 674c6efd3dab ! 9: dea72a470141 crypto: qat - simplify adf_enable_aer()
@@ drivers/crypto/qat/qat_4xxx/adf_drv.c: static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
-+ .err_handler = adf_err_handler,
++ .err_handler = &adf_err_handler,
};
module_pci_driver(adf_driver);
@@ drivers/crypto/qat/qat_c3xxx/adf_drv.c: static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
-+ .err_handler = adf_err_handler,
++ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ drivers/crypto/qat/qat_c62x/adf_drv.c: static struct pci_driver adf_driver = {
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
-+ .err_handler = adf_err_handler,
++ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
@@ drivers/crypto/qat/qat_common/adf_common_drv.h: void adf_ae_fw_release(struct ad
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);
@@ drivers/crypto/qat/qat_dh895xcc/adf_drv.c: static struct pci_driver adf_driver =
.probe = adf_probe,
.remove = adf_remove,
.sriov_configure = adf_sriov_configure,
-+ .err_handler = adf_err_handler,
++ .err_handler = &adf_err_handler,
};
static void adf_cleanup_pci_dev(struct adf_accel_dev *accel_dev)
7: 0e022deb0f75 = 10: b4165dda38ea PCI: Replace pci_dev::driver usage by pci_dev::dev.driver
8: edd9d24df02a = 11: d93a138bd7ab PCI: Drop duplicated tracking of a pci_dev's bound driver
base-commit: 5816b3e6577eaa676ceb00a848f0fd65fe2adc29
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver 2021-09-29 8:52 [PATCH v5 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver Uwe Kleine-König @ 2021-09-29 8:53 ` Uwe Kleine-König 2021-09-29 13:15 ` Andrew Donnellan 0 siblings, 1 reply; 6+ messages in thread From: Uwe Kleine-König @ 2021-09-29 8:53 UTC (permalink / raw) To: Bjorn Helgaas Cc: Christoph Hellwig, 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 | 2 +- drivers/misc/cxl/guest.c | 24 ++++++++++++--------- drivers/misc/cxl/pci.c | 30 ++++++++++++++++---------- drivers/pci/iov.c | 25 ++++++++++++++-------- drivers/pci/pci-driver.c | 25 +++++++++++----------- 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, 93 insertions(+), 71 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..36e84d904260 100644 --- a/arch/x86/kernel/probe_roms.c +++ b/arch/x86/kernel/probe_roms.c @@ -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) 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..f94660927544 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -164,13 +164,15 @@ 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) + pdrv = to_pci_driver(dev->driver); + if (!pdrv || !pdrv->sriov_get_vf_total_msix) goto unlock; - vf_total_msix = pdev->driver->sriov_get_vf_total_msix(pdev); + 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 +185,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 +196,14 @@ 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) { + pdrv = to_pci_driver(pdev->dev.driver); + if (!pdrv || !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 +213,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 +380,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 +397,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 +414,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 +426,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..4d20022b8631 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,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(pci_dev->dev.driver); pm_runtime_resume(dev); @@ -589,7 +589,7 @@ 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; @@ -630,7 +630,7 @@ 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); @@ -649,7 +649,7 @@ 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); /* @@ -1242,11 +1242,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 +1303,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 +1322,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 +1435,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] 6+ messages in thread
* Re: [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver 2021-09-29 8:53 ` [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver Uwe Kleine-König @ 2021-09-29 13:15 ` Andrew Donnellan 2021-09-29 13:43 ` Uwe Kleine-König 0 siblings, 1 reply; 6+ messages in thread From: Andrew Donnellan @ 2021-09-29 13:15 UTC (permalink / raw) To: Uwe Kleine-König, Bjorn Helgaas Cc: Christoph Hellwig, 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, 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 On 29/9/21 6:53 pm, Uwe Kleine-König wrote:> 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 && I'm not a huge fan of assignments in if statements and if you send a v6 I'd prefer you break it up. Apart from that everything in the powerpc and cxl sections looks good to me. -- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver 2021-09-29 13:15 ` Andrew Donnellan @ 2021-09-29 13:43 ` Uwe Kleine-König 2021-09-29 15:44 ` Andrew Donnellan 0 siblings, 1 reply; 6+ messages in thread From: Uwe Kleine-König @ 2021-09-29 13:43 UTC (permalink / raw) To: Andrew Donnellan Cc: Bjorn Helgaas, Mark Rutland, Peter Zijlstra, Benjamin Herrenschmidt, H. Peter Anvin, Oliver O'Halloran, Russell Currey, Jiri Olsa, Christoph Hellwig, Stefano Stabellini, Mathias Nyman, Michael Ellerman, x86, Alexander Shishkin, Ingo Molnar, linux-pci, xen-devel, Juergen Gross, Arnd Bergmann, Konrad Rzeszutek Wilk, Arnaldo Carvalho de Melo, Borislav Petkov, Bjorn Helgaas, Namhyung Kim, Thomas Gleixner, Boris Ostrovsky, Greg Kroah-Hartman, linux-usb, linux-perf-users, kernel, Frederic Barrat, Paul Mackerras, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 1935 bytes --] On Wed, Sep 29, 2021 at 11:15:37PM +1000, Andrew Donnellan wrote: > On 29/9/21 6:53 pm, Uwe Kleine-König wrote:> > 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 && > > I'm not a huge fan of assignments in if statements and if you send a v6 I'd > prefer you break it up. I'm not a huge fan either, I used it to keep the control flow as is and without introducing several calls to to_pci_driver. The whole code looks as follows: 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); } Without assignment in the if it could look as follows: 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->err_handler && afu_drv->err_handler->resume) afu_drv->err_handler->resume(afu_dev); } Fine for me. (Sidenote: What happens if the device is unbound directly after the check for afu_dev->dev.driver? This is a problem the old code had, too (assuming it is a real problem, didn't check deeply).) > Apart from that everything in the powerpc and cxl sections looks good to me. Thanks for checking. 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: 484 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver 2021-09-29 13:43 ` Uwe Kleine-König @ 2021-09-29 15:44 ` Andrew Donnellan 2021-09-30 13:48 ` Frederic Barrat 0 siblings, 1 reply; 6+ messages in thread From: Andrew Donnellan @ 2021-09-29 15:44 UTC (permalink / raw) To: Uwe Kleine-König Cc: Bjorn Helgaas, Mark Rutland, Peter Zijlstra, Benjamin Herrenschmidt, H. Peter Anvin, Oliver O'Halloran, Russell Currey, Jiri Olsa, Christoph Hellwig, Stefano Stabellini, Mathias Nyman, Michael Ellerman, x86, Alexander Shishkin, Ingo Molnar, linux-pci, xen-devel, Juergen Gross, Arnd Bergmann, Konrad Rzeszutek Wilk, Arnaldo Carvalho de Melo, Borislav Petkov, Bjorn Helgaas, Namhyung Kim, Thomas Gleixner, Boris Ostrovsky, Greg Kroah-Hartman, linux-usb, linux-perf-users, kernel, Frederic Barrat, Paul Mackerras, linuxppc-dev On 29/9/21 11:43 pm, Uwe Kleine-König wrote:> I'm not a huge fan either, I used it to keep the control flow as is and > without introducing several calls to to_pci_driver. > > The whole code looks as follows: > > 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); > } > > Without assignment in the if it could look as follows: > > 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->err_handler && afu_drv->err_handler->resume) > afu_drv->err_handler->resume(afu_dev); > } > > Fine for me. This looks fine. As an aside while writing my email I discovered the existence of container_of_safe(), a version of container_of() that handles the null and err ptr cases... if to_pci_driver() used that, the null check in the caller could be moved until after the to_pci_driver() call which would be neater. But then, grep tells me that container_of_safe() is used precisely zero times in the entire tree. Interesting. > (Sidenote: What happens if the device is unbound directly after the > check for afu_dev->dev.driver? This is a problem the old code had, too > (assuming it is a real problem, didn't check deeply).) Looking at any of the cxl PCI error handling paths brings back nightmares from a few years ago... Fred: I wonder if we need to add a lock here? -- Andrew Donnellan OzLabs, ADL Canberra ajd@linux.ibm.com IBM Australia Limited ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver 2021-09-29 15:44 ` Andrew Donnellan @ 2021-09-30 13:48 ` Frederic Barrat 0 siblings, 0 replies; 6+ messages in thread From: Frederic Barrat @ 2021-09-30 13:48 UTC (permalink / raw) To: Andrew Donnellan, Uwe Kleine-König Cc: Bjorn Helgaas, Mark Rutland, Peter Zijlstra, Benjamin Herrenschmidt, H. Peter Anvin, Oliver O'Halloran, Russell Currey, Jiri Olsa, Christoph Hellwig, Stefano Stabellini, Mathias Nyman, Michael Ellerman, x86, Alexander Shishkin, Ingo Molnar, linux-pci, xen-devel, Juergen Gross, Arnd Bergmann, Konrad Rzeszutek Wilk, Arnaldo Carvalho de Melo, Borislav Petkov, Bjorn Helgaas, Namhyung Kim, Thomas Gleixner, Boris Ostrovsky, Greg Kroah-Hartman, linux-usb, linux-perf-users, kernel, Paul Mackerras, linuxppc-dev On 29/09/2021 17:44, Andrew Donnellan wrote: > On 29/9/21 11:43 pm, Uwe Kleine-König wrote:> I'm not a huge fan either, > I used it to keep the control flow as is and >> without introducing several calls to to_pci_driver. >> >> The whole code looks as follows: >> >> 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); >> } >> >> Without assignment in the if it could look as follows: >> >> 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->err_handler && afu_drv->err_handler->resume) >> afu_drv->err_handler->resume(afu_dev); >> } >> >> Fine for me. > > This looks fine. > > As an aside while writing my email I discovered the existence of > container_of_safe(), a version of container_of() that handles the null > and err ptr cases... if to_pci_driver() used that, the null check in the > caller could be moved until after the to_pci_driver() call which would > be neater. > > But then, grep tells me that container_of_safe() is used precisely zero > times in the entire tree. Interesting. > >> (Sidenote: What happens if the device is unbound directly after the >> check for afu_dev->dev.driver? This is a problem the old code had, too >> (assuming it is a real problem, didn't check deeply).) > > Looking at any of the cxl PCI error handling paths brings back > nightmares from a few years ago... Fred: I wonder if we need to add a > lock here? Yes, it's indeed a potential issue, there's nothing to prevent the afu driver to unbind during that window. Sigh.. Fred ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-30 13:49 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-09-29 8:52 [PATCH v5 00/11] PCI: Drop duplicated tracking of a pci_dev's bound driver Uwe Kleine-König 2021-09-29 8:53 ` [PATCH v5 10/11] PCI: Replace pci_dev::driver usage by pci_dev::dev.driver Uwe Kleine-König 2021-09-29 13:15 ` Andrew Donnellan 2021-09-29 13:43 ` Uwe Kleine-König 2021-09-29 15:44 ` Andrew Donnellan 2021-09-30 13:48 ` Frederic Barrat
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).