Hi,

This series is an RFC to propose pwrctrl framework to control the PERST# GPIO
instead of letting the controller drivers to do so (which is a mistake btw).

Right now, the pwrctrl framework is controlling the power supplies to the
components (endpoints and such), but it is not controlling PERST#. This was
pointed out by Brian during a related conversation [1]. But we cannot just move
the PERST# control from controller drivers due to the following reasons:

1. Most of the controller drivers need to assert PERST# during the controller
initialization sequence. This is mostly as per their hardware reference manual
and should not be changed.

2. Controller drivers still need to toggle PERST# when pwrctrl is not used i.e.,
when the power supplies are not accurately described in PCI DT node. This can
happen on unsupported platforms and also for platforms with legacy DTs.

For this reason, I've kept the PERST# retrieval logic in the controller drivers
and just passed the gpio descriptors (for each slot) to the pwrctrl framework.
This will allow both the controller drivers and pwrctrl framework to share the
PERST# (which is ugly but can't be avoided). But care must be taken to ensure
that the controller drivers only assert PERST# and not deassert when pwrctrl is
used. I've added the change for the Qcom driver as a reference. The Qcom driver
is a slight mess because, it now has to support both new DT binding (PERST# and
PHY in Root Port node) and legacy (both in Host Bridge node). So I've allowed
the PERST# control only for the new binding (which is always going to use
pwrctrl framework to control the component supplies).

Testing
=======

This series is tested on Lenovo Thinkpad T14s laptop (with out-of-tree patch
enabling PCIe WLAN card) and on RB3 Gen2 with TC9563 switch (also with the not
yet merged series [2]). A big take away from this series is that, it is now
possible to get rid of the controversial {start/stop}_link() callback proposed
in the above mentioned switch pwrctrl driver [3].

- Mani

[1] https://lore.kernel.org/linux-pci/Z_6kZ7x7gnoH-P7x@google.com/
[2] https://lore.kernel.org/linux-pci/20250412-qps615_v4_1-v5-0-5b6a06132fec@oss.qualcomm.com/ 
[3] https://lore.kernel.org/linux-pci/20250412-qps615_v4_1-v5-4-5b6a06132fec@oss.qualcomm.com/

Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
---
Manivannan Sadhasivam (3):
      PCI/pwrctrl: Move pci_pwrctrl_init() before turning ON the supplies
      PCI/pwrctrl: Allow pwrctrl core to control PERST# GPIO if available
      PCI: qcom: Allow pwrctrl framework to control PERST#

 drivers/pci/controller/dwc/pcie-designware-host.c |  1 +
 drivers/pci/controller/dwc/pcie-designware.h      |  1 +
 drivers/pci/controller/dwc/pcie-qcom.c            | 26 ++++++++++++++-
 drivers/pci/pwrctrl/core.c                        | 39 +++++++++++++++++++++++
 drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c          |  4 +--
 drivers/pci/pwrctrl/slot.c                        |  4 +--
 include/linux/pci-pwrctrl.h                       |  2 ++
 include/linux/pci.h                               |  2 ++
 8 files changed, 74 insertions(+), 5 deletions(-)
---
base-commit: 00f0defc332be94b7f1fdc56ce7dcb6528cdf002
change-id: 20250707-pci-pwrctrl-perst-bdc6e36a335c

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>

Hi Manivannan,

Thanks for tackling this!

