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] [day] [month] [year] [list]

Message-ID: <wegcjrrgtzetavfujj24obsuc5av6kzqjtw62neffpgoga7qfo@pxnunfe5aqrc>
Date: Sun, 2 Nov 2025 22:09:01 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Manikandan Karunakaran Pillai <mpillai@...ence.com>
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

On Sun, Nov 02, 2025 at 03:53:01PM +0000, Manikandan Karunakaran Pillai wrote:
> 
> 
> >-----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.
> 

That was a completely different comment. Both Rob and myself suggested getting
rid of platform specific ops and move towards the common library.

Here, I'm suggesting you to have some functions in the common library accept a
callback as a function argument for the controller arch (host vs HPA). This will
allow you to have more functions in the common library as opposed to duplicating
the function definitions.

Both are not the same.

- Mani

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