From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
If the platform is using the new DT binding, let the pwrctrl core toggle
PERST# for the device. This is achieved by populating the
'pci_host_bridge::toggle_perst' callback with qcom_pcie_toggle_perst().
qcom_pcie_toggle_perst() will find the PERST# GPIO descriptor associated
with the supplied 'device_node' and toggles PERST#. If PERST# is not found
in the supplied node, the function will look for PERST# in the parent node
as a fallback. This is needed since PERST# won't be available in the
endpoint node as per the DT binding.
Note that the driver still asserts PERST# during the controller
initialization as it is needed as per the hardware documentation. Apart
from that, the driver wouldn't touch PERST# for the new binding.
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
drivers/pci/controller/dwc/pcie-qcom.c | 89 +++++++++++++++++++++++++++++-----
1 file changed, 78 insertions(+), 11 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 78355d12f10d263a0bb052e24c1e2d5e8f68603d..3c5c65d7d97cac186e1b671f80ba7296ad226d68 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -276,6 +276,7 @@ struct qcom_pcie_port {
struct qcom_pcie_perst {
struct list_head list;
struct gpio_desc *desc;
+ struct device_node *np;
};
struct qcom_pcie {
@@ -298,11 +299,50 @@ struct qcom_pcie {
#define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
-static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
+static struct gpio_desc *qcom_find_perst(struct qcom_pcie *pcie, struct device_node *np)
+{
+ struct qcom_pcie_perst *perst;
+
+ list_for_each_entry(perst, &pcie->perst, list) {
+ if (np == perst->np)
+ return perst->desc;
+ }
+
+ return NULL;
+}
+
+static void qcom_toggle_perst_per_device(struct qcom_pcie *pcie,
+ struct device_node *np, bool assert)
+{
+ int val = assert ? 1 : 0;
+ struct gpio_desc *perst;
+
+ perst = qcom_find_perst(pcie, np);
+ if (perst)
+ goto toggle_perst;
+
+ /*
+ * If PERST# is not available in the current node, try the parent. This
+ * fallback is needed if the current node belongs to an endpoint or
+ * switch upstream port.
+ */
+ if (np->parent)
+ perst = qcom_find_perst(pcie, np->parent);
+
+toggle_perst:
+ /* gpiod* APIs handle NULL gpio_desc gracefully. So no need to check. */
+ gpiod_set_value_cansleep(perst, val);
+}
+
+static void qcom_perst_reset(struct qcom_pcie *pcie, struct device_node *np,
+ bool assert)
{
struct qcom_pcie_perst *perst;
int val = assert ? 1 : 0;
+ if (np)
+ return qcom_toggle_perst_per_device(pcie, np, assert);
+
if (list_empty(&pcie->perst))
gpiod_set_value_cansleep(pcie->reset, val);
@@ -310,22 +350,34 @@ static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
gpiod_set_value_cansleep(perst->desc, val);
}
-static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
+static void qcom_ep_reset_assert(struct qcom_pcie *pcie, struct device_node *np)
{
- qcom_perst_assert(pcie, true);
+ qcom_perst_reset(pcie, np, true);
usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
}
-static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
+static void qcom_ep_reset_deassert(struct qcom_pcie *pcie,
+ struct device_node *np)
{
struct dw_pcie_rp *pp = &pcie->pci->pp;
msleep(PCIE_T_PVPERL_MS);
- qcom_perst_assert(pcie, false);
+ qcom_perst_reset(pcie, np, false);
if (!pp->use_linkup_irq)
msleep(PCIE_RESET_CONFIG_WAIT_MS);
}
+static void qcom_pcie_toggle_perst(struct pci_host_bridge *bridge,
+ struct device_node *np, bool assert)
+{
+ struct qcom_pcie *pcie = dev_get_drvdata(bridge->dev.parent);
+
+ if (assert)
+ qcom_ep_reset_assert(pcie, np);
+ else
+ qcom_ep_reset_deassert(pcie, np);
+}
+
static int qcom_pcie_start_link(struct dw_pcie *pci)
{
struct qcom_pcie *pcie = to_qcom_pcie(pci);
@@ -1320,7 +1372,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
struct qcom_pcie *pcie = to_qcom_pcie(pci);
int ret;
- qcom_ep_reset_assert(pcie);
+ qcom_ep_reset_assert(pcie, NULL);
ret = pcie->cfg->ops->init(pcie);
if (ret)
@@ -1336,7 +1388,13 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
goto err_disable_phy;
}
- qcom_ep_reset_deassert(pcie);
+ /*
+ * Only deassert PERST# for all devices here if legacy binding is used.
+ * For the new binding, pwrctrl driver is expected to toggle PERST# for
+ * individual devices.
+ */
+ if (list_empty(&pcie->perst))
+ qcom_ep_reset_deassert(pcie, NULL);
if (pcie->cfg->ops->config_sid) {
ret = pcie->cfg->ops->config_sid(pcie);
@@ -1344,10 +1402,12 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
goto err_assert_reset;
}
+ pci->pp.bridge->toggle_perst = qcom_pcie_toggle_perst;
+
return 0;
err_assert_reset:
- qcom_ep_reset_assert(pcie);
+ qcom_ep_reset_assert(pcie, NULL);
err_disable_phy:
qcom_pcie_phy_power_off(pcie);
err_deinit:
@@ -1361,7 +1421,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct qcom_pcie *pcie = to_qcom_pcie(pci);
- qcom_ep_reset_assert(pcie);
+ qcom_ep_reset_assert(pcie, NULL);
qcom_pcie_phy_power_off(pcie);
pcie->cfg->ops->deinit(pcie);
}
@@ -1740,6 +1800,9 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
return -ENOMEM;
perst->desc = reset;
+ /* Increase the refcount to make sure 'np' is valid till it is stored */
+ of_node_get(np);
+ perst->np = np;
list_add_tail(&perst->list, &pcie->perst);
parse_child_node:
@@ -1803,8 +1866,10 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
list_del(&port->list);
}
- list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list)
+ list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list) {
+ of_node_put(perst->np);
list_del(&perst->list);
+ }
return ret;
}
@@ -2044,8 +2109,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
qcom_pcie_phy_exit(pcie);
list_for_each_entry_safe(port, tmp_port, &pcie->ports, list)
list_del(&port->list);
- list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list)
+ list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list) {
+ of_node_put(perst->np);
list_del(&perst->list);
+ }
err_pm_runtime_put:
pm_runtime_put(dev);
pm_runtime_disable(dev);
--
2.45.2
On Wed, Sep 03, 2025 at 12:43:27PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> If the platform is using the new DT binding, let the pwrctrl core toggle
> PERST# for the device. This is achieved by populating the
> 'pci_host_bridge::toggle_perst' callback with qcom_pcie_toggle_perst().
Can we say something here about how to identify a "new DT binding"?
I assume there is a DT property or something that makes it "new"?
> qcom_pcie_toggle_perst() will find the PERST# GPIO descriptor associated
> with the supplied 'device_node' and toggles PERST#. If PERST# is not found
> in the supplied node, the function will look for PERST# in the parent node
> as a fallback. This is needed since PERST# won't be available in the
> endpoint node as per the DT binding.
>
> Note that the driver still asserts PERST# during the controller
> initialization as it is needed as per the hardware documentation. Apart
> from that, the driver wouldn't touch PERST# for the new binding.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 89 +++++++++++++++++++++++++++++-----
> 1 file changed, 78 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 78355d12f10d263a0bb052e24c1e2d5e8f68603d..3c5c65d7d97cac186e1b671f80ba7296ad226d68 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -276,6 +276,7 @@ struct qcom_pcie_port {
> struct qcom_pcie_perst {
> struct list_head list;
> struct gpio_desc *desc;
> + struct device_node *np;
> };
>
> struct qcom_pcie {
> @@ -298,11 +299,50 @@ struct qcom_pcie {
>
> #define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
>
> -static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
> +static struct gpio_desc *qcom_find_perst(struct qcom_pcie *pcie, struct device_node *np)
> +{
> + struct qcom_pcie_perst *perst;
> +
> + list_for_each_entry(perst, &pcie->perst, list) {
> + if (np == perst->np)
> + return perst->desc;
> + }
> +
> + return NULL;
> +}
> +
> +static void qcom_toggle_perst_per_device(struct qcom_pcie *pcie,
> + struct device_node *np, bool assert)
> +{
> + int val = assert ? 1 : 0;
> + struct gpio_desc *perst;
> +
> + perst = qcom_find_perst(pcie, np);
> + if (perst)
> + goto toggle_perst;
> +
> + /*
> + * If PERST# is not available in the current node, try the parent. This
> + * fallback is needed if the current node belongs to an endpoint or
> + * switch upstream port.
> + */
> + if (np->parent)
> + perst = qcom_find_perst(pcie, np->parent);
Ugh. I think we need to fix the data structures here before we go
much farther. We should be able to search for PERST# once at probe of
the Qcom controller. Hopefully we don't need lists of things.
See https://lore.kernel.org/r/20250908183325.GA1450728@bhelgaas.
> +toggle_perst:
> + /* gpiod* APIs handle NULL gpio_desc gracefully. So no need to check. */
> + gpiod_set_value_cansleep(perst, val);
> +}
> +
> +static void qcom_perst_reset(struct qcom_pcie *pcie, struct device_node *np,
> + bool assert)
> {
> struct qcom_pcie_perst *perst;
> int val = assert ? 1 : 0;
>
> + if (np)
> + return qcom_toggle_perst_per_device(pcie, np, assert);
> +
> if (list_empty(&pcie->perst))
> gpiod_set_value_cansleep(pcie->reset, val);
>
> @@ -310,22 +350,34 @@ static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
> gpiod_set_value_cansleep(perst->desc, val);
> }
>
> -static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> +static void qcom_ep_reset_assert(struct qcom_pcie *pcie, struct device_node *np)
> {
> - qcom_perst_assert(pcie, true);
> + qcom_perst_reset(pcie, np, true);
> usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
> }
>
> -static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
> +static void qcom_ep_reset_deassert(struct qcom_pcie *pcie,
> + struct device_node *np)
> {
> struct dw_pcie_rp *pp = &pcie->pci->pp;
>
> msleep(PCIE_T_PVPERL_MS);
> - qcom_perst_assert(pcie, false);
> + qcom_perst_reset(pcie, np, false);
> if (!pp->use_linkup_irq)
> msleep(PCIE_RESET_CONFIG_WAIT_MS);
> }
>
> +static void qcom_pcie_toggle_perst(struct pci_host_bridge *bridge,
> + struct device_node *np, bool assert)
> +{
> + struct qcom_pcie *pcie = dev_get_drvdata(bridge->dev.parent);
> +
> + if (assert)
> + qcom_ep_reset_assert(pcie, np);
> + else
> + qcom_ep_reset_deassert(pcie, np);
> +}
> +
> static int qcom_pcie_start_link(struct dw_pcie *pci)
> {
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
> @@ -1320,7 +1372,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
> int ret;
>
> - qcom_ep_reset_assert(pcie);
> + qcom_ep_reset_assert(pcie, NULL);
>
> ret = pcie->cfg->ops->init(pcie);
> if (ret)
> @@ -1336,7 +1388,13 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> goto err_disable_phy;
> }
>
> - qcom_ep_reset_deassert(pcie);
> + /*
> + * Only deassert PERST# for all devices here if legacy binding is used.
> + * For the new binding, pwrctrl driver is expected to toggle PERST# for
> + * individual devices.
Can we replace "new binding" with something explicit? In a few
months, "new binding" won't mean anything.
> + */
> + if (list_empty(&pcie->perst))
> + qcom_ep_reset_deassert(pcie, NULL);
>
> if (pcie->cfg->ops->config_sid) {
> ret = pcie->cfg->ops->config_sid(pcie);
> @@ -1344,10 +1402,12 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> goto err_assert_reset;
> }
>
> + pci->pp.bridge->toggle_perst = qcom_pcie_toggle_perst;
> +
> return 0;
>
> err_assert_reset:
> - qcom_ep_reset_assert(pcie);
> + qcom_ep_reset_assert(pcie, NULL);
> err_disable_phy:
> qcom_pcie_phy_power_off(pcie);
> err_deinit:
> @@ -1361,7 +1421,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
>
> - qcom_ep_reset_assert(pcie);
> + qcom_ep_reset_assert(pcie, NULL);
> qcom_pcie_phy_power_off(pcie);
> pcie->cfg->ops->deinit(pcie);
> }
> @@ -1740,6 +1800,9 @@ static int qcom_pcie_parse_perst(struct qcom_pcie *pcie,
> return -ENOMEM;
>
> perst->desc = reset;
> + /* Increase the refcount to make sure 'np' is valid till it is stored */
> + of_node_get(np);
> + perst->np = np;
> list_add_tail(&perst->list, &pcie->perst);
>
> parse_child_node:
> @@ -1803,8 +1866,10 @@ static int qcom_pcie_parse_ports(struct qcom_pcie *pcie)
> list_del(&port->list);
> }
>
> - list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list)
> + list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list) {
> + of_node_put(perst->np);
> list_del(&perst->list);
> + }
>
> return ret;
> }
> @@ -2044,8 +2109,10 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> qcom_pcie_phy_exit(pcie);
> list_for_each_entry_safe(port, tmp_port, &pcie->ports, list)
> list_del(&port->list);
> - list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list)
> + list_for_each_entry_safe(perst, tmp_perst, &pcie->perst, list) {
> + of_node_put(perst->np);
> list_del(&perst->list);
> + }
> err_pm_runtime_put:
> pm_runtime_put(dev);
> pm_runtime_disable(dev);
>
> --
> 2.45.2
>
>
On Mon, Sep 08, 2025 at 02:34:28PM GMT, Bjorn Helgaas wrote:
> On Wed, Sep 03, 2025 at 12:43:27PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > If the platform is using the new DT binding, let the pwrctrl core toggle
> > PERST# for the device. This is achieved by populating the
> > 'pci_host_bridge::toggle_perst' callback with qcom_pcie_toggle_perst().
>
> Can we say something here about how to identify a "new DT binding"?
> I assume there is a DT property or something that makes it "new"?
This is taken care now.
>
> > qcom_pcie_toggle_perst() will find the PERST# GPIO descriptor associated
> > with the supplied 'device_node' and toggles PERST#. If PERST# is not found
> > in the supplied node, the function will look for PERST# in the parent node
> > as a fallback. This is needed since PERST# won't be available in the
> > endpoint node as per the DT binding.
> >
> > Note that the driver still asserts PERST# during the controller
> > initialization as it is needed as per the hardware documentation. Apart
> > from that, the driver wouldn't touch PERST# for the new binding.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 89 +++++++++++++++++++++++++++++-----
> > 1 file changed, 78 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 78355d12f10d263a0bb052e24c1e2d5e8f68603d..3c5c65d7d97cac186e1b671f80ba7296ad226d68 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -276,6 +276,7 @@ struct qcom_pcie_port {
> > struct qcom_pcie_perst {
> > struct list_head list;
> > struct gpio_desc *desc;
> > + struct device_node *np;
> > };
> >
> > struct qcom_pcie {
> > @@ -298,11 +299,50 @@ struct qcom_pcie {
> >
> > #define to_qcom_pcie(x) dev_get_drvdata((x)->dev)
> >
> > -static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
> > +static struct gpio_desc *qcom_find_perst(struct qcom_pcie *pcie, struct device_node *np)
> > +{
> > + struct qcom_pcie_perst *perst;
> > +
> > + list_for_each_entry(perst, &pcie->perst, list) {
> > + if (np == perst->np)
> > + return perst->desc;
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static void qcom_toggle_perst_per_device(struct qcom_pcie *pcie,
> > + struct device_node *np, bool assert)
> > +{
> > + int val = assert ? 1 : 0;
> > + struct gpio_desc *perst;
> > +
> > + perst = qcom_find_perst(pcie, np);
> > + if (perst)
> > + goto toggle_perst;
> > +
> > + /*
> > + * If PERST# is not available in the current node, try the parent. This
> > + * fallback is needed if the current node belongs to an endpoint or
> > + * switch upstream port.
> > + */
> > + if (np->parent)
> > + perst = qcom_find_perst(pcie, np->parent);
>
> Ugh. I think we need to fix the data structures here before we go
> much farther. We should be able to search for PERST# once at probe of
> the Qcom controller. Hopefully we don't need lists of things.
>
> See https://lore.kernel.org/r/20250908183325.GA1450728@bhelgaas.
>
I've added a patch to fix in the next version of this series.
> > +toggle_perst:
> > + /* gpiod* APIs handle NULL gpio_desc gracefully. So no need to check. */
> > + gpiod_set_value_cansleep(perst, val);
> > +}
> > +
> > +static void qcom_perst_reset(struct qcom_pcie *pcie, struct device_node *np,
> > + bool assert)
> > {
> > struct qcom_pcie_perst *perst;
> > int val = assert ? 1 : 0;
> >
> > + if (np)
> > + return qcom_toggle_perst_per_device(pcie, np, assert);
> > +
> > if (list_empty(&pcie->perst))
> > gpiod_set_value_cansleep(pcie->reset, val);
> >
> > @@ -310,22 +350,34 @@ static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
> > gpiod_set_value_cansleep(perst->desc, val);
> > }
> >
> > -static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
> > +static void qcom_ep_reset_assert(struct qcom_pcie *pcie, struct device_node *np)
> > {
> > - qcom_perst_assert(pcie, true);
> > + qcom_perst_reset(pcie, np, true);
> > usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
> > }
> >
> > -static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
> > +static void qcom_ep_reset_deassert(struct qcom_pcie *pcie,
> > + struct device_node *np)
> > {
> > struct dw_pcie_rp *pp = &pcie->pci->pp;
> >
> > msleep(PCIE_T_PVPERL_MS);
> > - qcom_perst_assert(pcie, false);
> > + qcom_perst_reset(pcie, np, false);
> > if (!pp->use_linkup_irq)
> > msleep(PCIE_RESET_CONFIG_WAIT_MS);
> > }
> >
> > +static void qcom_pcie_toggle_perst(struct pci_host_bridge *bridge,
> > + struct device_node *np, bool assert)
> > +{
> > + struct qcom_pcie *pcie = dev_get_drvdata(bridge->dev.parent);
> > +
> > + if (assert)
> > + qcom_ep_reset_assert(pcie, np);
> > + else
> > + qcom_ep_reset_deassert(pcie, np);
> > +}
> > +
> > static int qcom_pcie_start_link(struct dw_pcie *pci)
> > {
> > struct qcom_pcie *pcie = to_qcom_pcie(pci);
> > @@ -1320,7 +1372,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > struct qcom_pcie *pcie = to_qcom_pcie(pci);
> > int ret;
> >
> > - qcom_ep_reset_assert(pcie);
> > + qcom_ep_reset_assert(pcie, NULL);
> >
> > ret = pcie->cfg->ops->init(pcie);
> > if (ret)
> > @@ -1336,7 +1388,13 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
> > goto err_disable_phy;
> > }
> >
> > - qcom_ep_reset_deassert(pcie);
> > + /*
> > + * Only deassert PERST# for all devices here if legacy binding is used.
> > + * For the new binding, pwrctrl driver is expected to toggle PERST# for
> > + * individual devices.
>
> Can we replace "new binding" with something explicit? In a few
> months, "new binding" won't mean anything.
>
So I've introduced a new flag, qcom_pcie::legacy_binding, which gets set if the
driver uses qcom_pcie_parse_legacy_binding(). Based on this flag, PERST# will be
deasserted in this driver.
And I've removed references to 'new binding' term.
- Mani
--
மணிவண்ணன் சதாசிவம்
Hi Manivannan,
kernel test robot noticed the following build errors:
[auto build test ERROR on 8f5ae30d69d7543eee0d70083daf4de8fe15d585]
url: https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam-via-B4-Relay/PCI-qcom-Wait-for-PCIE_RESET_CONFIG_WAIT_MS-after-PERST-deassert/20250903-151623
base: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
patch link: https://lore.kernel.org/r/20250903-pci-pwrctrl-perst-v2-5-2d461ed0e061%40oss.qualcomm.com
patch subject: [PATCH v2 5/5] PCI: qcom: Allow pwrctrl core to toggle PERST# for new DT binding
config: i386-buildonly-randconfig-002-20250904 (https://download.01.org/0day-ci/archive/20250904/202509041110.4DgQKyf1-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.4.0-5) 12.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250904/202509041110.4DgQKyf1-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509041110.4DgQKyf1-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/pci/controller/dwc/pcie-qcom.c: In function 'qcom_pcie_host_init':
>> drivers/pci/controller/dwc/pcie-qcom.c:1405:23: error: 'struct pci_host_bridge' has no member named 'toggle_perst'
1405 | pci->pp.bridge->toggle_perst = qcom_pcie_toggle_perst;
| ^~
vim +1405 drivers/pci/controller/dwc/pcie-qcom.c
1368
1369 static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
1370 {
1371 struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
1372 struct qcom_pcie *pcie = to_qcom_pcie(pci);
1373 int ret;
1374
1375 qcom_ep_reset_assert(pcie, NULL);
1376
1377 ret = pcie->cfg->ops->init(pcie);
1378 if (ret)
1379 return ret;
1380
1381 ret = qcom_pcie_phy_power_on(pcie);
1382 if (ret)
1383 goto err_deinit;
1384
1385 if (pcie->cfg->ops->post_init) {
1386 ret = pcie->cfg->ops->post_init(pcie);
1387 if (ret)
1388 goto err_disable_phy;
1389 }
1390
1391 /*
1392 * Only deassert PERST# for all devices here if legacy binding is used.
1393 * For the new binding, pwrctrl driver is expected to toggle PERST# for
1394 * individual devices.
1395 */
1396 if (list_empty(&pcie->perst))
1397 qcom_ep_reset_deassert(pcie, NULL);
1398
1399 if (pcie->cfg->ops->config_sid) {
1400 ret = pcie->cfg->ops->config_sid(pcie);
1401 if (ret)
1402 goto err_assert_reset;
1403 }
1404
> 1405 pci->pp.bridge->toggle_perst = qcom_pcie_toggle_perst;
1406
1407 return 0;
1408
1409 err_assert_reset:
1410 qcom_ep_reset_assert(pcie, NULL);
1411 err_disable_phy:
1412 qcom_pcie_phy_power_off(pcie);
1413 err_deinit:
1414 pcie->cfg->ops->deinit(pcie);
1415
1416 return ret;
1417 }
1418
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, Sep 04, 2025 at 11:19:30AM GMT, kernel test robot wrote: > Hi Manivannan, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on 8f5ae30d69d7543eee0d70083daf4de8fe15d585] > > url: https://github.com/intel-lab-lkp/linux/commits/Manivannan-Sadhasivam-via-B4-Relay/PCI-qcom-Wait-for-PCIE_RESET_CONFIG_WAIT_MS-after-PERST-deassert/20250903-151623 > base: 8f5ae30d69d7543eee0d70083daf4de8fe15d585 > patch link: https://lore.kernel.org/r/20250903-pci-pwrctrl-perst-v2-5-2d461ed0e061%40oss.qualcomm.com > patch subject: [PATCH v2 5/5] PCI: qcom: Allow pwrctrl core to toggle PERST# for new DT binding > config: i386-buildonly-randconfig-002-20250904 (https://download.01.org/0day-ci/archive/20250904/202509041110.4DgQKyf1-lkp@intel.com/config) > compiler: gcc-12 (Debian 12.4.0-5) 12.4.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250904/202509041110.4DgQKyf1-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202509041110.4DgQKyf1-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > drivers/pci/controller/dwc/pcie-qcom.c: In function 'qcom_pcie_host_init': > >> drivers/pci/controller/dwc/pcie-qcom.c:1405:23: error: 'struct pci_host_bridge' has no member named 'toggle_perst' > 1405 | pci->pp.bridge->toggle_perst = qcom_pcie_toggle_perst; > | ^~ > > We can ignore this error since CONFIG_PWRCTRL_SLOT will be selected by default with this commit: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=controller/qcom&id=add7b05aeeb417c86239e6731a168e6c46b83279 - Mani -- மணிவண்ணன் சதாசிவம்
© 2016 - 2026 Red Hat, Inc.