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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <78124cca-e10d-4b32-8760-d825ac88dffe@cixtech.com>
Date: Sat, 12 Apr 2025 23:19:07 +0800
From: Hans Zhang <hans.zhang@...tech.com>
To: Rob Herring <robh@...nel.org>
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kw@...ux.com,
 manivannan.sadhasivam@...aro.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Manikandan K Pillai <mpillai@...ence.com>
Subject: Re: [PATCH v3 3/6] PCI: cadence: Add header support for PCIe HPA
 controller


On 2025/4/12 04:31, Rob Herring wrote:
>> 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").
> 

Hi Rob,

Thanks your for reply. Will change.

>> +     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?

Will change.

> 
>> +
>>                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.

Will change.

Best regards,
Hans