On Mon, Jul 07, 2025 at 11:48:37PM +0530, Manivannan Sadhasivam wrote:
> Hi,
> 
> This series is an RFC to propose pwrctrl framework to control the PERST# GPIO
> instead of letting the controller drivers to do so (which is a mistake btw).
> 
> Right now, the pwrctrl framework is controlling the power supplies to the
> components (endpoints and such), but it is not controlling PERST#. This was
> pointed out by Brian during a related conversation [1]. But we cannot just move
> the PERST# control from controller drivers due to the following reasons:
> 
> 1. Most of the controller drivers need to assert PERST# during the controller
> initialization sequence. This is mostly as per their hardware reference manual
> and should not be changed.
> 
> 2. Controller drivers still need to toggle PERST# when pwrctrl is not used i.e.,
> when the power supplies are not accurately described in PCI DT node. This can
> happen on unsupported platforms and also for platforms with legacy DTs.
> 
> For this reason, I've kept the PERST# retrieval logic in the controller drivers
> and just passed the gpio descriptors (for each slot) to the pwrctrl framework.

How sure are we that GPIOs (and *only* GPIOs) are sufficient for this
feature? I've seen a few drivers that pair a GPIO with some kind of
"internal" reset too, and it's not always clear that they can/should be
operated separately.

For example, drivers/pci/controller/dwc/pci-imx6.c /
imx_pcie_{,de}assert_core_reset(), and pcie-tegra194.c's
APPL_PINMUX_PEX_RST. The tegra case especially seems pretty clear that
its non-GPIO "pex_rst" is resetting an endpoint.

> This will allow both the controller drivers and pwrctrl framework to share the
> PERST# (which is ugly but can't be avoided). But care must be taken to ensure
> that the controller drivers only assert PERST# and not deassert when pwrctrl is
> used. I've added the change for the Qcom driver as a reference. The Qcom driver
> is a slight mess because, it now has to support both new DT binding (PERST# and
> PHY in Root Port node) and legacy (both in Host Bridge node). So I've allowed
> the PERST# control only for the new binding (which is always going to use
> pwrctrl framework to control the component supplies).
> 
> Testing
> =======
> 
> This series is tested on Lenovo Thinkpad T14s laptop (with out-of-tree patch
> enabling PCIe WLAN card) and on RB3 Gen2 with TC9563 switch (also with the not
> yet merged series [2]). A big take away from this series is that, it is now
> possible to get rid of the controversial {start/stop}_link() callback proposed
> in the above mentioned switch pwrctrl driver [3].

This is a tiny bit tangential to the PERST# discussion, but I believe
there are other controller driver features that don't fit into the
sequence model of:

1. start LTSSM (controller driver)
2. pwrctrl eventually turns on power + delay per spec
3. pwrctrl deasserts PERST#
4. pwrctrl delays a fixed amount of time, per the CEM spec
5. pwrctrl rescans bus

For example, tegra_pcie_dw_start_link() notes some cases where it needs
to take action and retry when the link doesn't come up. Similarly, I've
seen drivers with retry loops for cases where the link comes up, but not
at the expected link rate. None of this is possible if the controller
driver only gets to take care of #1, and has no involvement in between
#3 and #5.

(Yes, I acknowledge that DWC's .start_link() is being abused in
tegra_pcie_dw_start_link(). But it's still reality, with a
seemingly-good use case.)

Brian

> 
> - Mani
> 
> [1] https://lore.kernel.org/linux-pci/Z_6kZ7x7gnoH-P7x@google.com/
> [2] https://lore.kernel.org/linux-pci/20250412-qps615_v4_1-v5-0-5b6a06132fec@oss.qualcomm.com/ 
> [3] https://lore.kernel.org/linux-pci/20250412-qps615_v4_1-v5-4-5b6a06132fec@oss.qualcomm.com/
> 
> Signed-off-by: Manivannan Sadhasivam <mani@kernel.org>
> ---
> Manivannan Sadhasivam (3):
>       PCI/pwrctrl: Move pci_pwrctrl_init() before turning ON the supplies
>       PCI/pwrctrl: Allow pwrctrl core to control PERST# GPIO if available
>       PCI: qcom: Allow pwrctrl framework to control PERST#
> 
>  drivers/pci/controller/dwc/pcie-designware-host.c |  1 +
>  drivers/pci/controller/dwc/pcie-designware.h      |  1 +
>  drivers/pci/controller/dwc/pcie-qcom.c            | 26 ++++++++++++++-
>  drivers/pci/pwrctrl/core.c                        | 39 +++++++++++++++++++++++
>  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c          |  4 +--
>  drivers/pci/pwrctrl/slot.c                        |  4 +--
>  include/linux/pci-pwrctrl.h                       |  2 ++
>  include/linux/pci.h                               |  2 ++
>  8 files changed, 74 insertions(+), 5 deletions(-)
> ---
> base-commit: 00f0defc332be94b7f1fdc56ce7dcb6528cdf002
> change-id: 20250707-pci-pwrctrl-perst-bdc6e36a335c
> 
> Best regards,
> -- 
> Manivannan Sadhasivam <mani@kernel.org>
>

