On Wed, Apr 23, 2025 at 10:54 PM Manikandan Karunakaran Pillai <mpillai@xxxxxxxxxxx> wrote: > > >> >What exactly is shared between these 2 implementations. Link handling, > >> >config space accesses, address translation, and host init are all > >> >different. What's left to share? MSIs (if not passed thru) and > >> >interrupts? I think it's questionable that this be the same driver. > >> > > >> The address of both these have changed as the controller architecture has > >> changed. In the event these driver have to be same driver, there will lot of > >> sprinkled "if(is_hpa)" and that was already rejected in earlier version of > >code. > > > >I'm saying they should *not* be the same driver because you don't > >share hardly anything. Again, what is really common here? > > The architecture of the PCie controller is next generation but the software flow > and functions are almost same. The addresses of the registers accessed for the > newly added functions have changed and to ensure that we reduce "if(is_hpa)" > checks, the ops method was adopted as in other existing drivers. Please listen when I say we do not want the ops method used in other drivers. That's called a midlayer and is an anti-pattern. Here's some background reading for you: https://lwn.net/Articles/708891/ https://blog.ffwll.ch/2016/12/midlayers-once-more-with-feeling.html So what are you supposed to do with the common parts? Make them a library that each driver can call into as I already suggested. If you want an example, see SDHCI drivers and library (sdhci.c). Actually, the current Cadence support is also an example of this. It's 2 different drivers (pcie-cadence-plat.c and pci-j721e.c) with a library of functions (pcie-cadence.c). We probably had this same discussion when all that was upstreamed. Sigh. Now, where there should be more ops is in struct pci_host_bridge for things like link state and PERST# control. Then the PCI core could manage the link state and drivers only have to provide start/stop/status. Rob
