linux-kernel - Re: [PATCH v6 5/7] PCI: spacemit: Add SpacemiT PCIe host 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: <7f5r2lnlrsltowfninu76bivebgwkr5i3pdorj3hywg22mtmdz@m35cpser6opt>
Date: Mon, 17 Nov 2025 18:47:00 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Vitor Soares <ivitro@...il.com>
Cc: Alex Elder <elder@...cstar.com>, lpieralisi@...nel.org, 
	kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com, dlan@...too.org, 
	aurelien@...el32.net, johannes@...felt.com, p.zabel@...gutronix.de, 
	christian.bruel@...s.st.com, thippeswamy.havalige@....com, krishna.chundru@....qualcomm.com, 
	mayank.rana@....qualcomm.com, qiang.yu@....qualcomm.com, shradha.t@...sung.com, 
	inochiama@...il.com, guodong@...cstar.com, linux-pci@...r.kernel.org, 
	spacemit@...ts.linux.dev, linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 5/7] PCI: spacemit: Add SpacemiT PCIe host driver

On Mon, Nov 17, 2025 at 01:01:04PM +0000, Vitor Soares wrote:

[...]

> > +static int k1_pcie_probe(struct platform_device *pdev)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct k1_pcie *k1;
> > +       int ret;
> > +
> > +       k1 = devm_kzalloc(dev, sizeof(*k1), GFP_KERNEL);
> > +       if (!k1)
> > +               return -ENOMEM;
> > +
> > +       k1->pmu = syscon_regmap_lookup_by_phandle_args(dev_of_node(dev),
> > +                                                      SYSCON_APMU, 1,
> > +                                                      &k1->pmu_off);
> > +       if (IS_ERR(k1->pmu))
> > +               return dev_err_probe(dev, PTR_ERR(k1->pmu),
> > +                                    "failed to lookup PMU registers\n");
> > +
> > +       k1->link = devm_platform_ioremap_resource_byname(pdev, "link");
> > +       if (IS_ERR(k1->link))
> > +               return dev_err_probe(dev, PTR_ERR(k1->link),
> > +                                    "failed to map \"link\" registers\n");
> > +
> > +       k1->pci.dev = dev;
> > +       k1->pci.ops = &k1_pcie_ops;
> > +       k1->pci.pp.num_vectors = MAX_MSI_IRQS;
> > +       dw_pcie_cap_set(&k1->pci, REQ_RES);
> > +
> > +       k1->pci.pp.ops = &k1_pcie_host_ops;
> > +
> > +       /* Hold the PHY in reset until we start the link */
> > +       regmap_set_bits(k1->pmu, k1->pmu_off + PCIE_CLK_RESET_CONTROL,
> > +                       APP_HOLD_PHY_RST);
> > +
> > +       ret = devm_regulator_get_enable(dev, "vpcie3v3");
> > +       if (ret)
> > +               return dev_err_probe(dev, ret,
> > +                                    "failed to get \"vpcie3v3\" supply\n");
> > +
> 
> Hi,
> 
> While investigation a similar PCIe setup, I come across this patch series and
> had a question regarding regulator handling.
> 
> In the patch, the host controller explicitly enables the vpcie3v3. However, the
> pci-pwctrl-slot driver (compatible pciclass,0604) is also instantiated via
> device tree and is supposed to manage the slot supply.
> 
> I'm wondering whether it's actually necessary to enable the regulator in the
> host controller, since the slot driver should already take care of it. However,
> in my tests, the endpoint is not discovered unless the host driver enables the
> regulator early. It's not entirely clear whether this is due to the PHY and
> reset sequence, or the timing of the vpcie3v3 regulator activation.
> 
> @Manivannan: In the patch, the host controller driver enables the vpcie3v3
> regulator, even though the pci-pwrctrl-slot driver (pciclass,0604) is supposed
> to manage it. Is this duplication intentional, or is there a preferred way to
> ensure the supply is active before bus scanning? Any guidance would be
> appreciated.
> 

Yes, the regulator handling is duplicated now. This is due to missing PERST#
handling in the pwrctrl-slot driver. We are working on a series to be posted
this week that will fix this issue, but until then, pwrctrl-slot driver is not
reliable to power up the endpoint. So I've asked Alex to handle the regulator
and PERST# in the controller driver for now. Once my rework series gets merged,
this part can be dropped from the controller driver and there is no change
required in the binding/dts.

- Mani

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