RE: [PATCH v4 5/5] PCI: cadence: Add callback functions for RP and EP controller

Manikandan Karunakaran Pillai
2025-06-02T02:05:46Z

Hi,

The next patch v5 still under works will have Rob's comments addressed by separating out the drivers for legacy(LGA)
and High performance arch(HPA) replacing the ops method that are in current patches.

>-----Original Message-----
>From: manivannan.sadhasivam@linaro.org
><manivannan.sadhasivam@linaro.org>
>Sent: Sunday, June 1, 2025 8:11 PM
>To: Manikandan Karunakaran Pillai <mpillai@cadence.com>
>Cc: Krzysztof Kozlowski <krzk@kernel.org>; hans.zhang@cixtech.com;
>bhelgaas@google.com; lpieralisi@kernel.org; kw@linux.com;
>robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; Milind Parab
><mparab@cadence.com>; peter.chen@cixtech.com; linux-pci@vger.kernel.org;
>devicetree@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH v4 5/5] PCI: cadence: Add callback functions for RP and EP
>controller
>
>EXTERNAL MAIL
>
>
>On Sun, Apr 27, 2025 at 03:52:13AM +0000, Manikandan Karunakaran Pillai
>wrote:
>> >
>> >> ---
>> >> drivers/pci/controller/cadence/pci-j721e.c | 12 +
>> >> .../pci/controller/cadence/pcie-cadence-ep.c | 29 +-
>> >> .../controller/cadence/pcie-cadence-host.c | 263 ++++++++++++++++--
>> >> .../controller/cadence/pcie-cadence-plat.c | 27 +-
>> >> drivers/pci/controller/cadence/pcie-cadence.c | 197 ++++++++++++-
>> >> drivers/pci/controller/cadence/pcie-cadence.h | 11 +-
>> >> 6 files changed, 495 insertions(+), 44 deletions(-)
>> >>
>> >> diff --git a/drivers/pci/controller/cadence/pci-j721e.c
>> >b/drivers/pci/controller/cadence/pci-j721e.c
>> >> index ef1cfdae33bb..154b36c30101 100644
>> >> --- a/drivers/pci/controller/cadence/pci-j721e.c
>> >> +++ b/drivers/pci/controller/cadence/pci-j721e.c
>> >> @@ -164,6 +164,14 @@ static const struct cdns_pcie_ops j721e_pcie_ops
>= {
>> >> .start_link = j721e_pcie_start_link,
>> >> .stop_link = j721e_pcie_stop_link,
>> >> .link_up = j721e_pcie_link_up,
>> >> + .host_init_root_port = cdns_pcie_host_init_root_port,
>> >> + .host_bar_ib_config = cdns_pcie_host_bar_ib_config,
>> >> + .host_init_address_translation =
>> >cdns_pcie_host_init_address_translation,
>> >> + .detect_quiet_min_delay_set =
>> >cdns_pcie_detect_quiet_min_delay_set,
>> >> + .set_outbound_region = cdns_pcie_set_outbound_region,
>> >> + .set_outbound_region_for_normal_msg =
>> >> +
>> >cdns_pcie_set_outbound_region_for_normal_msg,
>> >> + .reset_outbound_region = cdns_pcie_reset_outbound_region,
>> >
>> >How did you resolve Rob's comments?
>> >
>> >These were repeated I think three times finally with:
>> >
>> >"Please listen when I say we do not want the ops method used in other
>> >drivers. "
>> >
>> >I think you just send the same ignoring previous discussion which is the
>> >shortest way to get yourself NAKed.
>> >
>> >
>>
>> I was waiting to check if there are additional comments on the approach,
>because this approach was taken
>> based on an earlier comments on the patches. Since we have not got any
>adverse comments from other
>> maintainers on this, I will separate out the entire driver for old and new
>architecture. The few common functions
>> will be moved to a common file, to be used as library functions. There will be
>repetitions of
>> code but from Rob's comments, I believe it is fine.
>>
>
>I agree with Rob. We should really get rid of the callbacks and try to make the
>common code a library.
>
>- Mani
>
>--
>மணிவண்ணன் சதாசிவம்