linux-kernel - Re: [PATCH v3 5/6] PCI: cadence: Add callback functions for RP and EP controller

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL_JsqJgaeOcnUzw+rUF2yO4hQYCdZYssjxHzrDvvHGJimrASA@mail.gmail.com>
Date: Thu, 24 Apr 2025 10:07:38 -0500
From: Rob Herring <robh@...nel.org>
To: Manikandan Karunakaran Pillai <mpillai@...ence.com>
Cc: "hans.zhang@...tech.com" <hans.zhang@...tech.com>, "bhelgaas@...gle.com" <bhelgaas@...gle.com>, 
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>, "kw@...ux.com" <kw@...ux.com>, 
	"manivannan.sadhasivam@...aro.org" <manivannan.sadhasivam@...aro.org>, 
	"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>, 
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>, 
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 5/6] PCI: cadence: Add callback functions for RP and EP controller

On Wed, Apr 23, 2025 at 10:54 PM Manikandan Karunakaran Pillai
<mpillai@...ence.com> 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