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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <CH2PPF4D26F8E1C6C03FA2110C5AF9045F7A2852@CH2PPF4D26F8E1C.namprd07.prod.outlook.com>
Date: Thu, 24 Apr 2025 03:53:56 +0000
From: Manikandan Karunakaran Pillai <mpillai@...ence.com>
To: Rob Herring <robh@...nel.org>
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

>> >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.

>
>> Hence it was done similar to other drivers by architecture specific "ops".
>
>Yes, and IMO driver private/custom ops is the wrong direction to go.
>Read my prior reply below again.
>
>> The "if(is_hpa)" is now very limited where a specific ops functions does not
>make
>> any sense.
>
>But you still have them. In a separate driver, you would have 0.

The architecture is one changed and from a driver viewpoint, the addresses of the registers have changed.
The logic within the function still remains the same but it now accesses a different set of registers.
In the pcie-cadence-host.c and pcie-cadence-ep.c, there are still a lot of common functions.
If it is a separate driver, then the entire code needs to be put in two different files. The "is_hpa" checks will be 
0, but there will be a lot more of replicated code.

>
>> >A bunch of driver specific 'ops' is not the right direction despite
>> >other drivers (DWC) having that. If there are common parts, then make
>> >them library functions multiple drivers can call.