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

manivannan.sadhasivam
2025-06-01T14:40:41Z

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

--
மணிவண்ணன் சதாசிவம்