On Tue, Jul 08, 2025 at 06:39:37PM GMT, Brian Norris wrote:
> Hi Manivannan,
> 
> Thanks for tackling this!
> 
> On Mon, Jul 07, 2025 at 11:48:37PM +0530, Manivannan Sadhasivam wrote:
> > Hi,
> > 
> > This series is an RFC to propose pwrctrl framework to control the PERST# GPIO
> > instead of letting the controller drivers to do so (which is a mistake btw).
> > 
> > Right now, the pwrctrl framework is controlling the power supplies to the
> > components (endpoints and such), but it is not controlling PERST#. This was
> > pointed out by Brian during a related conversation [1]. But we cannot just move
> > the PERST# control from controller drivers due to the following reasons:
> > 
> > 1. Most of the controller drivers need to assert PERST# during the controller
> > initialization sequence. This is mostly as per their hardware reference manual
> > and should not be changed.
> > 
> > 2. Controller drivers still need to toggle PERST# when pwrctrl is not used i.e.,
> > when the power supplies are not accurately described in PCI DT node. This can
> > happen on unsupported platforms and also for platforms with legacy DTs.
> > 
> > For this reason, I've kept the PERST# retrieval logic in the controller drivers
> > and just passed the gpio descriptors (for each slot) to the pwrctrl framework.
> 
> How sure are we that GPIOs (and *only* GPIOs) are sufficient for this
> feature? I've seen a few drivers that pair a GPIO with some kind of
> "internal" reset too, and it's not always clear that they can/should be
> operated separately.
> 
> For example, drivers/pci/controller/dwc/pci-imx6.c /
> imx_pcie_{,de}assert_core_reset(), and pcie-tegra194.c's
> APPL_PINMUX_PEX_RST. The tegra case especially seems pretty clear that
> its non-GPIO "pex_rst" is resetting an endpoint.
> 

Right. But GPIO is the most commonly used approach for implementing PERST# and
it is the one supported on the platform I'm testing with. So I just went with
that. For sure there are other methods exist and the PCIe spec itself doesn't
define how PERST# should be implemented in a form factor. It merely defines
PERST# as an 'auxiliary signal'. So yes, other form of PERST# can exist. But for
the sake of keeping this proposal simple, I'm considering only GPIO based
PERST# atm.

Also, Tegra platforms are not converted to use pwrctrl framework and I don't
know if the platform maintainers are interested in it or not. But if they start
using it, we can tackle this situation by introducing a callback that
asserts/deasserts PERST# (yes, callbacks are evil, but I don't know any other
sensible way to support vendor specific PERST# implementations).

Oh and do take a look at pcie-brcmstb driver, which I promised to move to
pwrctrl framework for another reason. It uses multiple callbacks per SoC
revisions for toggling PERST#. So for these usecases, having a callback in
'struct pci_host_bridge' would be a good fit and I may introduce it after this
series gets in.

