BPF List
 help / color / mirror / Atom feed
From: Daniel Thompson <daniel@riscstar.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>,
	Alex Elder <elder@riscstar.com>,
	andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
	kuba@kernel.org, pabeni@redhat.com,
	maxime.chevallier@bootlin.com, rmk+kernel@armlinux.org.uk,
	andersson@kernel.org, konradybcio@kernel.org, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, linusw@kernel.org,
	brgl@kernel.org, arnd@arndb.de, gregkh@linuxfoundation.org,
	mohd.anwar@oss.qualcomm.com, a0987203069@gmail.com,
	alexandre.torgue@foss.st.com, ast@kernel.org,
	boon.khai.ng@altera.com, chenchuangyu@xiaomi.com,
	chenhuacai@kernel.org, daniel@iogearbox.net, hawk@kernel.org,
	hkallweit1@gmail.com, inochiama@gmail.com,
	john.fastabend@gmail.com, julianbraha@gmail.com,
	livelycarpet87@gmail.com, matthew.gerlach@altera.com,
	mcoquelin.stm32@gmail.com, me@ziyao.cc,
	prabhakar.mahadev-lad.rj@bp.renesas.com,
	richardcochran@gmail.com, rohan.g.thomas@altera.com,
	sdf@fomichev.me, siyanteng@cqsoftware.com.cn,
	weishangjuan@eswincomputing.com, wens@kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-gpio@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy
Date: Thu, 14 May 2026 16:23:41 +0100	[thread overview]
Message-ID: <agXo_evi1oFLBJoo@aspen.lan> (raw)
In-Reply-To: <3c6e7ec5-f600-44ee-a97a-211a99102744@lunn.ch>

On Wed, May 13, 2026 at 04:35:13PM +0200, Andrew Lunn wrote:
> > However the real reason we jammed this on is because I couldn't find a
> > way to get the phy/mdio code to turn one on. However it is possible to
> > add regulator support to MDIO devices by extending their existing logic
> > to manage resets so it can also manage a regulator. It comes out fairly
> > clean so we can add that to the patch set and remove the
> > regulator-always-on.
>
> We, I have rejected this before. It might look clean and easy, but it
> is not. How do you determine the order of enabling reset, regulators
> clocks? How do you specify the need sleeps in between these different
> operations?

I agree it is only easy for the easy cases. I can even be specific
that, for me, the easy cases mean enabling a single regulator and where
power-on reset is not significantly slower than reset-pin reset
meaning we don’t need to distinguish between a power-cycling reset and
a reset-pin reset).

However I think that complex cases cannot be handled by generic MDIO
code. I think these cases are best solved from the MDIO probe method of
individual PHY drivers.


> There is nothing in particular MDIO specific here, and there is
> generic power sequencing code in the kernel. And a while back,
> somebody said they would look at what is needed to make MDIO busses
> and Ethernet PHYs make use of that generic power sequencing code. That
> is the better way to do this.

I’d love to know if they got anywhere. I’ve reviewed the pwrseq
subsystem and I don’t think it is intended to solve the problems
presented by the ethernet phys on a rb3gen2. Let me try to explain my
reasoning. Maybe someone will be able to point out what I have missed!

I’m afraid this must be necessarily long but to help navigate the
general structure is:

 1. Quick review of the pwrseq subsystem
 2. Quick review of the PCI power control driver, both the concept and
    to note why some PCI power control drivers do not use pwrseq
 3. Summarize how this works on DT
 4. Introduce a description of an qca8081 in “modern devicetree”
 5. Examine whether pwrseq or the power control driver concept would
    make writing MDIO device drivers easier than “just doing it in the
    probe method”

pwrseq is only generic in the sense that has a compact consumer API and
that it provides reusable tools that allow *specific* power sequencing
drivers to be written relatively easily. The specific drivers created
with these tools typically bind to something concrete in the DT (e.g.
compatible = "pcie-m2-m-connector" or "qcom,wcn6855-pmu"): things you
can point to such as the M.2 slot or a special purpose power management
unit in a combo chip.

The pwrseq core allows each driver to register a .match() method to
allow pwrseq driver recognise that another driver has asked for its
help to enable a pwrseq target. They usually match, not by compatible
but by traversing the phandle relationships from the device’s DT node
to verify that they link back to the pwrseq driver in the expected
manner.

For ethernet phys I'm doubtful pwrseq offers any benefit because most
phys are standalone and therefore just get a bunch of board level
regulators: there isn’t anything concrete in the DT for which we can
instantiate a pwrseq device. Without a device it is difficult to fire
up a driver that is responsible for knowing what power sequence is
needed to activate the phy. It is therefore better to encode this
knowledge in the phy driver instead.

