Re: [PATCH v3 3/6] PCI: cadence: Add header support for PCIe HPA controller

Rob Herring
2025-04-11T20:31:30Z

On Fri, Apr 11, 2025 at 06:36:53PM +0800, hans.zhang@cixtech.com wrote:
> From: Manikandan K Pillai <mpillai@cadence.com>
>
> Add the required definitions for register addresses and register bits
> for the Cadence PCIe HPA controllers. Add the register bank offsets
> for different platform architecture and update the global platform
> data - platform architecture, EP or RP configuration and the correct
> values of register offsets for different register banks during the
> platform probe.
>
> Signed-off-by: Manikandan K Pillai <mpillai@cadence.com>
> Co-developed-by: Hans Zhang <hans.zhang@cixtech.com>
> Signed-off-by: Hans Zhang <hans.zhang@cixtech.com>
> ---
> .../controller/cadence/pcie-cadence-host.c | 13 +-
> .../controller/cadence/pcie-cadence-plat.c | 87 +++++
> drivers/pci/controller/cadence/pcie-cadence.h | 320 +++++++++++++++++-
> 3 files changed, 410 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index 8af95e9da7ce..ce035eef0a5c 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -175,7 +175,7 @@ static int cdns_pcie_host_start_link(struct cdns_pcie_rc *rc)
> return ret;
> }
>
> -static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
> +int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
> {
> struct cdns_pcie *pcie = &rc->pcie;
> u32 value, ctrl;
> @@ -215,10 +215,10 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
> return 0;
> }
>
> -static int cdns_pcie_host_bar_ib_config(struct cdns_pcie_rc *rc,
> - enum cdns_pcie_rp_bar bar,
> - u64 cpu_addr, u64 size,
> - unsigned long flags)
> +int cdns_pcie_host_bar_ib_config(struct cdns_pcie_rc *rc,
> + enum cdns_pcie_rp_bar bar,
> + u64 cpu_addr, u64 size,
> + unsigned long flags)
> {
> struct cdns_pcie *pcie = &rc->pcie;
> u32 addr0, addr1, aperture, value;
> @@ -428,7 +428,7 @@ static int cdns_pcie_host_map_dma_ranges(struct cdns_pcie_rc *rc)
> return 0;
> }
>
> -static int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
> +int cdns_pcie_host_init_address_translation(struct cdns_pcie_rc *rc)
> {
> struct cdns_pcie *pcie = &rc->pcie;
> struct pci_host_bridge *bridge = pci_host_bridge_from_priv(rc);
> @@ -536,7 +536,6 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
> return -ENOMEM;
>
> pcie = &rc->pcie;
> - pcie->is_rc = true;
>
> rc->vendor_id = 0xffff;
> of_property_read_u32(np, "vendor-id", &rc->vendor_id);
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> index 0456845dabb9..b24176d4df1f 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-plat.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-plat.c
> @@ -24,6 +24,15 @@ struct cdns_plat_pcie {
>
> struct cdns_plat_pcie_of_data {
> bool is_rc;
> + bool is_hpa;

These can be bitfields (e.g. "is_rc: 1").

> + u32 ip_reg_bank_off;
> + u32 ip_cfg_ctrl_reg_off;
> + u32 axi_mstr_common_off;
> + u32 axi_slave_off;
> + u32 axi_master_off;
> + u32 axi_hls_off;
> + u32 axi_ras_off;
> + u32 axi_dti_off;
> };
>
> static const struct of_device_id cdns_plat_pcie_of_match[];
> @@ -72,6 +81,19 @@ static int cdns_plat_pcie_probe(struct platform_device *pdev)
> rc = pci_host_bridge_priv(bridge);
> rc->pcie.dev = dev;
> rc->pcie.ops = &cdns_plat_ops;
> + rc->pcie.is_hpa = data->is_hpa;
> + rc->pcie.is_rc = data->is_rc;
> +
> + /* Store all the register bank offsets */
> + rc->pcie.cdns_pcie_reg_offsets.ip_reg_bank_off = data->ip_reg_bank_off;
> + rc->pcie.cdns_pcie_reg_offsets.ip_cfg_ctrl_reg_off = data->ip_cfg_ctrl_reg_off;
> + rc->pcie.cdns_pcie_reg_offsets.axi_mstr_common_off = data->axi_mstr_common_off;
> + rc->pcie.cdns_pcie_reg_offsets.axi_master_off = data->axi_master_off;
> + rc->pcie.cdns_pcie_reg_offsets.axi_slave_off = data->axi_slave_off;
> + rc->pcie.cdns_pcie_reg_offsets.axi_hls_off = data->axi_hls_off;
> + rc->pcie.cdns_pcie_reg_offsets.axi_ras_off = data->axi_ras_off;
> + rc->pcie.cdns_pcie_reg_offsets.axi_dti_off = data->axi_dti_off;

Why not just store the match data ptr instead of having 2 copies of the
information?

> +
> cdns_plat_pcie->pcie = &rc->pcie;
>
> ret = cdns_pcie_init_phy(dev, cdns_plat_pcie->pcie);
> @@ -99,6 +121,19 @@ static int cdns_plat_pcie_probe(struct platform_device *pdev)
>
> ep->pcie.dev = dev;
> ep->pcie.ops = &cdns_plat_ops;
> + ep->pcie.is_hpa = data->is_hpa;
> + ep->pcie.is_rc = data->is_rc;
> +
> + /* Store all the register bank offset */
> + ep->pcie.cdns_pcie_reg_offsets.ip_reg_bank_off = data->ip_reg_bank_off;
> + ep->pcie.cdns_pcie_reg_offsets.ip_cfg_ctrl_reg_off = data->ip_cfg_ctrl_reg_off;
> + ep->pcie.cdns_pcie_reg_offsets.axi_mstr_common_off = data->axi_mstr_common_off;
> + ep->pcie.cdns_pcie_reg_offsets.axi_master_off = data->axi_master_off;
> + ep->pcie.cdns_pcie_reg_offsets.axi_slave_off = data->axi_slave_off;
> + ep->pcie.cdns_pcie_reg_offsets.axi_hls_off = data->axi_hls_off;
> + ep->pcie.cdns_pcie_reg_offsets.axi_ras_off = data->axi_ras_off;
> + ep->pcie.cdns_pcie_reg_offsets.axi_dti_off = data->axi_dti_off;
> +
> cdns_plat_pcie->pcie = &ep->pcie;
>
> ret = cdns_pcie_init_phy(dev, cdns_plat_pcie->pcie);
> @@ -150,10 +185,54 @@ static void cdns_plat_pcie_shutdown(struct platform_device *pdev)
>
> static const struct cdns_plat_pcie_of_data cdns_plat_pcie_host_of_data = {
> .is_rc = true,
> + .is_hpa = false,
> + .ip_reg_bank_off = 0x0,
> + .ip_cfg_ctrl_reg_off = 0x0,
> + .axi_mstr_common_off = 0x0,
> + .axi_slave_off = 0x0,
> + .axi_master_off = 0x0,
> + .axi_hls_off = 0x0,
> + .axi_ras_off = 0x0,
> + .axi_dti_off = 0x0,

You can omit anything initialized to 0.

> };
>
> static const struct cdns_plat_pcie_of_data cdns_plat_pcie_ep_of_data = {
> .is_rc = false,
> + .is_hpa = false,
> + .ip_reg_bank_off = 0x0,
> + .ip_cfg_ctrl_reg_off = 0x0,
> + .axi_mstr_common_off = 0x0,
> + .axi_slave_off = 0x0,
> + .axi_master_off = 0x0,
> + .axi_hls_off = 0x0,
> + .axi_ras_off = 0x0,
> + .axi_dti_off = 0x0,
> +};
> +
> +static const struct cdns_plat_pcie_of_data cdns_plat_pcie_hpa_host_of_data = {
> + .is_rc = true,
> + .is_hpa = true,
> + .ip_reg_bank_off = CDNS_PCIE_HPA_IP_REG_BANK,
> + .ip_cfg_ctrl_reg_off = CDNS_PCIE_HPA_IP_CFG_CTRL_REG_BANK,
> + .axi_mstr_common_off = CDNS_PCIE_HPA_IP_AXI_MASTER_COMMON,
> + .axi_slave_off = CDNS_PCIE_HPA_AXI_SLAVE,
> + .axi_master_off = CDNS_PCIE_HPA_AXI_MASTER,
> + .axi_hls_off = 0,
> + .axi_ras_off = 0,
> + .axi_dti_off = 0,
> +};
> +
> +static const struct cdns_plat_pcie_of_data cdns_plat_pcie_hpa_ep_of_data = {
> + .is_rc = false,
> + .is_hpa = true,
> + .ip_reg_bank_off = CDNS_PCIE_HPA_IP_REG_BANK,
> + .ip_cfg_ctrl_reg_off = CDNS_PCIE_HPA_IP_CFG_CTRL_REG_BANK,
> + .axi_mstr_common_off = CDNS_PCIE_HPA_IP_AXI_MASTER_COMMON,
> + .axi_slave_off = CDNS_PCIE_HPA_AXI_SLAVE,
> + .axi_master_off = CDNS_PCIE_HPA_AXI_MASTER,
> + .axi_hls_off = 0,
> + .axi_ras_off = 0,
> + .axi_dti_off = 0,
> };