> > This will allow both the controller drivers and pwrctrl framework to share the
> > PERST# (which is ugly but can't be avoided). But care must be taken to ensure
> > that the controller drivers only assert PERST# and not deassert when pwrctrl is
> > used. I've added the change for the Qcom driver as a reference. The Qcom driver
> > is a slight mess because, it now has to support both new DT binding (PERST# and
> > PHY in Root Port node) and legacy (both in Host Bridge node). So I've allowed
> > the PERST# control only for the new binding (which is always going to use
> > pwrctrl framework to control the component supplies).
> > 
> > Testing
> > =======
> > 
> > This series is tested on Lenovo Thinkpad T14s laptop (with out-of-tree patch
> > enabling PCIe WLAN card) and on RB3 Gen2 with TC9563 switch (also with the not
> > yet merged series [2]). A big take away from this series is that, it is now
> > possible to get rid of the controversial {start/stop}_link() callback proposed
> > in the above mentioned switch pwrctrl driver [3].
> 
> This is a tiny bit tangential to the PERST# discussion, but I believe
> there are other controller driver features that don't fit into the
> sequence model of:
> 
> 1. start LTSSM (controller driver)
> 2. pwrctrl eventually turns on power + delay per spec
> 3. pwrctrl deasserts PERST#
> 4. pwrctrl delays a fixed amount of time, per the CEM spec
> 5. pwrctrl rescans bus
> 
> For example, tegra_pcie_dw_start_link() notes some cases where it needs
> to take action and retry when the link doesn't come up. Similarly, I've
> seen drivers with retry loops for cases where the link comes up, but not
> at the expected link rate. None of this is possible if the controller
> driver only gets to take care of #1, and has no involvement in between
> #3 and #5.
> 

Having this back and forth communication would make the pwrctrl driver a lot
messier. But I believe, we could teach pwrctrl driver to detect link up (similar
to dw_pcie_wait_for_link()) and if link didn't come up, it could do retry and
other steps with help from controller drivers. But these things should be
implemented only when platforms like Tegra start to show some love towards
pwrctrl.

- Mani

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

Hi,

On Wed, Jul 09, 2025 at 12:18:29PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Jul 08, 2025 at 06:39:37PM GMT, Brian Norris wrote:
> > On Mon, Jul 07, 2025 at 11:48:37PM +0530, Manivannan Sadhasivam wrote:
> > > Hi,
> > > 
> > > This series is an RFC to propose pwrctrl framework to control the PERST# GPIO
> > > instead of letting the controller drivers to do so (which is a mistake btw).
> > > 
> > > Right now, the pwrctrl framework is controlling the power supplies to the
> > > components (endpoints and such), but it is not controlling PERST#. This was
> > > pointed out by Brian during a related conversation [1]. But we cannot just move
> > > the PERST# control from controller drivers due to the following reasons:
> > > 
> > > 1. Most of the controller drivers need to assert PERST# during the controller
> > > initialization sequence. This is mostly as per their hardware reference manual
> > > and should not be changed.
> > > 
> > > 2. Controller drivers still need to toggle PERST# when pwrctrl is not used i.e.,
> > > when the power supplies are not accurately described in PCI DT node. This can
> > > happen on unsupported platforms and also for platforms with legacy DTs.
> > > 
> > > For this reason, I've kept the PERST# retrieval logic in the controller drivers
> > > and just passed the gpio descriptors (for each slot) to the pwrctrl framework.
> > 
> > How sure are we that GPIOs (and *only* GPIOs) are sufficient for this
> > feature? I've seen a few drivers that pair a GPIO with some kind of
> > "internal" reset too, and it's not always clear that they can/should be
> > operated separately.
> > 
> > For example, drivers/pci/controller/dwc/pci-imx6.c /
> > imx_pcie_{,de}assert_core_reset(), and pcie-tegra194.c's
> > APPL_PINMUX_PEX_RST. The tegra case especially seems pretty clear that
> > its non-GPIO "pex_rst" is resetting an endpoint.
> > 
> 
> Right. But GPIO is the most commonly used approach for implementing PERST# and
> it is the one supported on the platform I'm testing with. So I just went with
> that. For sure there are other methods exist and the PCIe spec itself doesn't
> define how PERST# should be implemented in a form factor. It merely defines
> PERST# as an 'auxiliary signal'. So yes, other form of PERST# can exist. But for
> the sake of keeping this proposal simple, I'm considering only GPIO based
> PERST# atm.

