Re: [PATCH v5 08/12] PCI: imx6: Config look up table(LUT) to support MSI ITS and IOMMU for i.MX95

[Bjorn Helgaas]

On Fri, May 31, 2024 at 12:14:18PM -0400, Frank Li wrote:
> On Thu, May 30, 2024 at 06:08:32PM -0500, Bjorn Helgaas wrote:
> > On Tue, May 28, 2024 at 03:39:21PM -0400, Frank Li wrote:
> > > For the i.MX95, configuration of a LUT is necessary to convert Bus Device
> > > Function (BDF) to stream IDs, which are utilized by both IOMMU and ITS.
> > > This involves examining the msi-map and smmu-map to ensure consistent
> > > mapping of PCI BDF to the same stream IDs. Subsequently, LUT-related
> > > registers are configured. In the absence of an msi-map, the built-in MSI
> > > controller is utilized as a fallback.
> > > 
> > > Additionally, register a PCI bus notifier to trigger imx_pcie_add_device()
> > > upon the appearance of a new PCI device and when the bus is an iMX6 PCI
> > > controller. This function configures the correct LUT based on Device Tree
> > > Settings (DTS).
> > 
> > This scheme is pretty similar to apple_pcie_bus_notifier().  If we
> > have to do this, I wish it were *more* similar, i.e., copy the
> > function names, bitmap tracking, code structure, etc.
> 
> Actually, I refer apple_pcie_bus_notifier(). I can't direct use apple's
> implement because in imx95 have difference PCI host controller, another one
> is PCI ECAM netc controller. At lease function name should be similar with
> apple. 

I know it's different hardware, so obviously it can't be exactly the
same.  These are the differences that looked possibly unnecessary:

  - registering from initcall instead of .probe():

      apple_pcie_probe                  # .probe() method
        bus_register_notifier(&pci_bus_type, &apple_pcie_nb)

      imx_pcie_init                     # device_initcall()
        bus_register_notifier(&pci_bus_type, &imx_pcie_nb)

  - naming BUS_NOTIFY_DEL_DEVICE function:

      apple_pcie_release_device()
      imx_pcie_del_device()

  - tracking entries in use via bitmap vs scanning hardware for
    invalid entries:

      bitmap_find_free_region           # apple

      imx_pcie_config_lut               # imx
        for (i = 0; i < IMX95_MAX_LUT; i++)
          regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, &data1)
          if (data1 & IMX95_PE0_LUT_VLD)
            continue

When we fix a bug in one driver, it's easier to check whether other
drivers also need the fix if they use the same structure and names.

Bjorn