Re: [PATCH v1 4/5] PCI: Adapt all code locations to not use struct pci_dev::driver directly

2021-08-02 Thread Boris Ostrovsky 


On 7/31/21 8:08 AM, Uwe Kleine-König wrote:
> Hello Boris,
>
> On Fri, Jul 30, 2021 at 04:37:31PM -0400, Boris Ostrovsky wrote:
>> On 7/29/21 4:37 PM, Uwe Kleine-König wrote:
>>> --- a/drivers/pci/xen-pcifront.c
>>> +++ b/drivers/pci/xen-pcifront.c
>>> @@ -599,12 +599,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) {
>>> +   pdrv = pci_driver_of_dev(pcidev);
>>> +   if (!pcidev || !pdrv) {
>> If pcidev is NULL we are dead by the time we reach 'if' statement.
> Oh, you're right. So this needs something like:
>
>   if (!pcidev || !(pdrv = pci_driver_of_dev(pcidev)))


Sure, that's fine. And while at it please also drop 'if (pdrv)' check below 
(it's not directly related to your change but is more noticeable now so since 
you are in that function anyway I'd appreciate if you could do that).


Thanks.

-boris


>
> or repeating the call to pci_driver_of_dev for each previous usage of
> pdev->driver.
>
> If there are no other preferences I'd got with the first approach for
> v2.
>
> Best regards and thanks for catching,
> Uwe
>

Re: [PATCH v1 4/5] PCI: Adapt all code locations to not use struct pci_dev::driver directly

2021-07-31 Thread Uwe Kleine-König 
Hello Boris,

On Fri, Jul 30, 2021 at 04:37:31PM -0400, Boris Ostrovsky wrote:
> On 7/29/21 4:37 PM, Uwe Kleine-König wrote:
> > --- a/drivers/pci/xen-pcifront.c
> > +++ b/drivers/pci/xen-pcifront.c
> > @@ -599,12 +599,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) {
> > +   pdrv = pci_driver_of_dev(pcidev);
> > +   if (!pcidev || !pdrv) {
> 
> If pcidev is NULL we are dead by the time we reach 'if' statement.

Oh, you're right. So this needs something like:

if (!pcidev || !(pdrv = pci_driver_of_dev(pcidev)))

or repeating the call to pci_driver_of_dev for each previous usage of
pdev->driver.

If there are no other preferences I'd got with the first approach for
v2.

Best regards and thanks for catching,
Uwe

-- 
Pengutronix e.K.   | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |


signature.asc
Description: PGP signature

Re: [PATCH v1 4/5] PCI: Adapt all code locations to not use struct pci_dev::driver directly

2021-07-30 Thread Boris Ostrovsky 


On 7/29/21 4:37 PM, Uwe Kleine-König wrote:

> --- a/drivers/pci/xen-pcifront.c
> +++ b/drivers/pci/xen-pcifront.c
> @@ -599,12 +599,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) {
> + pdrv = pci_driver_of_dev(pcidev);
> + if (!pcidev || !pdrv) {


If pcidev is NULL we are dead by the time we reach 'if' statement.


-boris



>   dev_err(&pdev->xdev->dev, "device or AER driver is NULL\n");
>   pci_dev_put(pcidev);
>   return result;
>   }
> - pdrv = pcidev->driver;
>

[PATCH v1 4/5] PCI: Adapt all code locations to not use struct pci_dev::driver directly

2021-07-29 Thread Uwe Kleine-König 
This prepares removing the driver member of struct pci_dev which holds the
same information than struct pci_dev::dev->driver.

Signed-off-by: Uwe Kleine-König 
---
 arch/powerpc/include/asm/ppc-pci.h|  3 +-
 arch/powerpc/kernel/eeh_driver.c  | 12 ---
 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_common/adf_aer.c   |  2 +-
 drivers/message/fusion/mptbase.c  |  4 +--
 drivers/misc/cxl/guest.c  | 21 +--
 drivers/misc/cxl/pci.c| 25 +++--
 .../ethernet/hisilicon/hns3/hns3_ethtool.c|  3 +-
 .../ethernet/marvell/prestera/prestera_pci.c  |  2 +-
 drivers/net/ethernet/mellanox/mlxsw/pci.c |  2 +-
 .../ethernet/netronome/nfp/nfp_net_ethtool.c  |  2 +-
 drivers/pci/iov.c | 23 +++-
 drivers/pci/pci-driver.c  | 28 ---
 drivers/pci/pci.c | 10 +++---
 drivers/pci/pcie/err.c| 35 ++-
 drivers/pci/xen-pcifront.c|  4 +--
 drivers/ssb/pcihost_wrapper.c |  7 ++--
 drivers/usb/host/xhci-pci.c   |  3 +-
 21 files changed, 113 insertions(+), 85 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc-pci.h 
b/arch/powerpc/include/asm/ppc-pci.h
index 2b9edbf6e929..63938c156c57 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -57,7 +57,8 @@ 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 : "";
+   struct pci_driver *pdrv;
+   return (pdev && (pdrv = pci_driver_of_dev(pdev))) ? pdrv->name : 
"";
 }
 
 #endif /* CONFIG_EEH */
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 3eff6a4888e7..0fc712a8775e 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -104,13 +104,14 @@ 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)
+   struct pci_driver *pdrv;
+   if (!pdev || !(pdrv = pci_driver_of_dev(pdev)))
return NULL;
 
-   if (!try_module_get(pdev->driver->driver.owner))
+   if (!try_module_get(pdrv->driver.owner))
return NULL;
 
-   return pdev->driver;
+   return pdrv;
 }
 
 /**
@@ -122,10 +123,11 @@ 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)
+   struct pci_driver *pdrv;
+   if (!pdev || !(pdrv = pci_driver_of_dev(pdev)))
return;
 
-   module_put(pdev->driver->driver.owner);
+   module_put(pdrv->driver.owner);
 }
 
 /**
diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
index 9bf4dbbc26e2..14eb141b6cf2 100644
--- a/arch/x86/events/intel/uncore.c
+++ b/arch/x86/events/intel/uncore.c
@@ -1176,7 +1176,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 & ~0x) {
-   struct pci_driver *pci_drv = pdev->driver;
+   struct pci_driver *pci_drv = pci_driver_of_dev(pdev);
 
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..55bfafec9684 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 = pci_driver_of_dev(pdev);
const struct pci_device_id *id;
 
if (pdev->vendor == vendor && pdev->device == device)
diff --git a/drivers/bcma/host_pci.c b/drivers/bcma/host_pci.c
index 69c10a7b7c61..f9bfae87ebd9 100644
--- a/drivers/bcma/host_pci.c
+++ b/drivers/bcma/host_pci.c
@@ -161,6 +161,7 @@ static int bcma_host_pci_probe(struct pci_dev *dev,
   const struct pci_device_id *id)
 {
struct bcma_bus *bus;
+   struct pci_driver *pdrv;
int err = -ENOMEM;
const char *name;
u32 val;
@@ -176,8 +177,9 @@ static int bcma_host_pci_probe(struct pci_dev *dev,
goto err_kfree_bus;
 
name = dev_name(&dev->dev);
-   if (dev->driver && dev->driver->name)
-   name = dev->driver->name;
+   pdrv = pci_drive