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

[manivannan.sadhasivam@xxxxxxxxxx]

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

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