Another related-but-different concept is PCI power control drivers.
When pwrseq was introduced the first client was a PCI power control
driver. PCI power control drivers are used to solve chicken-and-egg bus
enumeration problems. They work binding a platform bus driver to any
PCI device with a compatible string (e.g. compatible = "pci17cb,1103")
before attempting to enumerate the real device. The platform device
driver responsible for turning on the power ready for enumeration but
does not drive the actual PCI device. This ensures the device can
respond to enumeration requests and, eventually, probe the “real” PCI
driver.

Note that the PCI power control drivers do not have to use the pwrseq
framework to turn the power on. There are examples of both in the
current tree:

* pci-pwrctrl-pwrseq.c uses pwrseq and is, at it's core, just table of
  compatible strings and pwrseq target names. This allows it recognize,
  from the compatible string what pwrseq target to request.
  For example on wcn6855 it can request that only the WLAN hardware
  be enabled (BT power-on is requested separately before using the
  HCI UART).

* pci-pwrctrl-tc9563.c is the power controller driver for the
  TC956x PCIe switch. This is an example of the case where there
  is nothing for a pwrseq driver to bind to. TC956x just gets a bunch
  of individual regulators and a reset line. For that reason
  pci-pwrctrl-tc9563 just uses the regular C code to grab everything
  is needs, relying on things like the bulk regulator helpers keep the
  code as compact as possible.

One key insight about the above is that there are three separate device
drivers parsing specific properties of the node. Having all the
properties related to a device in a single node was very strongly
pushed for by Rob H[1] and AFAIK is a key expectation for new DT
bindings. This strongly influences the scope of pwrseq and PCI power
control (and power domains) and discourages giving DT a generic means
to express power sequencing. That knowledge is handed to us implicitly
by the compatible string!

For example, in the case of WCN6855[2] we have:

1. pci-pwrctrl-pwrseq which uses the compatible string to get itself
   bound and to decide what pwrseq target to enable.
2. pwrseq-qcom-wcn which reads all the *-supply properties together
   with a couple of clocks and enable lines. It ensures we don't
   enable anything until the power lines have settled.
3. ath11k[_pci] which isn’t probed until the device is “on” and
   then reads the remaining properties such as qcom,calibration-variant

[1]: https://lore.kernel.org/all/CAL_JsqLAnJqZ95_bf6_fFmPJFMjuy43UfP2UxzEmFMNnG_t-Ug@mail.gmail.com/
[2]: https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts#L987C1-L1004C3

In short, the pwrseq client API is beguilingly simple but that does not
mean the providers are generic. In the above example there must still
be something, written in C, that contains knowledge of how to turn on a
wcn6855.

Let’s turn our attention to MDIO.

Following the pattern above where all the links related to power come
from the main device node, then the phy node for the qca8081 in an
rb3gen2 would look like something like this:

    tc956x_emac1_phy: ethernet-phy@1c {
        compatible = "ethernet-phy-id004d.d101";
        reg = <0x1c>;

        reset-gpios = <&tc956x_emac0 1 GPIO_ACTIVE_LOW>;
        # On RB3gen2 all supplies are controlled by a single GPIO
        # so we link all supplies to that single regulator
        avdd-supply = <&qep_1p8>;
        avdd18-supply = <&qep_1p8>;
        vdd-supply = <&qep_1p8>;
        vddldo-supply = <&qep_1p8>;
        vdd18-supply = <&qep_1p8>;
        vdd125-supply = <&qep_1p8>;

        pinctrl-names = "default";
        pinctrl-0 = <&qep_irq_pin>;
        interrupts-extended = <&tlmm 101 IRQ_TYPE_LEVEL_LOW>;
    };

Once we have established what the DT looks like then the question
becomes where to put "something, written in C [or Rust], that contains
burned in knowledge of how to turn on a wcn6855^H^H^H^H^H^H^Hqca8081"?

The qca8081 on rb3gen2 just gets a bunch of individual regulators and a
reset. As in the pci-pwrctrl-tc9563 example above, this means there is
nothing in the DT for a pwrseq driver to bind to. Even if we could find
a way to do that, it is not obviously useful to decouple how to turn on
an MDIO device from how to drive it. Thus I think the right answer to
that is to put the code to fire up the regulators into the qca808x.c
driver and it looks to me like the existing probe/remove methods would
already work perfectly well as the place to put it.

Does MDIO bus code need to know about pwrseq at all? I don't think so.
Perhaps there are phys that are suitable to be managed via pwrseq
because they are part of a larger ethernet chip with some kind of PMU,
but that doesn’t require it to be exposed outside the driver. The
drivers for such a phy can simply call the pwrseq APIs from its probe
method.

Do we need something equivalent to PCI power control for the MDIO bus?
I don't think the same chicken-and-egg problem actually exists for MDIO
bus. If a subnode with a compatible string (and regulators) exists we
don't need to scan that address because we already know enough about
the bus to probe the driver and therefore can let the driver handle
turning on the power (just like we do for I2C or SPI drivers).

