linux-kernel - Re: [PATCH 7/7] pci: aspeed: Add ASPEED PCIe host controller driver

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: <a618588d-54fe-69a9-27f7-7b19ba905a52@linux.intel.com>
Date: Tue, 24 Jun 2025 13:50:06 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Jacky Chou <jacky_chou@...eedtech.com>
cc: "bhelgaas@...gle.com" <bhelgaas@...gle.com>, 
    "lpieralisi@...nel.org" <lpieralisi@...nel.org>, 
    "kwilczynski@...nel.org" <kwilczynski@...nel.org>, 
    "mani@...nel.org" <mani@...nel.org>, "robh@...nel.org" <robh@...nel.org>, 
    "krzk+dt@...nel.org" <krzk+dt@...nel.org>, 
    "conor+dt@...nel.org" <conor+dt@...nel.org>, 
    "joel@....id.au" <joel@....id.au>, 
    "andrew@...econstruct.com.au" <andrew@...econstruct.com.au>, 
    "vkoul@...nel.org" <vkoul@...nel.org>, 
    "kishon@...nel.org" <kishon@...nel.org>, 
    "linus.walleij@...aro.org" <linus.walleij@...aro.org>, 
    "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>, 
    "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>, 
    "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>, 
    "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, 
    "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>, 
    LKML <linux-kernel@...r.kernel.org>, 
    "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>, 
    "openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>, 
    "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>, 
    "elbadrym@...gle.com" <elbadrym@...gle.com>, 
    "romlem@...gle.com" <romlem@...gle.com>, 
    "anhphan@...gle.com" <anhphan@...gle.com>, 
    "wak@...gle.com" <wak@...gle.com>, 
    "yuxiaozhang@...gle.com" <yuxiaozhang@...gle.com>, 
    BMC-SW <BMC-SW@...eedtech.com>
Subject: Re: [PATCH 7/7] pci: aspeed: Add ASPEED PCIe host controller
 driver

On Mon, 23 Jun 2025, Jacky Chou wrote:

> > > +static bool aspeed_ast2700_get_link(struct aspeed_pcie *pcie) {
> > > +	u32 reg;
> > > +	bool link;
> > > +
> > > +	regmap_read(pcie->pciephy, PEHR_MISC_300, &reg);
> > > +	if (reg & CAPABILITY_GEN2) {
> > > +		regmap_read(pcie->pciephy, PEHR_MISC_344, &reg);
> > > +		link = !!(reg & LINK_STATUS_GEN2);
> > > +	} else {
> > > +		regmap_read(pcie->pciephy, PEHR_MISC_358, &reg);
> > > +		link = !!(reg & LINK_STATUS_GEN4);
> > 
> > While I don't entirely know the meaning of these bits, what if the link is not
> > using maximum speed it is capable of, does this check misbehave?
> > 
> 
> In our AST2700, there are gen4 RC and gen2 RC.
> Therefore, here will get capability to confirm it is gen2 or gen4.
> And the link status is in different register.

Okay, but then I'm a bit worried the naming of the defines as "Link 
Status" has a well-known meaning in PCIe Spec so it is confusing to call 
something else LINK_STATUS_*. Does that name come from some AST spec? In 
not, change the define naming. If yes, you should prefix it with so it is 
clear this is not referring to a generic PCIe thing and I suggest also 
adding a comment above those defines too so anyone looking them later 
won't wonder if you're just duplicating something from the PCIe spec.

-- 
 i.