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:
 <ZQ0PR01MB09816F6CF06E8021488DBCBC82D82@ZQ0PR01MB0981.CHNPR01.prod.partner.outlook.cn>
Date: Tue, 2 Dec 2025 01:45:34 +0000
From: Kevin Xie <kevin.xie@...rfivetech.com>
To: Bjorn Helgaas <helgaas@...nel.org>, Hal Feng <hal.feng@...rfivetech.com>
CC: Conor Dooley <conor+dt@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Palmer Dabbelt
	<palmer@...belt.com>, Paul Walmsley <pjw@...nel.org>, Albert Ou
	<aou@...s.berkeley.edu>, "Rafael J . Wysocki" <rafael@...nel.org>, Viresh
 Kumar <viresh.kumar@...aro.org>, Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczy��ski <kwilczynski@...nel.org>,
	Manivannan Sadhasivam <mani@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Emil
 Renner Berthing <emil.renner.berthing@...onical.com>, Heinrich Schuchardt
	<heinrich.schuchardt@...onical.com>, E Shattow <e@...eshell.de>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject:
 回复: [PATCH v4 1/6] PCI: starfive: Use regulator APIs instead of GPIO APIs to enable the 3V3 power supply of PCIe slots

> Krzysztof Kozlowski <krzk+dt@...nel.org>; Palmer Dabbelt
> <palmer@...belt.com>; Paul Walmsley <pjw@...nel.org>; Albert Ou
> <aou@...s.berkeley.edu>; Rafael J . Wysocki <rafael@...nel.org>; Viresh
> Kumar <viresh.kumar@...aro.org>; Lorenzo Pieralisi <lpieralisi@...nel.org>;
> Krzysztof Wilczy��ski <kwilczynski@...nel.org>; Manivannan Sadhasivam
> <mani@...nel.org>; Bjorn Helgaas <bhelgaas@...gle.com>; Liam Girdwood
> <lgirdwood@...il.com>; Mark Brown <broonie@...nel.org>; Emil Renner
> Berthing <emil.renner.berthing@...onical.com>; Heinrich Schuchardt
> <heinrich.schuchardt@...onical.com>; E Shattow <e@...eshell.de>;
> devicetree@...r.kernel.org; linux-pci@...r.kernel.org;
> linux-riscv@...ts.infradead.org; linux-kernel@...r.kernel.org; Kevin Xie
> <kevin.xie@...rfivetech.com>
> 主题: Re: [PATCH v4 1/6] PCI: starfive: Use regulator APIs instead of GPIO APIs
> to enable the 3V3 power supply of PCIe slots
> 
> [+cc Kevin, pcie-starfive.c maintainer; will need his ack]
> 
> Subject line is excessively long.
> 
> On Tue, Nov 25, 2025 at 03:55:59PM +0800, Hal Feng wrote:
> > The "enable-gpio" property is not documented in the dt-bindings and
> > using GPIO APIs is not a standard method to enable or disable PCIe
> > slot power, so use regulator APIs to replace them.
> 
> I can't tell from this whether existing DTs will continue to work after this
> change.  It looks like previously we looked for an "enable-gpios" or
> "enable-gpio" property and now we'll look for a "vpcie3v3-supply" regulator
> property.
> 
> I don't see "enable-gpios" or "enable-gpio" mentioned in any of the DT patches
> in this series, so maybe that property was never actually used before, and the
> code for pcie->power_gpio was actually dead?
> 

pcie->power_gpio is used in the our JH7110 EVB, it share the same pcie controller
driver with VisionFive2 board. Although JH7110 was not upstreamed, we still hope
to maintain the compatibility of the driver.

> Please add something here about how we know this won't break any existing
> setups using DTs that are already in the field.
> 
> > Tested-by: Matthias Brugger <mbrugger@...e.com>
> > Fixes: 39b91eb40c6a ("PCI: starfive: Add JH7110 PCIe controller")
> 
> Based on the cover letter, it looks like the point of this is to add support for a
> new device, which I don't think really qualifies as a "fix".
> 
> > Signed-off-by: Hal Feng <hal.feng@...rfivetech.com>
> > ---
> >  drivers/pci/controller/plda/pcie-starfive.c | 25
> > ++++++++++++---------
> >  1 file changed, 15 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/pci/controller/plda/pcie-starfive.c
> > b/drivers/pci/controller/plda/pcie-starfive.c
> > index 3caf53c6c082..298036c3e7f9 100644
> > --- a/drivers/pci/controller/plda/pcie-starfive.c
> > +++ b/drivers/pci/controller/plda/pcie-starfive.c
> > @@ -55,7 +55,7 @@ struct starfive_jh7110_pcie {
> >  	struct reset_control *resets;
> >  	struct clk_bulk_data *clks;
> >  	struct regmap *reg_syscon;
> > -	struct gpio_desc *power_gpio;
> > +	struct regulator *vpcie3v3;
> >  	struct gpio_desc *reset_gpio;
> >  	struct phy *phy;
> >
> > @@ -153,11 +153,13 @@ static int starfive_pcie_parse_dt(struct
> starfive_jh7110_pcie *pcie,
> >  		return dev_err_probe(dev, PTR_ERR(pcie->reset_gpio),
> >  				     "failed to get perst-gpio\n");
> >
> > -	pcie->power_gpio = devm_gpiod_get_optional(dev, "enable",
> > -						   GPIOD_OUT_LOW);
> > -	if (IS_ERR(pcie->power_gpio))
> > -		return dev_err_probe(dev, PTR_ERR(pcie->power_gpio),
> > -				     "failed to get power-gpio\n");
> > +	pcie->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
> > +	if (IS_ERR(pcie->vpcie3v3)) {
> > +		if (PTR_ERR(pcie->vpcie3v3) != -ENODEV)
> > +			return dev_err_probe(dev, PTR_ERR(pcie->vpcie3v3),
> > +					     "failed to get vpcie3v3 regulator\n");
> > +		pcie->vpcie3v3 = NULL;
> > +	}
> >
> >  	return 0;
> >  }
> > @@ -270,8 +272,8 @@ static void starfive_pcie_host_deinit(struct
> plda_pcie_rp *plda)
> >  		container_of(plda, struct starfive_jh7110_pcie, plda);
> >
> >  	starfive_pcie_clk_rst_deinit(pcie);
> > -	if (pcie->power_gpio)
> > -		gpiod_set_value_cansleep(pcie->power_gpio, 0);
> > +	if (pcie->vpcie3v3)
> > +		regulator_disable(pcie->vpcie3v3);
> >  	starfive_pcie_disable_phy(pcie);
> >  }
> >
> > @@ -304,8 +306,11 @@ static int starfive_pcie_host_init(struct
> plda_pcie_rp *plda)
> >  	if (ret)
> >  		return ret;
> >
> > -	if (pcie->power_gpio)
> > -		gpiod_set_value_cansleep(pcie->power_gpio, 1);
> > +	if (pcie->vpcie3v3) {
> > +		ret = regulator_enable(pcie->vpcie3v3);
> > +		if (ret)
> > +			dev_err_probe(dev, ret, "failed to enable vpcie3v3 regulator\n");
> > +	}
> >
> >  	if (pcie->reset_gpio)
> >  		gpiod_set_value_cansleep(pcie->reset_gpio, 1);
> > --
> > 2.43.2
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