Hmm, OK. A simple start is fine, but I'm pointing out this will quickly
show its limitations.

> Also, Tegra platforms are not converted to use pwrctrl framework and I don't
> know if the platform maintainers are interested in it or not. But if they start
> using it, we can tackle this situation by introducing a callback that
> asserts/deasserts PERST# (yes, callbacks are evil, but I don't know any other
> sensible way to support vendor specific PERST# implementations).

IMO, it's pretty fair game to at least account for things people are
doing in upstream drivers today, even if they aren't wholly ready to
adopt the new thing. It's harder to gain new users when you actively
don't support things you know the users need.

> Oh and do take a look at pcie-brcmstb driver, which I promised to move to
> pwrctrl framework for another reason. It uses multiple callbacks per SoC
> revisions for toggling PERST#. So for these usecases, having a callback in
> 'struct pci_host_bridge' would be a good fit and I may introduce it after this
> series gets in.

Sure. I think there are plenty of drivers that will need it. And that's
why I brought it up.

But if that's a "phase 2" thing, so be it.

> > > This will allow both the controller drivers and pwrctrl framework to share the
> > > PERST# (which is ugly but can't be avoided). But care must be taken to ensure
> > > that the controller drivers only assert PERST# and not deassert when pwrctrl is
> > > used. I've added the change for the Qcom driver as a reference. The Qcom driver
> > > is a slight mess because, it now has to support both new DT binding (PERST# and
> > > PHY in Root Port node) and legacy (both in Host Bridge node). So I've allowed
> > > the PERST# control only for the new binding (which is always going to use
> > > pwrctrl framework to control the component supplies).
> > > 
> > > Testing
> > > =======
> > > 
> > > This series is tested on Lenovo Thinkpad T14s laptop (with out-of-tree patch
> > > enabling PCIe WLAN card) and on RB3 Gen2 with TC9563 switch (also with the not
> > > yet merged series [2]). A big take away from this series is that, it is now
> > > possible to get rid of the controversial {start/stop}_link() callback proposed
> > > in the above mentioned switch pwrctrl driver [3].
> > 
> > This is a tiny bit tangential to the PERST# discussion, but I believe
> > there are other controller driver features that don't fit into the
> > sequence model of:
> > 
> > 1. start LTSSM (controller driver)
> > 2. pwrctrl eventually turns on power + delay per spec
> > 3. pwrctrl deasserts PERST#
> > 4. pwrctrl delays a fixed amount of time, per the CEM spec
> > 5. pwrctrl rescans bus
> > 
> > For example, tegra_pcie_dw_start_link() notes some cases where it needs
> > to take action and retry when the link doesn't come up. Similarly, I've
> > seen drivers with retry loops for cases where the link comes up, but not
> > at the expected link rate. None of this is possible if the controller
> > driver only gets to take care of #1, and has no involvement in between
> > #3 and #5.
> > 
> 
> Having this back and forth communication would make the pwrctrl driver a lot
> messier. But I believe, we could teach pwrctrl driver to detect link up (similar
> to dw_pcie_wait_for_link()) and if link didn't come up, it could do retry and
> other steps with help from controller drivers. But these things should be
> implemented only when platforms like Tegra start to show some love towards
> pwrctrl.

Never mind the lack of love you feel here :)
But I'm actively looking at drivers that don't yet fit into what pwrctrl
supports, and I'd like them to use pwrctrl someday instead of
reinventing the wheel.

You're arguing against more callbacks, and start_link()-like
functionality, but I'm pretty sure some of these things are necessities,
if you're trying to abstract power control away from controller drivers.

