linux-kernel - RE: [PATCH v10 04/10] PCI: cadence: Add support for High Perf Architecture (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] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <CH2PPF4D26F8E1CB5EFC6AC9818A065A519A2C6A@CH2PPF4D26F8E1C.namprd07.prod.outlook.com>
Date: Sun, 2 Nov 2025 15:53:01 +0000
From: Manikandan Karunakaran Pillai <mpillai@...ence.com>
To: Manivannan Sadhasivam <mani@...nel.org>
CC: "hans.zhang@...tech.com" <hans.zhang@...tech.com>,
        "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>,
        "helgaas@...nel.org" <helgaas@...nel.org>,
        "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
        "kw@...ux.com"
	<kw@...ux.com>, "robh@...nel.org" <robh@...nel.org>,
        "kwilczynski@...nel.org"
	<kwilczynski@...nel.org>,
        "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
        "conor+dt@...nel.org" <conor+dt@...nel.org>,
        "fugang.duan@...tech.com"
	<fugang.duan@...tech.com>,
        "guoyin.chen@...tech.com"
	<guoyin.chen@...tech.com>,
        "peter.chen@...tech.com" <peter.chen@...tech.com>,
        "cix-kernel-upstream@...tech.com" <cix-kernel-upstream@...tech.com>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v10 04/10] PCI: cadence: Add support for High Perf
 Architecture (HPA) controller



>-----Original Message-----
>From: Manivannan Sadhasivam <mani@...nel.org>
>Sent: Sunday, November 2, 2025 8:38 PM
>To: Manikandan Karunakaran Pillai <mpillai@...ence.com>
>Cc: hans.zhang@...tech.com; bhelgaas@...gle.com; helgaas@...nel.org;
>lpieralisi@...nel.org; kw@...ux.com; robh@...nel.org;
>kwilczynski@...nel.org; krzk+dt@...nel.org; conor+dt@...nel.org;
>fugang.duan@...tech.com; guoyin.chen@...tech.com;
>peter.chen@...tech.com; cix-kernel-upstream@...tech.com; linux-
>pci@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org
>Subject: Re: [PATCH v10 04/10] PCI: cadence: Add support for High Perf
>Architecture (HPA) controller
>
>EXTERNAL MAIL
>
>
>On Sun, Nov 02, 2025 at 05:51:05AM +0000, Manikandan Karunakaran Pillai
>wrote:
>> Hi Mani,
>>
>> Pls find my comments below.
>>
>> >> >> +			value |=
>> >> >HPA_LM_RC_BAR_CFG_CTRL_PREF_MEM_64BITS(bar);
>> >> >> +	} else {
>> >> >> +		value |=
>HPA_LM_RC_BAR_CFG_CTRL_MEM_32BITS(bar);
>> >> >> +		if ((flags & IORESOURCE_PREFETCH))
>> >> >> +			value |=
>> >> >HPA_LM_RC_BAR_CFG_CTRL_PREF_MEM_32BITS(bar);
>> >> >> +	}
>> >> >> +
>> >> >> +	value |= HPA_LM_RC_BAR_CFG_APERTURE(bar, aperture);
>> >> >> +	cdns_pcie_hpa_writel(pcie, REG_BANK_IP_CFG_CTRL_REG,
>> >> >CDNS_PCIE_HPA_LM_RC_BAR_CFG, value);
>> >> >> +
>> >> >> +	return 0;
>> >> >> +}
>> >> >> +
>> >> >> +static int cdns_pcie_hpa_host_bar_config(struct cdns_pcie_rc *rc,
>> >> >> +					 struct resource_entry *entry)
>> >> >
>> >> >This and other functions are almost same as in 'pcie-cadence-host'. Why
>> >don't
>> >> >you reuse them in a common library?
>> >>
>> >> The function cdns_pcie_hpa_host_bar_config() calls functions
>> >cdns_pcie_hpa_host_bar_ib_config()
>> >> which is not common. All functions that are common between the two
>> >architecture are moved to the
>> >> common library file based on earlier comments.
>> >>
>> >
>> >This is not a good reason to duplicate the whole function. You could just
>make
>> >the common function accept the callback ib_config() and pass either
>> >cdns_pcie_host_bar_ib_config() or cdns_pcie_hpa_host_bar_ib_config().
>> >
>> >This pattern could be done for other functions as well. Please audit all of
>them
>> >and move them to common library. Currently, I could see a lot of
>duplications
>> >that could be avoided.
>>
>> The very first patch  for this feature included an ops struct  which was
>registered (very similar to a callback). Are are asking me to again implement
>the same design which was earlier rejected ?
>>
>
>You didn't provide any link to the discussion, so how can I decide without
>looking into it?

https://www.spinics.net/lists//devicetree/msg814276.html 

Patch v5 - 5/5 has the comments on callback.

>
>> Secondly except the names of the functions, the registers and their offset
>written and the sequence also changes for the implementations.
>>
>
>I don't think so. From a quick look, at least cdns_pcie_hpa_host_bar_config(),
>cdns_pcie_hpa_host_map_dma_ranges() are mostly similar. You can keep the
>functions having different register configurations, but should try to move
>others.
>
>- Mani
>
>--
>மணிவண்ணன் சதாசிவம்