MDIO does have bus scanning but we only need to scan for things we
don’t know about and must do so on the assumption they are already
powered on. That’s because when we don’t know what’s there then your
earlier question ("How do you determine the order of enabling reset,
regulators clocks?") is impossible to answer. There could be a generic
bus-supply property to handle easy cases where a single regulator
activates everything on the bus although this isn’t needed on RB3gen2
since we statically know what is on the bus.

If you've got this far and not found a fatal mistake in the reasoning
then perhaps it sounds like an awful lot of churn to have to modify
each PHY driver every time that PHY is used in an embedded platform
with software controlled regulators!

That, in a nutshell, is why I was tempted to copy the phy-supply
property to cover the "easy cases" we discussed at the outset. Doing so
does nothing to impede the hard cases since the code for that would
still work fine from the MDIO probe method. At worst a driver might
have to register with a flag to suppress any generic power/reset logic
from the core (since it's obviously wrong to wait for a reset to
complete on a device that isn't powered up).

To be clear I’m very happy to back away from phy-supply. We use the same
patterns we see in I2C and SPI drivers and rely purely on probe methods
instead to turn on regulators. However I don’t see how to exploit power
sequencing code to help describe these things generically.


Daniel.

  reply	other threads:[~2026-05-14 15:23 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-01 15:54 [PATCH net-next 00/12] net: enable TC956x support Alex Elder
2026-05-01 15:54 ` [PATCH net-next 01/12] net: pcs: pcs-xpcs-regmap: support XPCS memory-mapped MDIO bus via regmap Alex Elder
2026-05-02 15:56   ` sashiko-bot
2026-05-01 15:54 ` [PATCH net-next 02/12] net: pcs: pcs-xpcs: select operating mode for 10G-baseR capable PCS Alex Elder
2026-05-01 16:50   ` Andrew Lunn
2026-05-01 18:07     ` Alex Elder
2026-05-05 15:58     ` Daniel Thompson
2026-05-02 15:56   ` sashiko-bot
2026-05-01 15:54 ` [PATCH net-next 03/12] net: pcs: pcs-xpcs: Preserve BMCR_ANENBLE during link up Alex Elder
2026-05-01 17:06   ` Andrew Lunn
2026-05-06  9:46     ` Daniel Thompson
2026-05-02 15:56   ` sashiko-bot
2026-05-01 15:54 ` [PATCH net-next 04/12] net: stmmac: dma: create a separate dma_device pointer Alex Elder
2026-05-01 17:13   ` Andrew Lunn
2026-05-01 18:06     ` Alex Elder
2026-05-01 20:55       ` Andrew Lunn
2026-05-04 13:36         ` Alex Elder
2026-05-02 15:56   ` sashiko-bot
2026-05-01 15:54 ` [PATCH net-next 05/12] net: stmmac: dwxgmac2: Add multi MSI interrupt mode Alex Elder
2026-05-01 17:21   ` Andrew Lunn
2026-05-01 15:54 ` [PATCH net-next 06/12] net: stmmac: dwxgmac2: Add XGMAC 3.01a support Alex Elder
2026-05-01 15:54 ` [PATCH net-next 07/12] net: stmmac: dwxgmac2: export symbols for XGMAC 3.01a DMA Alex Elder
2026-05-02 15:56   ` sashiko-bot
2026-05-01 15:54 ` [PATCH net-next 08/12] dt-bindings: net: toshiba,tc965x-dwmac: add TC956x Ethernet bridge Alex Elder
2026-05-01 17:38   ` Andrew Lunn
2026-05-03  2:22     ` Alex Elder
2026-05-07 22:17     ` Alex Elder
2026-05-07 23:39       ` Rob Herring
2026-05-02 15:56   ` sashiko-bot
2026-05-07 19:02     ` Rob Herring
2026-05-08 14:36       ` Alex Elder
2026-05-04 11:00   ` Krzysztof Kozlowski
2026-05-04 13:34     ` Alex Elder
2026-05-07 14:47     ` Daniel Thompson
2026-05-07 14:12   ` Bjorn Andersson
2026-05-07 14:19     ` Andrew Lunn
2026-05-07 16:12       ` Bjorn Andersson
2026-05-07 18:37     ` Alex Elder
2026-05-10  2:25       ` Bjorn Andersson
2026-05-07 23:41   ` Rob Herring
2026-05-01 15:54 ` [PATCH net-next 09/12] gpio: tc956x: add TC956x/QPS615 support Alex Elder
2026-05-01 18:36   ` Andrew Lunn
2026-05-03  1:45     ` Alex Elder
2026-05-03  2:48       ` Andrew Lunn
2026-05-07 18:39         ` Alex Elder
2026-05-03  3:05       ` Andrew Lunn
2026-05-06 18:21         ` Alex Elder
2026-05-06 19:43           ` Andrew Lunn
2026-05-06 20:25             ` Alex Elder
2026-05-06 21:43               ` Andrew Lunn
2026-05-06 22:41                 ` Alex Elder
2026-05-02 15:56   ` sashiko-bot
2026-05-03  3:42   ` Julian Braha
2026-05-06 18:51     ` Alex Elder
2026-05-04 12:46   ` Bartosz Golaszewski
2026-05-04 13:07     ` Alex Elder
2026-05-07 12:15   ` Linus Walleij
2026-05-07 12:20     ` Alex Elder
2026-05-01 15:54 ` [PATCH net-next 10/12] net: stmmac: " Alex Elder
2026-05-01 19:04   ` Andrew Lunn
2026-05-07 16:03     ` Daniel Thompson
2026-05-07 16:29       ` Andrew Lunn
2026-05-08 11:25         ` Daniel Thompson
2026-05-08 13:34           ` Andrew Lunn
2026-05-08 15:54             ` Daniel Thompson
2026-05-02 15:56   ` sashiko-bot
2026-05-05 16:38   ` Mohd Ayaan Anwar
2026-05-05 16:46     ` Alex Elder
2026-05-06  2:30   ` Xilin Wu
2026-05-06 17:44     ` Alex Elder
2026-05-07 13:57       ` Xilin Wu
2026-05-07 14:14         ` Andrew Lunn
2026-05-11 15:41         ` Daniel Thompson
2026-05-06 12:59   ` Xilin Wu
2026-05-06 14:19     ` Andrew Lunn
2026-05-06 14:35       ` Xilin Wu
2026-05-06 14:45         ` Andrew Lunn
2026-05-06 15:38           ` Xilin Wu
2026-05-06 15:39         ` Daniel Thompson
2026-05-06 15:44           ` Xilin Wu
2026-05-06 15:56             ` Andrew Lunn
2026-05-06 16:00               ` Xilin Wu
2026-05-06 15:28       ` Daniel Thompson
2026-05-06 19:52         ` Andrew Lunn
2026-05-07 18:44     ` Alex Elder
2026-05-08 13:09       ` Xilin Wu
2026-05-08 13:36         ` Andrew Lunn
2026-05-08 13:41           ` Xilin Wu
2026-05-01 15:54 ` [PATCH net-next 11/12] misc: tc956x_pci: " Alex Elder
2026-05-01 21:07   ` Andrew Lunn
2026-05-03  2:06     ` Alex Elder
2026-05-02 15:56   ` sashiko-bot
2026-05-02 16:45   ` Jakub Kicinski
2026-05-03  2:06     ` Alex Elder
2026-05-03  2:14       ` Jakub Kicinski
2026-05-03  2:23         ` Alex Elder
2026-05-01 15:54 ` [PATCH net-next 12/12] arm64: dts: qcom: qcs6490-rb3gen2: enable TC9564 with a single QCS8081 phy Alex Elder
2026-05-01 21:09   ` Andrew Lunn
2026-05-05 16:25     ` Daniel Thompson
2026-05-05 16:42   ` Mohd Ayaan Anwar
2026-05-05 16:46     ` Alex Elder
2026-05-08 14:03   ` Konrad Dybcio
2026-05-13 12:49     ` Daniel Thompson
2026-05-13 14:35       ` Andrew Lunn
2026-05-14 15:23         ` Daniel Thompson [this message]
2026-05-14 16:14           ` Andrew Lunn
2026-05-02 16:47 ` [PATCH net-next 00/12] net: enable TC956x support Jakub Kicinski
2026-05-03  2:07   ` Alex Elder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agXo_evi1oFLBJoo@aspen.lan \
    --to=daniel@riscstar.com \
    --cc=a0987203069@gmail.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andersson@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=andrew@lunn.ch \
    --cc=arnd@arndb.de \
    --cc=ast@kernel.org \
    --cc=boon.khai.ng@altera.com \
    --cc=bpf@vger.kernel.org \
    --cc=brgl@kernel.org \
    --cc=chenchuangyu@xiaomi.com \
    --cc=chenhuacai@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=elder@riscstar.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hawk@kernel.org \
    --cc=hkallweit1@gmail.com \
    --cc=inochiama@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=julianbraha@gmail.com \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linusw@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=livelycarpet87@gmail.com \
    --cc=matthew.gerlach@altera.com \
    --cc=maxime.chevallier@bootlin.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=me@ziyao.cc \
    --cc=mohd.anwar@oss.qualcomm.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=richardcochran@gmail.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=robh@kernel.org \
    --cc=rohan.g.thomas@altera.com \
    --cc=sdf@fomichev.me \
    --cc=siyanteng@cqsoftware.com.cn \
    --cc=weishangjuan@eswincomputing.com \
    --cc=wens@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox