On Sun, Mar 09, 2025 at 11:18:21AM +0800, Hans Zhang wrote: > > > On 2025/3/9 10:38, Siddharth Vadapalli wrote: > > On Sat, Mar 08, 2025 at 09:39:03PM +0800, Hans Zhang wrote: > > > Add configuration space capability search API using struct cdns_pcie* > > > pointer. > > > > > > The offset address of capability or extended capability designed by > > > different SOC design companies may not be the same. Therefore, a flexible > > > public API is required to find the offset address of a capability or > > > extended capability in the configuration space. > > > > > > Signed-off-by: Hans Zhang <18255117159@xxxxxxx> > > > --- > > > Changes since v1: > > > https://lore.kernel.org/linux-pci/20250123070935.1810110-1-18255117159@xxxxxxx > > > > > > - Added calling the new API in PCI-Cadence ep.c. > > > - Add a commit message reason for adding the API. > > > > In reply to your v1 patch, you have mentioned the following: > > "Our controller driver currently has no plans for upstream and needs to > > wait for notification from the boss." > > at: > > https://lore.kernel.org/linux-pci/fcfd4827-4d9e-4bcd-b1d0-8f9e349a6be7@xxxxxxx/ > > > > Since you have posted this patch, does it mean that you will be > > upstreaming your driver as well? If not, we still end up in the same > > situation as earlier where the Upstream Linux has APIs to support a > > Downstream driver. > > > > Bjorn indicated the above already at: > > https://lore.kernel.org/linux-pci/20250123170831.GA1226684@bhelgaas/ > > and you did agree to do so. But this patch has no reference to the > > upstream driver series which shall be making use of the APIs in this > > patch. > > Hi Siddharth, > > > Bjorn: > If/when you upstream code that needs this interface, include this > patch as part of the series. As Siddharth pointed out, we avoid > merging code that has no upstream users. > > > Hans: This user is: pcie-cadence-ep.c. I think this is an optimization of > Cadence common code. I think this is an optimization of Cadence common code. > Siddharth, what do you think? This seems to be an extension of the driver rather than an optimization. At first glance, though it seems like this patch is enabling code-reuse, it is actually attempting to walk through the config space registers to identify a capability. Prior to this patch, those offsets were hard-coded, saving the trouble of having to walk through the capability pointers to arrive at the capability. This patch will affect the following functions: 01. cdns_pcie_get_fn_from_vfn() 02. cdns_pcie_ep_write_header() 03. cdns_pcie_ep_set_msi() 04. cdns_pcie_ep_get_msi() 05. cdns_pcie_ep_get_msix() 06. cdns_pcie_ep_set_msix() 07. cdns_pcie_ep_send_msi_irq() 08. cdns_pcie_ep_map_msi_irq() 09. cdns_pcie_ep_send_msix_irq() 10. cdns_pcie_ep_start() which will now take longer to get to the capability whose offset was known. I understand that you wish to extend these functions to support your SoC where the offsets don't match the hard-coded ones. I will let Bjorn and others share their views on this patch. Regards, Siddharth.
