| Message ID | 20240926-imx95_lut-v1-1-d0c62087dbab@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: add enabe(disable)_device() hook for bridge | expand |
On Thu, Sep 26, 2024 at 06:07:47PM -0400, Frank Li wrote: > Some PCIe bridges require special handling when enabling or disabling > PCIe devices. For example, on the i.MX95 platform, a lookup table must be > configured to inform the hardware how to convert pci_device_id to stream > (bus master) ID, which is used by the IOMMU and MSI controller to identify > bus master device. I don't think you're talking about PCI-to-PCI bridges (including PCIe Root Ports and Switch Ports). Those are all standardized and don't do anything with Requester IDs or Stream IDs. A PCI host bridge, e.g., a PCIe Root Complex, might have to deal with Stream IDs, and I think that's what you're enabling here. If so, I think the hooks should be in struct pci_host_bridge instead of pci_ops. > Enablement will be failure when there is not enough lookup table resource. > Avoid DMA write to wrong position. That is the reason why pci_fixup_enable > can't work since not return value for fixup function. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/pci/pci.c | 19 +++++++++++++++++++ > include/linux/pci.h | 2 ++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 7d85c04fbba2a..e0f83ed53d964 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2057,6 +2057,7 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars) > { > int err; > struct pci_dev *bridge; > + struct pci_bus *bus; > u16 cmd; > u8 pin; > > @@ -2068,6 +2069,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars) > if (bridge) > pcie_aspm_powersave_config_link(bridge); > > + bus = dev->bus; > + while (bus) { > + if (bus->ops->enable_device) > + err = bus->ops->enable_device(bus, dev); > + if (err) > + return err; > + bus = bus->parent; > + } > + > err = pcibios_enable_device(dev, bars); > if (err < 0) > return err; > @@ -2262,12 +2272,21 @@ void pci_disable_enabled_device(struct pci_dev *dev) > */ > void pci_disable_device(struct pci_dev *dev) > { > + struct pci_bus *bus; > + > dev_WARN_ONCE(&dev->dev, atomic_read(&dev->enable_cnt) <= 0, > "disabling already-disabled device"); > > if (atomic_dec_return(&dev->enable_cnt) != 0) > return; > > + bus = dev->bus; > + while (bus) { > + if (bus->ops->disable_device) > + bus->ops->disable_device(bus, dev); > + bus = bus->parent; > + } > + > do_pci_disable_device(dev); > > dev->is_busmaster = 0; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 573b4c4c2be61..42c25b8efd538 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -803,6 +803,8 @@ static inline int pcibios_err_to_errno(int err) > struct pci_ops { > int (*add_bus)(struct pci_bus *bus); > void (*remove_bus)(struct pci_bus *bus); > + int (*enable_device)(struct pci_bus *bus, struct pci_dev *dev); > + void (*disable_device)(struct pci_bus *bus, struct pci_dev *dev); > void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where); > int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val); > int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val); > > -- > 2.34.1 >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 7d85c04fbba2a..e0f83ed53d964 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2057,6 +2057,7 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars) { int err; struct pci_dev *bridge; + struct pci_bus *bus; u16 cmd; u8 pin; @@ -2068,6 +2069,15 @@ static int do_pci_enable_device(struct pci_dev *dev, int bars) if (bridge) pcie_aspm_powersave_config_link(bridge); + bus = dev->bus; + while (bus) { + if (bus->ops->enable_device) + err = bus->ops->enable_device(bus, dev); + if (err) + return err; + bus = bus->parent; + } + err = pcibios_enable_device(dev, bars); if (err < 0) return err; @@ -2262,12 +2272,21 @@ void pci_disable_enabled_device(struct pci_dev *dev) */ void pci_disable_device(struct pci_dev *dev) { + struct pci_bus *bus; + dev_WARN_ONCE(&dev->dev, atomic_read(&dev->enable_cnt) <= 0, "disabling already-disabled device"); if (atomic_dec_return(&dev->enable_cnt) != 0) return; + bus = dev->bus; + while (bus) { + if (bus->ops->disable_device) + bus->ops->disable_device(bus, dev); + bus = bus->parent; + } + do_pci_disable_device(dev); dev->is_busmaster = 0; diff --git a/include/linux/pci.h b/include/linux/pci.h index 573b4c4c2be61..42c25b8efd538 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -803,6 +803,8 @@ static inline int pcibios_err_to_errno(int err) struct pci_ops { int (*add_bus)(struct pci_bus *bus); void (*remove_bus)(struct pci_bus *bus); + int (*enable_device)(struct pci_bus *bus, struct pci_dev *dev); + void (*disable_device)(struct pci_bus *bus, struct pci_dev *dev); void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where); int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val); int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
Some PCIe bridges require special handling when enabling or disabling PCIe devices. For example, on the i.MX95 platform, a lookup table must be configured to inform the hardware how to convert pci_device_id to stream (bus master) ID, which is used by the IOMMU and MSI controller to identify bus master device. Enablement will be failure when there is not enough lookup table resource. Avoid DMA write to wrong position. That is the reason why pci_fixup_enable can't work since not return value for fixup function. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/pci/pci.c | 19 +++++++++++++++++++ include/linux/pci.h | 2 ++ 2 files changed, 21 insertions(+)