Again, maybe this is a problem to be solved later. But I think you're
kidding yourself that pwrctrl is ready as-is, and that you can avoid
these kinds of callbacks.

Regards,
Brian

On Fri, Jul 11, 2025 at 05:04:38PM GMT, Brian Norris wrote:
> Hi,
> 
> On Wed, Jul 09, 2025 at 12:18:29PM +0530, Manivannan Sadhasivam wrote:
> > On Tue, Jul 08, 2025 at 06:39:37PM GMT, Brian Norris wrote:
> > > On Mon, Jul 07, 2025 at 11:48:37PM +0530, Manivannan Sadhasivam wrote:
> > > > Hi,
> > > > 
> > > > This series is an RFC to propose pwrctrl framework to control the PERST# GPIO
> > > > instead of letting the controller drivers to do so (which is a mistake btw).
> > > > 
> > > > Right now, the pwrctrl framework is controlling the power supplies to the
> > > > components (endpoints and such), but it is not controlling PERST#. This was
> > > > pointed out by Brian during a related conversation [1]. But we cannot just move
> > > > the PERST# control from controller drivers due to the following reasons:
> > > > 
> > > > 1. Most of the controller drivers need to assert PERST# during the controller
> > > > initialization sequence. This is mostly as per their hardware reference manual
> > > > and should not be changed.
> > > > 
> > > > 2. Controller drivers still need to toggle PERST# when pwrctrl is not used i.e.,
> > > > when the power supplies are not accurately described in PCI DT node. This can
> > > > happen on unsupported platforms and also for platforms with legacy DTs.
> > > > 
> > > > For this reason, I've kept the PERST# retrieval logic in the controller drivers
> > > > and just passed the gpio descriptors (for each slot) to the pwrctrl framework.
> > > 
> > > How sure are we that GPIOs (and *only* GPIOs) are sufficient for this
> > > feature? I've seen a few drivers that pair a GPIO with some kind of
> > > "internal" reset too, and it's not always clear that they can/should be
> > > operated separately.
> > > 
> > > For example, drivers/pci/controller/dwc/pci-imx6.c /
> > > imx_pcie_{,de}assert_core_reset(), and pcie-tegra194.c's
> > > APPL_PINMUX_PEX_RST. The tegra case especially seems pretty clear that
> > > its non-GPIO "pex_rst" is resetting an endpoint.
> > > 
> > 
> > Right. But GPIO is the most commonly used approach for implementing PERST# and
> > it is the one supported on the platform I'm testing with. So I just went with
> > that. For sure there are other methods exist and the PCIe spec itself doesn't
> > define how PERST# should be implemented in a form factor. It merely defines
> > PERST# as an 'auxiliary signal'. So yes, other form of PERST# can exist. But for
> > the sake of keeping this proposal simple, I'm considering only GPIO based
> > PERST# atm.
> 
> Hmm, OK. A simple start is fine, but I'm pointing out this will quickly
> show its limitations.
> 

I don't disagree :)

