Re: [PATCH v9 07/14] PCI: cadence: Add support for High Perf Architecture (HPA) controller

On Tue, Sep 09, 2025 at 12:41:33AM GMT, Hans Zhang wrote:

[...]

Dear Mani,

Thank you very much for your reply. This function is a new one I added
during my collaboration with Manikandan. Here I'll reply to you. Other
Manikandan will reply to you.

Please trim your replies so that your message appears at the top.

quoted

rp_bdf, rc_bus_nr

Will change.

quoted

quoted

+     u32 ecam_addr_0, region_size_0, request_id_0;

So ECAM address is always 32bit? For all Cadence IP implementations?

Sorry, it's my fault here. Since the ECAM distribution of CIX SKY1 is as
follows:

X8: 0x2c00_0000 ~ 0x3000_0000
X4: 0x2900_0000 ~ 0x2c00_0000
X2: 0x2600_0000 ~ 0x2900_0000
X1_1: 0x2300_0000 ~ 0x2600_0000
X1_0: 0x2000_0000 ~ 0x2300_0000

This is only the CIX SKY1 designed in the low 32-bit address range. In fact,
Cadence IP supports 64-bit ECAM addresses. Will change.

quoted

quoted

+     int busnr = 0, secbus = 0, subbus = 0;
+     struct resource_entry *entry;
+     resource_size_t size;
+     u32 axi_address_low;
+     int nbits;
+     u64 sz;
+
+     entry = resource_list_first_type(&bridge->windows, IORESOURCE_BUS);
+     if (entry) {
+             busnr = entry->res->start;
+             secbus = (busnr < 0xff) ? (busnr + 1) : 0xff;
+             subbus = entry->res->end;
+     }
+     size = resource_size(cfg_res);
+     sz = 1ULL << fls64(size - 1);
+     nbits = ilog2(sz);
+     if (nbits < 8)
+             nbits = 8;
+
+     root_port_req_id_reg = ((busnr & 0xff) << 8);
+     pcie_bus_number_reg = ((subbus & 0xff) << 16) | ((secbus & 0xff) << 8) |
+                           (busnr & 0xff);
+     ecam_addr_0 = cfg_res->start;

Doesn't the platform require 'cfg_res->start' to be aligned to 256 MiB? The bus
range seem to be 0xff, so the Cadence IP allocates 8 bits for 'bus'. As per the
PCIe spec r6.0, sec 7.2.2, says:

'the base address of the range is aligned to a 2(n+20)-byte memory address
boundary'

So the 'cfg_res->start' should be aligned to 2^28 byte (256 MiB) address.

Just as the ECAM address range mentioned above, our bus is not 0xff.
Therefore, aligned to 2^28 byte (256 MiB) address is not required.

X8: 0xc0 ~ 0xff
X4: 0x90 ~ 0xbf
X2: 0x60 ~ 0x8f
X1_1: 0x30 ~ 0x5f
X1_0: 0x00 ~ 0x2f

So n is 6 for your platforma and you need to check for 64MiB alignment. But
since this check is performed in the common code, you need to somehow pass the
'n' bits value from your csky driver.

quoted

quoted

+     region_size_0 = nbits - 1;
+     request_id_0 = ((busnr & 0xff) << 8);
+
+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
+                          CDNS_PCIE_HPA_TAG_MANAGEMENT, 0x200000);
+
+     /* Taking slave err as OKAY */
+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE, CDNS_PCIE_HPA_SLAVE_RESP,
+                          0x0);
+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
+                          CDNS_PCIE_HPA_SLAVE_RESP + 0x4, 0x0);
+
+     /* Program the register "i_root_port_req_id_reg" with RP's BDF */
+     cdns_pcie_hpa_writel(pcie, REG_BANK_IP_REG, I_ROOT_PORT_REQ_ID_REG,
+                          root_port_req_id_reg);
+
+     /**
+      * Program the register "i_pcie_bus_numbers" with Primary(RP's bus number),
+      * secondary and subordinate bus numbers
+      */
+     cdns_pcie_hpa_writel(pcie, REG_BANK_RP, I_PCIE_BUS_NUMBERS,
+                          pcie_bus_number_reg);
+
+     /* Program the register "lm_hal_sbsa_ctrl[0]" to enable the sbsa */
+     value = cdns_pcie_hpa_readl(pcie, REG_BANK_IP_REG, LM_HAL_SBSA_CTRL);
+     value |= BIT(0);
+     cdns_pcie_hpa_writel(pcie, REG_BANK_IP_REG, LM_HAL_SBSA_CTRL, value);
+
+     /* Program region[0] for ECAM */
+     axi_address_low = (ecam_addr_0 & 0xfff00000) | region_size_0;

Could you explain what is getting programmed and why?

Cadence IP register description:

Region Base Address [31:8] (24 bits)
Lower [31:8] bits of the AXI region base address.

Region Size (6 bits)
The programme value in this field + 1 gives the region size. Minimum value
to be programmed into this field is 7 as the lower 8 bits of the AXI region
base address are replaced by zeros by the region select logic. Minumum
supported region size is 256 bytes.

Add some of this info in the comments.

The original intention of ecam_addr_0 & 0xfff00000 is to align with 1MB.
This is clearly redundant, as one Bus alone requires a 1MB ECAM range.

You need to check for alignment at the start as I mentioned above.

Will delete & 0xfff00000.

Here it refers to CPU address.

quoted

quoted

+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
+                          CDNS_PCIE_HPA_AT_OB_REGION_CPU_ADDR0(0),
+                          axi_address_low);
+
+     /* rc0-high-axi-address */
+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
+                          CDNS_PCIE_HPA_AT_OB_REGION_CPU_ADDR1(0), 0x0);
+     /* Type-1 CFG */
+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
+                          CDNS_PCIE_HPA_AT_OB_REGION_DESC0(0), 0x05000000);
+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
+                          CDNS_PCIE_HPA_AT_OB_REGION_DESC1(0),
+                          (request_id_0 << 16));
+
+     /* All AXI bits pass through PCIe */
+     cdns_pcie_hpa_writel(pcie, REG_BANK_AXI_SLAVE,
+                          CDNS_PCIE_HPA_AT_OB_REGION_PCI_ADDR0(0), 0x1b);
+     /* PCIe address-high */

What is this address?

Cadence IP register description:

AXI to PCIe Address Translation
AXI Slave to PCIe Address Translation registers. Provides bits 31:8 of the
PCIe address and the number of AXI address bits passed through.

The annotations here will be changed to:  /* AXI to PCIe Address Translation
*/

Okay.

- Mani

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