linux-kernel - Re: [PATCH v4 3/5] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <aAmx0CaXh24co_cm@nchen-desktop>
Date: Thu, 24 Apr 2025 11:36:48 +0800
From: "Peter Chen (CIX)" <peter.chen@...nel.org>
To: hans.zhang@...tech.com
Cc: bhelgaas@...gle.com, lpieralisi@...nel.org, kw@...ux.com,
	manivannan.sadhasivam@...aro.org, robh@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, peter.chen@...tech.com,
	linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Manikandan K Pillai <mpillai@...ence.com>
Subject: Re: [PATCH v4 3/5] PCI: cadence: Add header support for PCIe HPA
 controller

On 25-04-24 09:04:42, hans.zhang@...tech.com wrote:
> From: Manikandan K Pillai <mpillai@...ence.com>
> 
> +/**
> + * struct cdns_plat_pcie_of_data - Register bank offset for a platform
> + * @is_rc: controller is a RC
> + * @is_hpa: Controller architecture is HPA
> + * @ip_reg_bank_off: ip register bank start offset
> + * @ip_cfg_ctrl_reg_off: ip config control register start offset
> + * @axi_mstr_common_off: AXI master common register start
> + * @axi_slave_off: AXI slave offset start
> + * @axi_master_off: AXI master offset start
> + * @axi_hls_off: AXI HLS offset start
> + * @axi_ras_off: AXI RAS offset
> + * @axi_dti_off: AXI DTI offset

The variable's suffix _off may confuse the reader, since off stands for
something is turned off and also used at device driver commonly,
suggest using _offset and align with your other code.

> + */
> +struct cdns_plat_pcie_of_data {
> +	u32 is_rc:1;
> +	u32 is_hpa: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 inline void cdns_pcie_hpa_writel(struct cdns_pcie *pcie,
> +					enum cdns_pcie_reg_bank bank,
> +					u32 reg,
> +					u32 value)
> +{
> +	u32 offset =  cdns_reg_bank_to_off(pcie, bank);

More than one blank space after "=".

> +}
> +
> +static inline u32 cdns_pcie_hpa_readl(struct cdns_pcie *pcie,
> +				      enum cdns_pcie_reg_bank bank,
> +				      u32 reg)
> +{
> +	u32 offset =  cdns_reg_bank_to_off(pcie, bank);

ditto

-- 

Best regards,
Peter