> > Also, Tegra platforms are not converted to use pwrctrl framework and I don't
> > know if the platform maintainers are interested in it or not. But if they start
> > using it, we can tackle this situation by introducing a callback that
> > asserts/deasserts PERST# (yes, callbacks are evil, but I don't know any other
> > sensible way to support vendor specific PERST# implementations).
> 
> IMO, it's pretty fair game to at least account for things people are
> doing in upstream drivers today, even if they aren't wholly ready to
> adopt the new thing. It's harder to gain new users when you actively
> don't support things you know the users need.
> 
> > Oh and do take a look at pcie-brcmstb driver, which I promised to move to
> > pwrctrl framework for another reason. It uses multiple callbacks per SoC
> > revisions for toggling PERST#. So for these usecases, having a callback in
> > 'struct pci_host_bridge' would be a good fit and I may introduce it after this
> > series gets in.
> 
> Sure. I think there are plenty of drivers that will need it. And that's
> why I brought it up.
> 
> But if that's a "phase 2" thing, so be it.
> 
> > > > This will allow both the controller drivers and pwrctrl framework to share the
> > > > PERST# (which is ugly but can't be avoided). But care must be taken to ensure
> > > > that the controller drivers only assert PERST# and not deassert when pwrctrl is
> > > > used. I've added the change for the Qcom driver as a reference. The Qcom driver
> > > > is a slight mess because, it now has to support both new DT binding (PERST# and
> > > > PHY in Root Port node) and legacy (both in Host Bridge node). So I've allowed
> > > > the PERST# control only for the new binding (which is always going to use
> > > > pwrctrl framework to control the component supplies).
> > > > 
> > > > Testing
> > > > =======
> > > > 
> > > > This series is tested on Lenovo Thinkpad T14s laptop (with out-of-tree patch
> > > > enabling PCIe WLAN card) and on RB3 Gen2 with TC9563 switch (also with the not
> > > > yet merged series [2]). A big take away from this series is that, it is now
> > > > possible to get rid of the controversial {start/stop}_link() callback proposed
> > > > in the above mentioned switch pwrctrl driver [3].
> > > 
> > > This is a tiny bit tangential to the PERST# discussion, but I believe
> > > there are other controller driver features that don't fit into the
> > > sequence model of:
> > > 
> > > 1. start LTSSM (controller driver)
> > > 2. pwrctrl eventually turns on power + delay per spec
> > > 3. pwrctrl deasserts PERST#
> > > 4. pwrctrl delays a fixed amount of time, per the CEM spec
> > > 5. pwrctrl rescans bus
> > > 
> > > For example, tegra_pcie_dw_start_link() notes some cases where it needs
> > > to take action and retry when the link doesn't come up. Similarly, I've
> > > seen drivers with retry loops for cases where the link comes up, but not
> > > at the expected link rate. None of this is possible if the controller
> > > driver only gets to take care of #1, and has no involvement in between
> > > #3 and #5.
> > > 
> > 
> > Having this back and forth communication would make the pwrctrl driver a lot
> > messier. But I believe, we could teach pwrctrl driver to detect link up (similar
> > to dw_pcie_wait_for_link()) and if link didn't come up, it could do retry and
> > other steps with help from controller drivers. But these things should be
> > implemented only when platforms like Tegra start to show some love towards
> > pwrctrl.
> 
> Never mind the lack of love you feel here :)
> But I'm actively looking at drivers that don't yet fit into what pwrctrl
> supports, and I'd like them to use pwrctrl someday instead of
> reinventing the wheel.
> 

I'm not against supporting these controller drivers/platforms in pwrctrl. It's
just that I do not want to implement solutions now without having users. But I'm
glad that you are bringing these up. I'm adding these to my pwrctrl/todo list.

> You're arguing against more callbacks, and start_link()-like
> functionality, but I'm pretty sure some of these things are necessities,
> if you're trying to abstract power control away from controller drivers.
> 

Well, that's my personal preference. These days I feel (mostly after spending my
time as a maintainer of PCI controller drivers) that callbacks are evil and they
pay way for 'midlayers'. But having said that, I myself implemented callbacks
whereever I felt that no other options were possible. And here also, I'm just
claiming that this series avoids the '{start/stop}_link' callbacks which was
hated by many (including me) as it ties the Qcom switch driver implementing
these callbacks to be tied to a specific platform.

> Again, maybe this is a problem to be solved later. But I think you're
> kidding yourself that pwrctrl is ready as-is, and that you can avoid
> these kinds of callbacks.
> 

No, I never claimed that pwrctrl is perfect and it would solve all the PCI power
issues. But I'm happy that it atleast solves a couple of issues and allows me to
address the rest of them. We had been talking about a framework like this for
several years without any upstream solution. But now we finally have one and I'm
merely trying to make use of it to address issues.

- Mani

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