* Re: [PATCH v7 09/13] clk: zte: Introduce a driver for zx297520v3 top clocks
From: sashiko-bot @ 2026-07-16 22:42 UTC (permalink / raw)
To: Stefan Dösinger
Cc: devicetree, olteanv, robh, vkoul, neil.armstrong, linux-phy,
conor+dt
In-Reply-To: <20260717-zx29clk-v7-9-408411cfcf36@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Using `clk_main[0]` to initialize the `.parent` field in the `zx297520v3_top_gates` static array is a violation of ISO C rules for constant expressions, resulting in a hard compile error.
--
--- Patch [9]: [PATCH v7 09/13] clk: zte: Introduce a driver for zx297520v3 top clocks ---
commit 8a7fee3879cf08b60157cde425eed4bbaa05fcb1
Author: Stefan Dösinger <stefandoesinger@gmail.com>
clk: zte: Introduce a driver for zx297520v3 top clocks
This commit introduces a clock driver for the ZTE zx297520v3 SoC top clocks.
It supports core devices such as PLLs, the AHB bus, timers, the USB
controller, and the Cortex M0 processor.
> diff --git a/drivers/clk/zte/clk-zx297520v3.c b/drivers/clk/zte/clk-zx297520v3.c
> new file mode 100644
> index 0000000000000..20ff00ba5f670
> --- /dev/null
> +++ b/drivers/clk/zte/clk-zx297520v3.c
[ ... ]
> +static const struct zx_gate_desc zx297520v3_top_gates[] = {
[ ... ]
> + /* AHB clock gates: Disabling these cuts off the A53 from register access into 0x130000/
> + * 0x140000, including the topcrm region, so it can't be re-enabled. Mark them critical for
> + * that reason.
> + *
> + * Gating this off does not stop the M0 rproc, nor does it stop M0 from accessing topcrm. It
> + * does however gate off M0 from the AXI interconnect, so it can't read RAM, Matrix, etc. So
> + * these bits here merely gate the bus, not the actual devices.
> + */
> + GATE(ZX297520V3_AHB_PCLK, "ahb-pclk", "ahb-mux", 0x54, 12,
> + CLK_IS_CRITICAL),
> + GATE(ZX297520V3_AHB_WCLK, "ahb-wclk", "m0-mux", 0x54, 13,
> + CLK_IS_CRITICAL),
> +
> + /* SRAM1 and 2 clocks. Leave them on for now, as turning them off carelessly hangs the M0 */
> + GATE(ZX297520V3_SRAM1_PCLK, "sram1-pclk", clk_main[0], 0x54, 18,
> + CLK_IS_CRITICAL),
[Severity: Low]
Does using clk_main[0] to initialize elements of a static array cause a build
failure?
In C, array accesses are not treated as constant expressions. Initializing the
zx297520v3_top_gates static array with clk_main[0] will cause the compiler
to fail with an "initializer element is not constant" error.
Could a string literal like "clock-26m" be used directly instead?
> + GATE(ZX297520V3_SRAM2_PCLK, "sram2-pclk", clk_main[0], 0x54, 16,
> + CLK_IS_CRITICAL),
> +
> + /* Pinmux (AON, TOP, IOCFG but not PDCFG). Critical as well until we have a driver that
> + * consumes it. I don't think we'll realistically shut this off ever.
> + *
> + * Setting either bit 0 or 1 in register 0x58 makes the device work.
> + */
> + GATE(ZX297520V3_PMM_WCLK, "pmm-wclk", clk_main[0], 0x58, 0,
> + CLK_IS_CRITICAL),
> + GATE(ZX297520V3_PMM_PCLK, "pmm-pclk", clk_main[0], 0x58, 1,
> + CLK_IS_CRITICAL),
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-zx29clk-v7-0-408411cfcf36@gmail.com?part=9
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v7 01/13] dt-bindings: phy: Add zx297520v3 USB phy documentation
From: Rob Herring (Arm) @ 2026-07-17 0:05 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Philipp Zabel, Russell King, linux-phy, Stephen Boyd,
linux-kernel, linux-arm-kernel, devicetree, Michael Turquette,
Brian Masney, Conor Dooley, Neil Armstrong, Vinod Koul,
Krzysztof Kozlowski, linux-clk
In-Reply-To: <20260717-zx29clk-v7-1-408411cfcf36@gmail.com>
On Fri, 17 Jul 2026 00:35:37 +0300, Stefan Dösinger wrote:
> This binding will be used as a subnode of topcrm, which will be added in
> the next patch.
>
> Why is this a child node and not #phy-cells added to topcrm itself,
> like clocks and resets as writing-bindings.rst suggests? Because
> of_phy_provider_register checks if the of_node the PHY provider is added
> to belongs to the device or is a child of the device, so I can't put the
> PHY driver into an MFD (or AUX) child without its own node.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
>
> How do I handle examples? It would be identical to the example in
> topcrm. The example needs the reset defines from the next patch in
> either case.
> ---
> .../bindings/phy/zte,zx297520v3-usb-phy.yaml | 96 ++++++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/phy/phy-zte-zx297520v3-usb.h | 12 +++
> 3 files changed, 110 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: /example-0/clock-controller: failed to match any schema with compatible: ['zte,zx297520v3-topcrm', 'syscon']
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'reg' is a required property
from schema $id: http://devicetree.org/schemas/mfd/syscon-common.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260717-zx29clk-v7-1-408411cfcf36@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v7 02/13] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller
From: Rob Herring (Arm) @ 2026-07-17 0:05 UTC (permalink / raw)
To: Stefan Dösinger
Cc: linux-phy, Conor Dooley, Neil Armstrong, Michael Turquette,
linux-kernel, Brian Masney, Vinod Koul, Russell King, linux-clk,
devicetree, Krzysztof Kozlowski, Philipp Zabel, linux-arm-kernel,
Stephen Boyd
In-Reply-To: <20260717-zx29clk-v7-2-408411cfcf36@gmail.com>
On Fri, 17 Jul 2026 00:35:38 +0300, Stefan Dösinger wrote:
> These SoCs have 3 clock and reset controllers: Top, Matrix and LSP.
> Clocks go from oscillator -> top -> matrix -> LSP, with a register in
> top controlling most clocks that get passed to matrix and a register in
> matrix controlling all the clocks that get passed to LSP.
>
> Generally every device has two clocks (one work clock, and one that
> connects it to the bus, I call it PCLK), two reset bits (I don't know
> what the difference is - sometimes asserting one is enough to reset the
> device, sometimes both need to be asserted). PCLK and WCLK are
> controlled by individual gates. Some devices have a mux and/or a
> divider for their work clock. Some devices, like the GPIO controller,
> only have reset bits and no clocks.
>
> The top clock controller is fed by a 26mhz external oscillator and has 4
> PLLs to generate other clock rates. ZTE's kernel mostly relies on the
> boot ROM to set up PLLs, but one LTE-Related PLL is not configured
> on some boards. Therefore my driver contains code to program PLLs. It
> produces identical settings as the boot ROM for the pre-programmed
> frequencies.
>
> Not all clocks will have an explicit user in the end. I am defining a
> lot of them simply to shut them off. The boot loader sets up a few of
> the proprietary timers, which will send regular IRQs (although the
> kernel of course doesn't need to listen to them). I don't plan to add a
> driver for the proprietary timer as I see no use for them - the ARM arch
> timer works just fine. I will add a driver for the very similar
> proprietary watchdog though.
>
> The clock list in this patch is pretty complete but not exhaustive.
> There are other bits that are enabled, but I couldn't deduce what they
> are controlling by trial and error. Some of them seem to do nothing.
> Others cause an instant hang of the board when disabled. It is quite
> likely that a handful more clocks will be added in the future, but not a
> large number.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
>
> Changes v6->v7:
> *) Moved the bingings back into clock/ (Krzysztof). use clock-controller@
> in example and in the later DTSI patch.
>
> *) With as lucky find in strings in LTE's cpko.ko blob I could make sense
> of the register that controls which clocks get passed from PLLs into
> the clock distribution inside topcrm. Importantly that also narrows
> which clocks can possibly be passed to Matrix, so I could slim down the
> bindings considerably.
>
> *) Add a usb phy child node with the phy schema added in the previous
> patch. I don't expect the USB status register to ever be in another
> place, so the binding expresses the @84 expectation.
>
> Changes v5->v6:
> Set value for syscon-reboot example (Sashiko). It was my intention to
> set only the lowest bit, and I think Sashiko is right that without
> 'value' being set, all other bits are actively set to 0. It shouldn't
> matter given my understanding of the hardware (afaics all other bits are
> ignored), but actively clearing bits was not my intention.
>
> I haven't changed the name match for "syscon-reboot". I see plenty of
> examples of hardcoding this string as opposed to having a regex for
> syscon-reboot@12345678 in other bindings.
>
> Changes v4->v5:
>
> Rename from zte,zx297520v3-topclk to zte,zx297520v3-topcrm and move to
> soc/zte
> Fix path in MAINTAINERS
> Add syscon-reboot node to the binding
> Give the USB and HSIC PHY resets their own reset control
> ---
> .../bindings/clock/zte,zx297520v3-topcrm.yaml | 123 +++++++++++++++++++++
> MAINTAINERS | 3 +
> include/dt-bindings/clock/zte,zx297520v3-clk.h | 66 +++++++++++
> include/dt-bindings/reset/zte,zx297520v3-reset.h | 32 ++++++
> 4 files changed, 224 insertions(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'clocks' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'clock-names' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): 'syscon-reboot' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.example.dtb: clock-controller (zte,zx297520v3-topcrm): '#clock-cells' is a required property
from schema $id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260717-zx29clk-v7-2-408411cfcf36@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* RE: [RFC PATCH 06/10] net: pcs: xpcs: improve SGMII AN state handling for Rockchip RK3568
From: Jiawen Wu @ 2026-07-17 2:22 UTC (permalink / raw)
To: 'Coia Prant', 'Andrew Lunn'
Cc: kuba, davem, edumazet, pabeni, andrew+netdev, robh, krzk+dt,
heiko, netdev, linux-rockchip, devicetree, linux-arm-kernel,
linux-phy, 'Mengyuan Lou'
In-Reply-To: <CALj3r0jdkDEjpO+7NOTGs+wgukKvD=fzFn4ewpurxxPswCE5GA@mail.gmail.com>
On Wed, Jul 15, 2026 7:05 AM, Coia Prant wrote:
> Andrew Lunn <andrew@lunn.ch> 于2026年7月15日周三 06:44写道：
> >
> > On Wed, Jul 15, 2026 at 03:08:34AM +0800, Coia Prant wrote:
> > > Commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> >
> > You do not appear to Cc: the Wangxun NIC people. It would be good to
> > have there comments on this change.
>
> I apologize; the output from get_maintainer.pl is very long. I’ve
> heard that having too many recipients can cause the PATCH to be
> rejected by the LKML mail server.
>
> I have added Wangxun maintainer (Jiawen Wu <jiawenwu@trustnetic.com>
> and Mengyuan Lou <mengyuanlou@net-swift.com>) to the CC list.
>
> > > Fixes: 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> >
> > Please don't mix fixed and new code. Is this a real fix? Should it be
> > back ported to stable?
>
> I am not sure if this is a specific characteristic of Wangxun NICs, as
> I do not have any available for testing.
>
> The behavior of the Rockchip DW XPCS IP core matches what is described
> in the commit message (even though phylink brings the link to down
> based on the phydev link status).
>
> This appears to be a bug (at least on Rockchip platforms) or Wangxun
> NICs features.
>
> However, I cannot confirm whether Wangxun NICs behave the same way.
> Therefore, I have kept their code as is for now.
>
> Could a Wangxun NICs maintainer provide some feedback based on testing?
>
> If we can confirm that this is indeed a bug, I can submit a separate fix.
>
> I would greatly appreciate it.
>
> Thanks.
Hi Coia,
I have tested this patch and it works on Wangxun NICs, thanks.
The commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
is too long ago for me to recall exactly what that thought was.
The log shows:
"On this device, CL37_ANSGM_STS (bit[4:1] of VR_MII_AN_INTR_STS) indicates
the status received from remote link during the auto-negotiation, and
self-clear after the auto-negotiation is complete.
Meanwhile, CL37_ANCMPLT_INTR will be set to 1, to indicate CL37 AN is
complete. So add another way to get the state for CL37 SGMII."
I tried to reproduce this issue, but it didn't seem to exist.
In the current logic:
link up -> read status from CL37_ANSGM_STS -> CL37_ANCMPLT_INTR not clear
link down -> read status from BMCR -> CL37_ANCMPLT_INTR clear
It also works, although CL37_ANCMPLT_INTR is not cleared every time, and AN
restart is absent. But BMCR looks like it only wants to be return as 0, it is
weird.
So I think Wangxun NICs also can be applied to general code as well, for
getting state in C37 SGMII mode.
However, I am wondering whether CL37_ANCMPLT_INTR clear and ANRESTART are
necessary.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH RFC v4 9/9] arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY
From: Qiang Yu @ 2026-07-17 3:02 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Bjorn Andersson, Konrad Dybcio,
linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <f83556e7-dbb1-4926-a2a0-4690cdbad941@oss.qualcomm.com>
On Fri, Jul 10, 2026 at 03:25:37PM +0200, Konrad Dybcio wrote:
> On 7/10/26 3:11 PM, Qiang Yu wrote:
> > On Fri, Jul 10, 2026 at 10:44:20AM +0200, Konrad Dybcio wrote:
> >> On 7/10/26 4:19 AM, Qiang Yu wrote:
> >>> On Mon, Jun 29, 2026 at 11:20:07AM +0200, Konrad Dybcio wrote:
> >>>> On 6/29/26 7:05 AM, Qiang Yu wrote:
> >>>>> On Wed, Jun 17, 2026 at 01:19:49PM +0200, Konrad Dybcio wrote:
> >>>>>> On 5/19/26 7:47 AM, Qiang Yu wrote:
> >>>>>>> Glymur PCIe3 uses a single shared Gen5x8 QMP PHY block. Model PCIe3a and
> >>>>>>> PCIe3b as consumers of that shared PHY provider instead of separate PHY
> >>>>>>> nodes.
>
> [...]
>
> >>>>>>> + linux,pci-domain = <3>;
> >>>>>>> + num-lanes = <8>;
> >>>>>>
> >>>>>> Is it fine to keep num-lanes 8 here even for configurations with
> >>>>>> bifurcated PHY?
> >>>>>>
> >>>>>> I would assume so, given essentially this is a x8 host, whose 4
> >>>>>> lanes may simply be effectively NC
> >>>>>>
> >>>>> Actually, on existing platforms, the PCIe3a and PCIe3b controllers are
> >>>>> never enabled at the same time. When PCIe3a is exposed, it is always in an
> >>>>> x8 slot. But if we have a x4+x4 platform in future, we can simply override
> >>>>> num-lanes to 4 in the board.dts.
> >>>>
> >>>> My question is whether that will be necessary - if yes, sure, we
> >>>> can do it, but if not, we can conclude on this early and not have
> >>>> to fight over it in a couple months
> >>>>
> >>> I think we do need to override it in that case. If both PCIe3a and PCIe3b
> >>> are enabled in x4+x4 mode but PCIe3a keeps num-lanes = <8>, userspace
> >>> will see an 8-lane slot. If an x8-capable EP is connected to that slot,
> >>> both ends will advertise x8 support, but the link is up at x4. That looks
> >>> like a genuine bug from the user's point of view.
> >>
> >> Do we know what's advertised on x86 PCs with bifurcated lanes?
> >>
> > On QCB, it advertise x8. On CRD, PCIe3a is hidden.
>
> No no, I meant to ask whether we know what the userspace gets to see on
> e.g. an AMD system with a bifurcated slot. I would expect that LnkCap
> may report a different value, but not necessarily. I would assume we
> should probably mimic whatever that does, unless the spec takes a
> stance on what should be done.
>
I don't have any AMD/Intel hardware with a bifurcated slot at hand,
and I couldn't find an lspci example showing what LnkCap reports in
that case either, so I can't confirm what x86 does here.
But can you elaborate on what is the benefits to set it to 8 in x4+x4
mode?
- Qiang Yu
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [RFC PATCH 06/10] net: pcs: xpcs: improve SGMII AN state handling for Rockchip RK3568
From: Coia Prant @ 2026-07-17 6:28 UTC (permalink / raw)
To: Jiawen Wu
Cc: Andrew Lunn, kuba, davem, edumazet, pabeni, andrew+netdev, robh,
krzk+dt, heiko, netdev, linux-rockchip, devicetree,
linux-arm-kernel, linux-phy, Mengyuan Lou
In-Reply-To: <000c01dd1593$2ac0b0f0$804212d0$@trustnetic.com>
Jiawen Wu <jiawenwu@trustnetic.com> 于2026年7月17日周五 10:23写道：
>
> On Wed, Jul 15, 2026 7:05 AM, Coia Prant wrote:
> > Andrew Lunn <andrew@lunn.ch> 于2026年7月15日周三 06:44写道：
> > >
> > > On Wed, Jul 15, 2026 at 03:08:34AM +0800, Coia Prant wrote:
> > > > Commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > >
> > > You do not appear to Cc: the Wangxun NIC people. It would be good to
> > > have there comments on this change.
> >
> > I apologize; the output from get_maintainer.pl is very long. I’ve
> > heard that having too many recipients can cause the PATCH to be
> > rejected by the LKML mail server.
> >
> > I have added Wangxun maintainer (Jiawen Wu <jiawenwu@trustnetic.com>
> > and Mengyuan Lou <mengyuanlou@net-swift.com>) to the CC list.
> >
> > > > Fixes: 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > > > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > >
> > > Please don't mix fixed and new code. Is this a real fix? Should it be
> > > back ported to stable?
> >
> > I am not sure if this is a specific characteristic of Wangxun NICs, as
> > I do not have any available for testing.
> >
> > The behavior of the Rockchip DW XPCS IP core matches what is described
> > in the commit message (even though phylink brings the link to down
> > based on the phydev link status).
> >
> > This appears to be a bug (at least on Rockchip platforms) or Wangxun
> > NICs features.
> >
> > However, I cannot confirm whether Wangxun NICs behave the same way.
> > Therefore, I have kept their code as is for now.
> >
> > Could a Wangxun NICs maintainer provide some feedback based on testing?
> >
> > If we can confirm that this is indeed a bug, I can submit a separate fix.
> >
> > I would greatly appreciate it.
> >
> > Thanks.
>
> Hi Coia,
>
> I have tested this patch and it works on Wangxun NICs, thanks.
>
> The commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> is too long ago for me to recall exactly what that thought was.
>
> The log shows:
>
> "On this device, CL37_ANSGM_STS (bit[4:1] of VR_MII_AN_INTR_STS) indicates
> the status received from remote link during the auto-negotiation, and
> self-clear after the auto-negotiation is complete.
> Meanwhile, CL37_ANCMPLT_INTR will be set to 1, to indicate CL37 AN is
> complete. So add another way to get the state for CL37 SGMII."
>
> I tried to reproduce this issue, but it didn't seem to exist.
> In the current logic:
>
> link up -> read status from CL37_ANSGM_STS -> CL37_ANCMPLT_INTR not clear
> link down -> read status from BMCR -> CL37_ANCMPLT_INTR clear
>
> It also works, although CL37_ANCMPLT_INTR is not cleared every time, and AN
> restart is absent. But BMCR looks like it only wants to be return as 0, it is
> weird.
>
> So I think Wangxun NICs also can be applied to general code as well, for
> getting state in C37 SGMII mode.
Hi Jiawen,
Thanks so much for testing and confirming that the patch works on Wangxun
NICs. That's a huge relief.
You're right to ask about the CL37_ANCMPLT_INTR clear and ANRESTART. Let
me explain what I observed on RK3568.
The key difference is that on RK3568's XPCS, we're working in the MAC
side (TX_CONFIG = 0), whereas on Wangxun it seems you're in the PHY side
(TX_CONFIG = 1). This is based on the commit 2a22b7ae2fa3 where you set
TX_CONFIG = 1.
> However, I am wondering whether CL37_ANCMPLT_INTR clear and ANRESTART are
> necessary.
On the RK3568 (MAC side), according to the DesignWare XPCS TRM, the
hardware behavior is as follows:
- After a software reset (BMCR_RESET), ANENABLE is automatically set to 1
(the hardware reset value). So auto-negotiation starts automatically.
- When the link is up, CL37_ANSGM_STS contains the negotiated speed/duplex
and the link status.
- When the link goes down, CL37_ANSGM_STS becomes all zero, but
CL37_ANCMPLT_INTR remains set to 1.
- Crucially, the PCS does **not** automatically restart AN when the link
comes back up. Without an explicit ANRESTART, the link stays down
forever.
So on RK3568, the ANRESTART is mandatory to recover from a link-down
event. Without it, the link never comes back up after a cable disconnect.
In my current logic:
1. If CL37_ANSGM_STS has valid speed/duplex bits, report the link as up
and return.
2. If CL37_ANSGM_STS is zero but CL37_ANCMPLT_INTR is set, the link is
down. We clear the interrupt and issue an ANRESTART to start a new
negotiation.
This works reliably on RK3568 in SGMII MAC side mode.
Since you confirmed that the general code path also works on Wangxun, I
wonder: does the ANRESTART also work on your side? If yes, perhaps we can
make this the common path for all hardware, not just RK3568.
If you can test the ANRESTART logic on Wangxun and it works, I'd be happy
to submit a standalone fix that makes this the common code path.
Otherwise, the current approach (skipping the ANRESTART trigger on
Wangxun NICs in c37_sgmii_get_state) remains safe.
If you're comfortable with the change, I'd appreciate it if you could add
your Tested-by tag to the commit. I'll also make sure to include you on
the CC list for any future revisions or related fixes.
Thanks again for testing!
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v7 01/13] dt-bindings: phy: Add zx297520v3 USB phy documentation
From: Krzysztof Kozlowski @ 2026-07-17 6:32 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, Vinod Koul,
Neil Armstrong, Russell King, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, linux-phy
In-Reply-To: <20260717-zx29clk-v7-1-408411cfcf36@gmail.com>
On Fri, Jul 17, 2026 at 12:35:37AM +0300, Stefan Dösinger wrote:
> This binding will be used as a subnode of topcrm, which will be added in
> the next patch.
Irrelevant, drop. Also not true, because next patch is driver patch.
>
> Why is this a child node and not #phy-cells added to topcrm itself,
> like clocks and resets as writing-bindings.rst suggests? Because
> of_phy_provider_register checks if the of_node the PHY provider is added
> to belongs to the device or is a child of the device, so I can't put the
> PHY driver into an MFD (or AUX) child without its own node.
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
>
> How do I handle examples? It would be identical to the example in
> topcrm. The example needs the reset defines from the next patch in
> either case.
1. No, it does not need. Just use whatever numbers.
2. Other code would give you the hint: one example in parent's schema.
> ---
> .../bindings/phy/zte,zx297520v3-usb-phy.yaml | 96 ++++++++++++++++++++++
> MAINTAINERS | 2 +
> include/dt-bindings/phy/phy-zte-zx297520v3-usb.h | 12 +++
> 3 files changed, 110 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.yaml b/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.yaml
> new file mode 100644
> index 000000000000..8bad9365d5cc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/zte,zx297520v3-usb-phy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ZTE zx297520v3 USB and HSIC PHY
> +
> +maintainers:
> + - Stefan Dösinger <stefandoesinger@gmail.com>
> +
> +description: |
> + This PHY is found on zx297520v3 boards. It has no configurability on its own,
> + but it does require a correct reset and wait sequence to initialize. It can
> + provide interrupt notification when USB is connected and disconnected.
> +
> + The phy is a component of the board's topcrm controller. The hardware needs to
> + be declared as a child node of the zte,zx297520v3-topcrm node.
> +
> + The register space and IRQs always account for two PHYs: One USB 2.0 OTG phy
> + and a HSIC PHY. Not all boards have both. If one is missing, the corresponding
> + ready flag will never be set.
> +
> + The "include/dt-bindings/phy/phy-zte-zx297520v3-usb.h" header contains the
> + definition for the PHY indices.
> +
> +properties:
> + compatible:
> + const: zte,zx297520v3-usb-phy
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + items:
> + - description: IRQ reporting USB connection
> + - description: IRQ reporting USB disconnection
> + - description: IRQ reporting HSIC connection
> + - description: IRQ reporting HSIC disconnection
> +
> + interrupt-names:
> + items:
> + - const: usb-up
> + - const: usb-down
> + - const: hsic-up
> + - const: hsic-down
> +
> + resets:
> + items:
> + - description: USB phy reset
> + - description: HSIC phy reset
Why do you reference here resets from the parent? That's a clear signal
you have one device, so one device node regardless of Linux phy core
behavior.
> +
> + reset-names:
> + items:
> + - const: usb
> + - const: hsic
> +
> + "#phy-cells":
> + const: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-names
> + - resets
> + - reset-names
> + - "#phy-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/phy/phy-zte-zx297520v3-usb.h>
> +
> + topcrm: clock-controller {
> + compatible = "zte,zx297520v3-topcrm", "syscon";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #reset-cells = <1>;
Drop node, you have warnings.
> +
> + usb-phy@84 {
> + compatible = "zte,zx297520v3-usb-phy";
> + reg = <0x84>;
> + interrupts = <GIC_SPI 42 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 43 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 45 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 46 IRQ_TYPE_EDGE_RISING>;
Align it properly/
> + interrupt-names = "usb-up", "usb-down", "hsic-up", "hsic-down";
> + resets = <&topcrm 18>,
> + <&topcrm 20>;
Same here.
> + reset-names = "usb", "hsic";
> + #phy-cells = <1>;
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8729cea57c3d..cb50c2e3b4ea 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3878,8 +3878,10 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> S: Odd fixes
> F: Documentation/arch/arm/zte/
> F: Documentation/devicetree/bindings/arm/zte.yaml
> +F: Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.yaml
> F: arch/arm/boot/dts/zte/
> F: arch/arm/mach-zte/
> +F: include/dt-bindings/phy/phy-zte-zx297520v3-usb.h
>
> ARM/ZYNQ ARCHITECTURE
> M: Michal Simek <michal.simek@amd.com>
> diff --git a/include/dt-bindings/phy/phy-zte-zx297520v3-usb.h b/include/dt-bindings/phy/phy-zte-zx297520v3-usb.h
> new file mode 100644
> index 000000000000..8a0a3ccbdd63
> --- /dev/null
> +++ b/include/dt-bindings/phy/phy-zte-zx297520v3-usb.h
Filename MUST match compatible.
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (C) Stefan Dösinger.
> + */
> +
> +#ifndef __DT_BINDINGS_PHY_USB_ZX297520V3_H
> +#define __DT_BINDINGS_PHY_USB_ZX297520V3_H
> +
> +#define ZX297520V3_USB_PHY 0
> +#define ZX297520V3_HSIC_PHY 1
> +
> +#endif /* __DT_BINDINGS_PHY_USB_ZX297520V3_H */
>
> --
> 2.54.0
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v7 02/13] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller
From: Krzysztof Kozlowski @ 2026-07-17 6:36 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, Vinod Koul,
Neil Armstrong, Russell King, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, linux-phy
In-Reply-To: <20260717-zx29clk-v7-2-408411cfcf36@gmail.com>
On Fri, Jul 17, 2026 at 12:35:38AM +0300, Stefan Dösinger wrote:
> diff --git a/Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml b/Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml
> new file mode 100644
> index 000000000000..454997063573
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml
> @@ -0,0 +1,123 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/zte,zx297520v3-topcrm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ZTE zx297520v3 SoC top clock and reset controller
> +
> +maintainers:
> + - Stefan Dösinger <stefandoesinger@gmail.com>
> +
> +description: |
> + The zx297520v3 top clock and reset controller generates clocks for core
> + devices on the board like the main bus, USB and timers. In addition to clocks
> + it has reset controls for peripherals, a global board reset, watchdog reset
> + controls and a USB status register.
> +
> + The controller has two clock inputs: a 26 MHz and a 32 KHz external
> + oscillator. They need to be provided as input clocks. The controller provides
> + PLL output frequencies to downstream clock controllers.
> +
> + The USB PHY functionality is exposed as a child node documented in
> + zte,zx297520v3-usb-phy.yaml.
> +
> + All available clocks are defined as preprocessor macros in the
> + "include/dt-bindings/clock/zte,zx297520v3-clk.h" header. The resets are
> + defined in the "include/dt-bindings/reset/zte,zx297520v3-reset.h" header.
> +
> +properties:
> + compatible:
> + items:
> + - const: zte,zx297520v3-topcrm
> + - const: syscon
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: 26 MHz external oscillator
> + - description: 32 KHz external oscillator
> +
> + clock-names:
> + items:
> + - const: osc26m
> + - const: osc32k
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + "#clock-cells":
> + const: 1
> +
> + "#reset-cells":
> + const: 1
> +
> + syscon-reboot:
> + type: object
> + $ref: /schemas/power/reset/syscon-reboot.yaml#
> + description:
> + Reboot method for the SoC.
> +
> + usb-phy@84:
No, do not children with and without addressing. syscon-reboot should
have address as well. Or this one should not, since it looks pretty
redundant - it is deducible from the top level compatible, no? So just
usb-phy.
> + compatible = "zte,zx297520v3-usb-phy";
> + type: object
> + $ref: /schemas/phy/zte,zx297520v3-usb-phy.yaml#
> + description:
> + USB and HSIC PHY controller.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - syscon-reboot
> + - usb-phy@84
> + - "#address-cells"
> + - "#size-cells"
> + - "#clock-cells"
> + - "#reset-cells"
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/reset/zte,zx297520v3-reset.h>
> + #include <dt-bindings/phy/phy-zte-zx297520v3-usb.h>
> + #include <dt-bindings/clock/zte,zx297520v3-clk.h>
> +
> + topcrm: clock-controller@13b000 {
> + compatible = "zte,zx297520v3-topcrm", "syscon";
> + reg = <0x0013b000 0x400>;
> + clocks = <&osc26m>, <&osc32k>;
> + clock-names = "osc26m", "osc32k";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> +
> + syscon-reboot {
> + compatible = "syscon-reboot";
> + offset = <0x0>;
> + mask = <0x1>;
> + value = <0x1>;
> + };
> +
> + usb-phy@84 {
> + reg = <0x84>;
> + interrupts = <GIC_SPI 42 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 43 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 45 IRQ_TYPE_EDGE_RISING>,
> + <GIC_SPI 46 IRQ_TYPE_EDGE_RISING>;
Here also misaligned.
> + interrupt-names = "usb-up", "usb-down", "hsic-up", "hsic-down";
> + resets = <&topcrm ZX297520V3_USB_PHY_RESET>,
> + <&topcrm ZX297520V3_HSIC_PHY_RESET>;
> + reset-names = "usb", "hsic";
> + #phy-cells = <1>;
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index cb50c2e3b4ea..5f962b9947ab 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3878,10 +3878,13 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> S: Odd fixes
> F: Documentation/arch/arm/zte/
> F: Documentation/devicetree/bindings/arm/zte.yaml
> +F: Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml
> F: Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.yaml
> F: arch/arm/boot/dts/zte/
> F: arch/arm/mach-zte/
> +F: include/dt-bindings/clock/zte,zx297520v3-clk.h
> F: include/dt-bindings/phy/phy-zte-zx297520v3-usb.h
> +F: include/dt-bindings/reset/zte,zx297520v3-reset.h
>
> ARM/ZYNQ ARCHITECTURE
> M: Michal Simek <michal.simek@amd.com>
> diff --git a/include/dt-bindings/clock/zte,zx297520v3-clk.h b/include/dt-bindings/clock/zte,zx297520v3-clk.h
Same mistake. Filename MUST match the compatible.
> new file mode 100644
> index 000000000000..1f010ff4e6ca
> --- /dev/null
> +++ b/include/dt-bindings/clock/zte,zx297520v3-clk.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (C) Stefan Dösinger.
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_ZX297520V3_H
> +#define __DT_BINDINGS_CLOCK_ZX297520V3_H
> +
> +#define ZX297520V3_M0_WCLK 1
Start from 0 since you started resets also from 0.
> +#define ZX297520V3_SRAM1_PCLK 2
> +#define ZX297520V3_SRAM2_PCLK 3
> +#define ZX297520V3_UART0_WCLK 4
> +#define ZX297520V3_UART0_PCLK 5
> +#define ZX297520V3_I2C0_WCLK 6
> +#define ZX297520V3_I2C0_PCLK 7
> +#define ZX297520V3_RTC_WCLK 8
> +#define ZX297520V3_RTC_PCLK 9
> +#define ZX297520V3_LPM_GSM_WCLK 10
> +#define ZX297520V3_LPM_GSM_PCLK 11
> +#define ZX297520V3_LPM_LTE_WCLK 12
> +#define ZX297520V3_LPM_LTE_PCLK 13
> +#define ZX297520V3_LPM_TD_WCLK 14
> +#define ZX297520V3_LPM_TD_PCLK 15
> +#define ZX297520V3_LPM_W_WCLK 16
> +#define ZX297520V3_LPM_W_PCLK 17
> +#define ZX297520V3_TIMER_T08_WCLK 18
> +#define ZX297520V3_TIMER_T08_PCLK 19
> +#define ZX297520V3_TIMER_T09_WCLK 20
> +#define ZX297520V3_TIMER_T09_PCLK 21
> +#define ZX297520V3_MPLL 22
> +#define ZX297520V3_MPLL_D5 23
> +#define ZX297520V3_DPLL 24
> +#define ZX297520V3_GPLL 25
> +#define ZX297520V3_GPLL_D2 26
> +#define ZX297520V3_GATED_OSC26M 27
> +#define ZX297520V3_PMM_WCLK 28
> +#define ZX297520V3_PMM_PCLK 29
> +#define ZX297520V3_OUT0_WCLK 30
> +#define ZX297520V3_OUT1_WCLK 31
> +#define ZX297520V3_OUT2_WCLK 32
> +#define ZX297520V3_OUT32K_WCLK 33
> +#define ZX297520V3_RMIIPHY_WCLK 34
> +#define ZX297520V3_TIMER_T12_WCLK 35
> +#define ZX297520V3_TIMER_T12_PCLK 36
> +#define ZX297520V3_TIMER_T13_WCLK 37
> +#define ZX297520V3_TIMER_T13_PCLK 38
> +#define ZX297520V3_TIMER_T14_WCLK 39
> +#define ZX297520V3_TIMER_T14_PCLK 40
> +#define ZX297520V3_TIMER_T15_WCLK 41
> +#define ZX297520V3_TIMER_T15_PCLK 42
> +#define ZX297520V3_TIMER_T16_WCLK 43
> +#define ZX297520V3_TIMER_T16_PCLK 44
> +#define ZX297520V3_TIMER_T17_WCLK 45
> +#define ZX297520V3_TIMER_T17_PCLK 46
> +#define ZX297520V3_WDT_T18_WCLK 47
> +#define ZX297520V3_WDT_T18_PCLK 48
> +#define ZX297520V3_USIM1_WCLK 49
> +#define ZX297520V3_USIM1_PCLK 50
> +#define ZX297520V3_AHB_WCLK 51
> +#define ZX297520V3_AHB_PCLK 52
> +#define ZX297520V3_USB_WCLK 53
> +#define ZX297520V3_USB_PCLK 54
> +#define ZX297520V3_HSIC_WCLK 55
> +#define ZX297520V3_HSIC_PCLK 56
> +
> +#endif /* __DT_BINDINGS_CLOCK_ZX297520V3_H */
> diff --git a/include/dt-bindings/reset/zte,zx297520v3-reset.h b/include/dt-bindings/reset/zte,zx297520v3-reset.h
> new file mode 100644
> index 000000000000..43db72bb59de
> --- /dev/null
> +++ b/include/dt-bindings/reset/zte,zx297520v3-reset.h
Same here.
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v7 00/13] ZTE zx297520v3 clock bindings and driver
From: Krzysztof Kozlowski @ 2026-07-17 6:37 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, Vinod Koul,
Neil Armstrong, Russell King, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, linux-phy
In-Reply-To: <20260717-zx29clk-v7-0-408411cfcf36@gmail.com>
On Fri, Jul 17, 2026 at 12:35:36AM +0300, Stefan Dösinger wrote:
> Hi,
>
Where did you explain merging dependencies? Patch 2 (or 3) depends on
patch 1, so do not expect maintainers to guess that.
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* RE: [RFC PATCH 06/10] net: pcs: xpcs: improve SGMII AN state handling for Rockchip RK3568
From: Jiawen Wu @ 2026-07-17 7:03 UTC (permalink / raw)
To: 'Coia Prant'
Cc: 'Andrew Lunn', kuba, davem, edumazet, pabeni,
andrew+netdev, robh, krzk+dt, heiko, netdev, linux-rockchip,
devicetree, linux-arm-kernel, linux-phy, 'Mengyuan Lou'
In-Reply-To: <CALj3r0hH=mF+8oVbisoECYo6HRuSdfSt_nuhFunHowyPsw2srQ@mail.gmail.com>
On Fri, Jul 17, 2026 2:29 PM, Coia Prant wrote:
> Jiawen Wu <jiawenwu@trustnetic.com> 于2026年7月17日周五 10:23写道：
> >
> > On Wed, Jul 15, 2026 7:05 AM, Coia Prant wrote:
> > > Andrew Lunn <andrew@lunn.ch> 于2026年7月15日周三 06:44写道：
> > > >
> > > > On Wed, Jul 15, 2026 at 03:08:34AM +0800, Coia Prant wrote:
> > > > > Commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > > >
> > > > You do not appear to Cc: the Wangxun NIC people. It would be good to
> > > > have there comments on this change.
> > >
> > > I apologize; the output from get_maintainer.pl is very long. I’ve
> > > heard that having too many recipients can cause the PATCH to be
> > > rejected by the LKML mail server.
> > >
> > > I have added Wangxun maintainer (Jiawen Wu <jiawenwu@trustnetic.com>
> > > and Mengyuan Lou <mengyuanlou@net-swift.com>) to the CC list.
> > >
> > > > > Fixes: 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > > > > Signed-off-by: Coia Prant <coiaprant@gmail.com>
> > > >
> > > > Please don't mix fixed and new code. Is this a real fix? Should it be
> > > > back ported to stable?
> > >
> > > I am not sure if this is a specific characteristic of Wangxun NICs, as
> > > I do not have any available for testing.
> > >
> > > The behavior of the Rockchip DW XPCS IP core matches what is described
> > > in the commit message (even though phylink brings the link to down
> > > based on the phydev link status).
> > >
> > > This appears to be a bug (at least on Rockchip platforms) or Wangxun
> > > NICs features.
> > >
> > > However, I cannot confirm whether Wangxun NICs behave the same way.
> > > Therefore, I have kept their code as is for now.
> > >
> > > Could a Wangxun NICs maintainer provide some feedback based on testing?
> > >
> > > If we can confirm that this is indeed a bug, I can submit a separate fix.
> > >
> > > I would greatly appreciate it.
> > >
> > > Thanks.
> >
> > Hi Coia,
> >
> > I have tested this patch and it works on Wangxun NICs, thanks.
> >
> > The commit 2a22b7ae2fa3 ("net: pcs: xpcs: adapt Wangxun NICs for SGMII mode")
> > is too long ago for me to recall exactly what that thought was.
> >
> > The log shows:
> >
> > "On this device, CL37_ANSGM_STS (bit[4:1] of VR_MII_AN_INTR_STS) indicates
> > the status received from remote link during the auto-negotiation, and
> > self-clear after the auto-negotiation is complete.
> > Meanwhile, CL37_ANCMPLT_INTR will be set to 1, to indicate CL37 AN is
> > complete. So add another way to get the state for CL37 SGMII."
> >
> > I tried to reproduce this issue, but it didn't seem to exist.
> > In the current logic:
> >
> > link up -> read status from CL37_ANSGM_STS -> CL37_ANCMPLT_INTR not clear
> > link down -> read status from BMCR -> CL37_ANCMPLT_INTR clear
> >
> > It also works, although CL37_ANCMPLT_INTR is not cleared every time, and AN
> > restart is absent. But BMCR looks like it only wants to be return as 0, it is
> > weird.
> >
> > So I think Wangxun NICs also can be applied to general code as well, for
> > getting state in C37 SGMII mode.
>
> Hi Jiawen,
>
> Thanks so much for testing and confirming that the patch works on Wangxun
> NICs. That's a huge relief.
>
> You're right to ask about the CL37_ANCMPLT_INTR clear and ANRESTART. Let
> me explain what I observed on RK3568.
>
> The key difference is that on RK3568's XPCS, we're working in the MAC
> side (TX_CONFIG = 0), whereas on Wangxun it seems you're in the PHY side
> (TX_CONFIG = 1). This is based on the commit 2a22b7ae2fa3 where you set
> TX_CONFIG = 1.
Get it.
>
> > However, I am wondering whether CL37_ANCMPLT_INTR clear and ANRESTART are
> > necessary.
>
> On the RK3568 (MAC side), according to the DesignWare XPCS TRM, the
> hardware behavior is as follows:
>
> - After a software reset (BMCR_RESET), ANENABLE is automatically set to 1
> (the hardware reset value). So auto-negotiation starts automatically.
>
> - When the link is up, CL37_ANSGM_STS contains the negotiated speed/duplex
> and the link status.
>
> - When the link goes down, CL37_ANSGM_STS becomes all zero, but
> CL37_ANCMPLT_INTR remains set to 1.
>
> - Crucially, the PCS does **not** automatically restart AN when the link
> comes back up. Without an explicit ANRESTART, the link stays down
> forever.
>
> So on RK3568, the ANRESTART is mandatory to recover from a link-down
> event. Without it, the link never comes back up after a cable disconnect.
>
> In my current logic:
>
> 1. If CL37_ANSGM_STS has valid speed/duplex bits, report the link as up
> and return.
>
> 2. If CL37_ANSGM_STS is zero but CL37_ANCMPLT_INTR is set, the link is
> down. We clear the interrupt and issue an ANRESTART to start a new
> negotiation.
>
> This works reliably on RK3568 in SGMII MAC side mode.
>
> Since you confirmed that the general code path also works on Wangxun, I
> wonder: does the ANRESTART also work on your side? If yes, perhaps we can
> make this the common path for all hardware, not just RK3568.
>
> If you can test the ANRESTART logic on Wangxun and it works, I'd be happy
> to submit a standalone fix that makes this the common code path.
I've test this patch with removing the restriction on Wangxun PMA. At least
ANRESTART logic does not have a negative impact.
>
> Otherwise, the current approach (skipping the ANRESTART trigger on
> Wangxun NICs in c37_sgmii_get_state) remains safe.
>
> If you're comfortable with the change, I'd appreciate it if you could add
> your Tested-by tag to the commit. I'll also make sure to include you on
> the CC list for any future revisions or related fixes.
>
> Thanks again for testing!
Thanks for the improvements.
Tested-by: Jiawen Wu <jiawenwu@trustnetic.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v3 0/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: Chaoyi Chen @ 2026-07-17 7:23 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul,
Chaoyi Chen, Sebastian Reichel, Nicolas Frattaroli
Cc: linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip,
linux-phy
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
This series is split from the v15 "Add Type-C DP support for RK3399 EVB
IND board" series [1]. It focuses on the DRM bridge and Rockchip
platform CDN-DP controller changes.
[1] https://lore.kernel.org/all/20260304094152.92-1-kernel@airkyi.com/
====
1. Generic Type-C DP HPD bridge
Currently, several USB-C controller drivers register their own DP HPD
bridge via aux-hpd-bridge.c, each duplicating the same logic. For
devicetree based platforms, the USB-C controller may vary across boards,
and not every USB-C controller driver implements this feature. Patch 1
implements a generic DP HPD bridge that monitors Type-C bus events and
automatically creates an HPD bridge when a Type-C port device with DP
SVID is registered.
2. Multiple bridge model for CDN-DP
The RK3399 has two USB/DP combo PHY and one CDN-DP controller. Patch 5
introduces a multi-bridge model where each PHY port gets a separate
encoder and bridge, allowing flexible selection of the output PHY port.
This is based on the DRM AUX HPD bridge rather than extcon.
====
Patch 1 adds generic USB Type-C DP HPD bridge (Dmitry, Heikki).
Patch 2 adds new API drm_aux_bridge_register_from_node() (Neil).
Patch 3 adds DRM AUX bridge support for RK3399 USBDP PHY (Neil).
Patch 4 drops CDN-DP's extcon dependency when Type-C is present (Dmitry).
Patch 5 adds multiple bridges to support PHY port selection (Dmitry, Luca).
Changes in v3:
- Link to v2: https://lore.kernel.org/all/20260608070805.88-1-kernel@airkyi.com/
(Sorry, I forgot to add the "v2" title, so it looks like v1.)
- Remove now-redundant call to drm_connector_attach_encoder()
- Rebase to v7.2-rc2
Changes in v2:
- Link to v1: https://lore.kernel.org/all/20260521032854.103-1-kernel@airkyi.com/
- Add copyright text.
- Remove useless goto.
- Fix incorrect function names in the documentation comments.
- Fix the check logic for dp->active_port (0 is valid).
- Fix the uninitialized prev_port.
- Remove duplicate logic from cdn_dp_switch_port().
- Fix the prototype of cdn_dp_bridge_hpd_notify().
- Properly release connector->fwnode.
- Properly release next_bridge.
Chaoyi Chen (5):
drm/bridge: Implement generic USB Type-C DP HPD bridge
drm/bridge: aux: Add drm_aux_bridge_register_from_node()
phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
drm/rockchip: cdn-dp: Support handle lane info without extcon
drm/rockchip: cdn-dp: Add multiple bridges to support PHY port
selection
drivers/gpu/drm/bridge/Kconfig | 10 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/aux-bridge.c | 26 +-
.../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c | 54 +++
drivers/gpu/drm/rockchip/Kconfig | 1 +
drivers/gpu/drm/rockchip/cdn-dp-core.c | 358 ++++++++++++++----
drivers/gpu/drm/rockchip/cdn-dp-core.h | 18 +-
drivers/phy/rockchip/Kconfig | 2 +
drivers/phy/rockchip/phy-rockchip-typec.c | 13 +-
include/drm/bridge/aux-bridge.h | 6 +
10 files changed, 418 insertions(+), 71 deletions(-)
create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v3 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: Chaoyi Chen @ 2026-07-17 7:23 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul,
Chaoyi Chen, Sebastian Reichel, Nicolas Frattaroli
Cc: linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip,
linux-phy
In-Reply-To: <20260717072323.96-1-kernel@airkyi.com>
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
The HPD function of Type-C DP is implemented through
drm_connector_oob_hotplug_event(). For embedded DP, it is required
that the DRM connector fwnode corresponds to the Type-C port fwnode.
To describe the relationship between the DP controller and the Type-C
port device, we usually using drm_bridge to build a bridge chain.
Now several USB-C controller drivers have already implemented the DP
HPD bridge function provided by aux-hpd-bridge.c, it will build a DP
HPD bridge on USB-C connector port device.
But this requires the USB-C controller driver to manually register the
HPD bridge. If the driver does not implement this feature, the bridge
will not be create.
So this patch implements a generic DP HPD bridge based on
aux-hpd-bridge.c. It will monitor Type-C bus events, and when a
Type-C port device containing the DP svid is registered, it will
create an HPD bridge for it without the need for the USB-C controller
driver to implement it.
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
(no changes since v3)
Changes in v2:
- Add copyright text.
- Remove useless goto.
---
drivers/gpu/drm/bridge/Kconfig | 10 ++++
drivers/gpu/drm/bridge/Makefile | 1 +
.../gpu/drm/bridge/aux-hpd-typec-dp-bridge.c | 54 +++++++++++++++++++
3 files changed, 65 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 4a57d49b4c6d..9739b2a19758 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -30,6 +30,16 @@ config DRM_AUX_HPD_BRIDGE
Simple bridge that terminates the bridge chain and provides HPD
support.
+if DRM_AUX_HPD_BRIDGE
+config DRM_AUX_HPD_TYPEC_BRIDGE
+ tristate
+ depends on TYPEC || !TYPEC
+ default TYPEC
+ help
+ Simple bridge that terminates the bridge chain and provides HPD
+ support. It build bridge on each USB-C connector device node.
+endif
+
menu "Display Interface Bridges"
depends on DRM && DRM_BRIDGE
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 15cc821d85b7..d88a9e1ccc9a 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DRM_AUX_BRIDGE) += aux-bridge.o
obj-$(CONFIG_DRM_AUX_HPD_BRIDGE) += aux-hpd-bridge.o
+obj-$(CONFIG_DRM_AUX_HPD_TYPEC_BRIDGE) += aux-hpd-typec-dp-bridge.o
obj-$(CONFIG_DRM_CHIPONE_ICN6211) += chipone-icn6211.o
obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
obj-$(CONFIG_DRM_CROS_EC_ANX7688) += cros-ec-anx7688.o
diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
new file mode 100644
index 000000000000..c09579ff7ab9
--- /dev/null
+++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2026 Rockchip Electronics Co., Ltd.
+ *
+ * Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
+ */
+#include <linux/of.h>
+#include <linux/usb/typec_altmode.h>
+#include <linux/usb/typec_dp.h>
+
+#include <drm/bridge/aux-bridge.h>
+
+static int drm_typec_bus_event(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct device *dev = (struct device *)data;
+ struct typec_altmode *alt = to_typec_altmode(dev);
+
+ if (action != BUS_NOTIFY_ADD_DEVICE)
+ return NOTIFY_OK;
+
+ /*
+ * alt->dev.parent->parent : USB-C controller device
+ * alt->dev.parent : USB-C connector device
+ */
+ if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID)
+ drm_dp_hpd_bridge_register(alt->dev.parent->parent,
+ to_of_node(alt->dev.parent->fwnode));
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block drm_typec_event_nb = {
+ .notifier_call = drm_typec_bus_event,
+};
+
+static void drm_aux_hpd_typec_dp_bridge_module_exit(void)
+{
+ bus_unregister_notifier(&typec_bus, &drm_typec_event_nb);
+}
+
+static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
+{
+ bus_register_notifier(&typec_bus, &drm_typec_event_nb);
+
+ return 0;
+}
+
+module_init(drm_aux_hpd_typec_dp_bridge_module_init);
+module_exit(drm_aux_hpd_typec_dp_bridge_module_exit);
+
+MODULE_AUTHOR("Chaoyi Chen <chaoyi.chen@rock-chips.com>");
+MODULE_DESCRIPTION("DRM TYPEC DP HPD BRIDGE");
+MODULE_LICENSE("GPL");
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v3 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node()
From: Chaoyi Chen @ 2026-07-17 7:23 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul,
Chaoyi Chen, Sebastian Reichel, Nicolas Frattaroli
Cc: linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip,
linux-phy
In-Reply-To: <20260717072323.96-1-kernel@airkyi.com>
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
The drm_aux_bridge_register() uses the device->of_node as the
bridge->of_node.
This patch adds drm_aux_bridge_register_from_node() to allow
specifying the of_node corresponding to the bridge.
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
(no changes since v3)
Changes in v2:
- Fix incorrect function names in the documentation comments.
---
drivers/gpu/drm/bridge/aux-bridge.c | 26 +++++++++++++++++++++++---
include/drm/bridge/aux-bridge.h | 6 ++++++
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
index 1ed21a8713bf..06a1466f49bd 100644
--- a/drivers/gpu/drm/bridge/aux-bridge.c
+++ b/drivers/gpu/drm/bridge/aux-bridge.c
@@ -33,8 +33,9 @@ static void drm_aux_bridge_unregister_adev(void *_adev)
}
/**
- * drm_aux_bridge_register - Create a simple bridge device to link the chain
+ * drm_aux_bridge_register_from_node - Create a simple bridge device to link the chain
* @parent: device instance providing this bridge
+ * @np: device node pointer corresponding to this bridge instance
*
* Creates a simple DRM bridge that doesn't implement any drm_bridge
* operations. Such bridges merely fill a place in the bridge chain linking
@@ -42,7 +43,7 @@ static void drm_aux_bridge_unregister_adev(void *_adev)
*
* Return: zero on success, negative error code on failure
*/
-int drm_aux_bridge_register(struct device *parent)
+int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np)
{
struct auxiliary_device *adev;
int ret;
@@ -62,7 +63,10 @@ int drm_aux_bridge_register(struct device *parent)
adev->dev.parent = parent;
adev->dev.release = drm_aux_bridge_release;
- device_set_of_node_from_dev(&adev->dev, parent);
+ if (np)
+ device_set_node(&adev->dev, of_fwnode_handle(np));
+ else
+ device_set_of_node_from_dev(&adev->dev, parent);
ret = auxiliary_device_init(adev);
if (ret) {
@@ -80,6 +84,22 @@ int drm_aux_bridge_register(struct device *parent)
return devm_add_action_or_reset(parent, drm_aux_bridge_unregister_adev, adev);
}
+EXPORT_SYMBOL_GPL(drm_aux_bridge_register_from_node);
+
+/**
+ * drm_aux_bridge_register - Create a simple bridge device to link the chain
+ * @parent: device instance providing this bridge
+ *
+ * Creates a simple DRM bridge that doesn't implement any drm_bridge
+ * operations. Such bridges merely fill a place in the bridge chain linking
+ * surrounding DRM bridges.
+ *
+ * Return: zero on success, negative error code on failure
+ */
+int drm_aux_bridge_register(struct device *parent)
+{
+ return drm_aux_bridge_register_from_node(parent, NULL);
+}
EXPORT_SYMBOL_GPL(drm_aux_bridge_register);
struct drm_aux_bridge_data {
diff --git a/include/drm/bridge/aux-bridge.h b/include/drm/bridge/aux-bridge.h
index c2f5a855512f..7dd1f17a1354 100644
--- a/include/drm/bridge/aux-bridge.h
+++ b/include/drm/bridge/aux-bridge.h
@@ -13,11 +13,17 @@ struct auxiliary_device;
#if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE)
int drm_aux_bridge_register(struct device *parent);
+int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np);
#else
static inline int drm_aux_bridge_register(struct device *parent)
{
return 0;
}
+
+static inline int drm_aux_bridge_register_from_node(struct device *parent, struct device_node *np)
+{
+ return 0;
+}
#endif
#if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE)
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v3 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
From: Chaoyi Chen @ 2026-07-17 7:23 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul,
Chaoyi Chen, Sebastian Reichel, Nicolas Frattaroli
Cc: linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip,
linux-phy
In-Reply-To: <20260717072323.96-1-kernel@airkyi.com>
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
The RK3399 has two USB/DP combo PHY and one CDN-DP controller. And
the CDN-DP can be switched to output to one of the PHYs. If both ports
are plugged into DP, DP will select the first port for output.
This patch adds support for multiple bridges, enabling users to flexibly
select the output port. For each PHY port, a separate encoder and bridge
are registered.
The change is based on the DRM AUX HPD bridge, rather than the
extcon approach. This requires the DT to correctly describe the
connections between the first bridge in bridge chain and DP
controller. For example, the bridge chain may be like this:
PHY aux birdge -> fsa4480 analog audio switch bridge ->
onnn,nb7vpq904m USB reminder bridge -> USB-C controller AUX HPD bridge
In this case, the connection relationships among the PHY aux bridge
and the DP contorller need to be described in DT.
In addition, the cdn_dp_parse_next_bridge_dt() will parses it and
determines whether to register one or two bridges.
Since there is only one DP controller, only one of the PHY ports can
output at a time. The key is how to switch between different PHYs,
which is handled by cdn_dp_switch_port() and cdn_dp_enable().
There are two cases:
1. Neither bridge is enabled. In this case, both bridges can
independently read the EDID, and the PHY port may switch before
reading the EDID.
2. One bridge is already enabled. In this case, other bridges are not
allowed to read the EDID. So we will try to return the cached EDID.
Since the scenario of two ports plug in at the same time is rare,
I don't have a board which support two TypeC connector to test this.
Therefore, I tested forced switching on a single PHY port, as well as
output using a fake PHY port alongside a real PHY port.
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
Changes in v3:
- Remove now-redundant call to drm_connector_attach_encoder()
Changes in v2:
- Fix the check logic for dp->active_port (0 is valid).
- Fix the uninitialized prev_port.
- Remove duplicate logic from cdn_dp_switch_port().
- Fix the prototype of cdn_dp_bridge_hpd_notify().
- Properly release connector->fwnode.
- Properly release next_bridge.
---
drivers/gpu/drm/rockchip/Kconfig | 1 +
drivers/gpu/drm/rockchip/cdn-dp-core.c | 333 ++++++++++++++++++++-----
drivers/gpu/drm/rockchip/cdn-dp-core.h | 18 +-
3 files changed, 294 insertions(+), 58 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/Kconfig b/drivers/gpu/drm/rockchip/Kconfig
index e7f49fe845ea..ce70f91c13e6 100644
--- a/drivers/gpu/drm/rockchip/Kconfig
+++ b/drivers/gpu/drm/rockchip/Kconfig
@@ -60,6 +60,7 @@ config ROCKCHIP_CDN_DP
select DRM_DISPLAY_HELPER
select DRM_BRIDGE_CONNECTOR
select DRM_DISPLAY_DP_HELPER
+ select DRM_AUX_HPD_BRIDGE
help
This selects support for Rockchip SoC specific extensions
for the cdn DP driver. If you want to enable Dp on
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 74e1f5422a6e..5e513f6930cb 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -28,16 +28,17 @@
#include "cdn-dp-core.h"
#include "cdn-dp-reg.h"
-static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge)
+static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port,
+ struct cdn_dp_port *port);
+
+static inline struct cdn_dp_bridge *bridge_to_dp_bridge(struct drm_bridge *bridge)
{
- return container_of(bridge, struct cdn_dp_device, bridge);
+ return container_of(bridge, struct cdn_dp_bridge, bridge);
}
-static inline struct cdn_dp_device *encoder_to_dp(struct drm_encoder *encoder)
+static inline struct cdn_dp_device *bridge_to_dp(struct drm_bridge *bridge)
{
- struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
-
- return container_of(rkencoder, struct cdn_dp_device, encoder);
+ return bridge_to_dp_bridge(bridge)->parent;
}
#define GRF_SOC_CON9 0x6224
@@ -192,14 +193,27 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
static struct cdn_dp_port *cdn_dp_connected_port(struct cdn_dp_device *dp)
{
struct cdn_dp_port *port;
- int i, lanes;
+ int i, lanes[MAX_PHY];
for (i = 0; i < dp->ports; i++) {
port = dp->port[i];
- lanes = cdn_dp_get_port_lanes(port);
- if (lanes)
+ lanes[i] = cdn_dp_get_port_lanes(port);
+ if (!dp->next_bridge_valid)
return port;
}
+
+ if (dp->next_bridge_valid) {
+ /* If more than one port is available, pick the last active port */
+ if (dp->active_port >= 0 && lanes[dp->active_port])
+ return dp->port[dp->active_port];
+
+ /* If the last active port is not available, pick an available port in order */
+ for (i = 0; i < dp->bridge_count; i++) {
+ if (lanes[i])
+ return dp->port[i];
+ }
+ }
+
return NULL;
}
@@ -254,12 +268,45 @@ static const struct drm_edid *
cdn_dp_bridge_edid_read(struct drm_bridge *bridge, struct drm_connector *connector)
{
struct cdn_dp_device *dp = bridge_to_dp(bridge);
- const struct drm_edid *drm_edid;
+ struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
+ struct cdn_dp_port *port = dp->port[dp_bridge->id];
+ struct cdn_dp_port *prev_port = NULL;
+ const struct drm_edid *drm_edid = NULL;
+ int i, ret;
mutex_lock(&dp->lock);
+
+ /* More than one port is available */
+ if (dp->bridge_count > 1 && !port->phy_enabled) {
+ for (i = 0; i < dp->bridge_count; i++) {
+ /* Another port already enable */
+ if (dp->bridge_list[i] != dp_bridge && dp->bridge_list[i]->enabled)
+ goto get_cache;
+ /* Find already enabled port */
+ if (dp->port[i]->phy_enabled)
+ prev_port = dp->port[i];
+ }
+
+ /* Switch to current port */
+ if (prev_port) {
+ ret = cdn_dp_switch_port(dp, prev_port, port);
+ if (ret)
+ goto get_cache;
+ }
+ }
+
drm_edid = drm_edid_read_custom(connector, cdn_dp_get_edid_block, dp);
+ /* replace edid cache */
+ if (dp->edid_cache[dp_bridge->id])
+ drm_edid_free(dp->edid_cache[dp_bridge->id]);
+ dp->edid_cache[dp_bridge->id] = drm_edid_dup(drm_edid);
+
mutex_unlock(&dp->lock);
+ return drm_edid;
+get_cache:
+ drm_edid = drm_edid_dup(dp->edid_cache[dp_bridge->id]);
+ mutex_unlock(&dp->lock);
return drm_edid;
}
@@ -268,12 +315,13 @@ cdn_dp_bridge_mode_valid(struct drm_bridge *bridge,
const struct drm_display_info *display_info,
const struct drm_display_mode *mode)
{
+ struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
struct cdn_dp_device *dp = bridge_to_dp(bridge);
u32 requested, actual, rate, sink_max, source_max = 0;
u8 lanes, bpc;
/* If DP is disconnected, every mode is invalid */
- if (!dp->connected)
+ if (!dp_bridge->connected || !dp->connected)
return MODE_BAD;
switch (display_info->bpc) {
@@ -551,6 +599,50 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
return drm_dp_channel_eq_ok(link_status, min(port->lanes, sink_lanes));
}
+static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port,
+ struct cdn_dp_port *port)
+{
+ int ret;
+
+ ret = cdn_dp_disable_phy(dp, prev_port);
+ if (ret)
+ goto out;
+ ret = cdn_dp_enable_phy(dp, port);
+ if (ret)
+ goto out;
+
+ ret = cdn_dp_get_sink_capability(dp);
+ if (ret) {
+ cdn_dp_disable_phy(dp, port);
+ goto out;
+ }
+
+ dp->lanes = port->lanes;
+
+ if (!cdn_dp_check_link_status(dp)) {
+ dev_info(dp->dev, "Connected with sink; re-train link\n");
+
+ ret = cdn_dp_train_link(dp);
+ if (ret) {
+ dev_err(dp->dev, "Training link failed: %d\n", ret);
+ goto out;
+ }
+
+ ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
+ if (ret) {
+ dev_err(dp->dev, "Failed to idle video %d\n", ret);
+ goto out;
+ }
+
+ ret = cdn_dp_config_video(dp);
+ if (ret)
+ dev_err(dp->dev, "Failed to configure video: %d\n", ret);
+ }
+
+out:
+ return ret;
+}
+
static void cdn_dp_display_info_update(struct cdn_dp_device *dp,
struct drm_display_info *display_info)
{
@@ -572,6 +664,7 @@ static void cdn_dp_display_info_update(struct cdn_dp_device *dp,
static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_atomic_commit *state)
{
struct cdn_dp_device *dp = bridge_to_dp(bridge);
+ struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
struct drm_connector *connector;
int ret, val;
@@ -581,7 +674,7 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
cdn_dp_display_info_update(dp, &connector->display_info);
- ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, &dp->encoder.encoder);
+ ret = drm_of_encoder_active_endpoint_id(dp->dev->of_node, &dp_bridge->encoder.encoder);
if (ret < 0) {
DRM_DEV_ERROR(dp->dev, "Could not get vop id, %d", ret);
return;
@@ -600,6 +693,9 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
mutex_lock(&dp->lock);
+ if (dp->next_bridge_valid)
+ dp->active_port = dp_bridge->id;
+
ret = cdn_dp_enable(dp);
if (ret) {
DRM_DEV_ERROR(dp->dev, "Failed to enable bridge %d\n",
@@ -632,6 +728,7 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
goto out;
}
+ dp_bridge->enabled = true;
out:
mutex_unlock(&dp->lock);
}
@@ -639,9 +736,11 @@ static void cdn_dp_bridge_atomic_enable(struct drm_bridge *bridge, struct drm_at
static void cdn_dp_bridge_atomic_disable(struct drm_bridge *bridge, struct drm_atomic_commit *state)
{
struct cdn_dp_device *dp = bridge_to_dp(bridge);
+ struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
int ret;
mutex_lock(&dp->lock);
+ dp_bridge->enabled = false;
if (dp->active) {
ret = cdn_dp_disable(dp);
@@ -828,6 +927,17 @@ static int cdn_dp_audio_mute_stream(struct drm_bridge *bridge,
return ret;
}
+static void cdn_dp_bridge_hpd_notify(struct drm_bridge *bridge,
+ struct drm_connector *connector,
+ enum drm_connector_status status)
+{
+ struct cdn_dp_bridge *dp_bridge = bridge_to_dp_bridge(bridge);
+ struct cdn_dp_device *dp = bridge_to_dp(bridge);
+
+ dp->bridge_list[dp_bridge->id]->connected = status == connector_status_connected;
+ schedule_work(&dp->event_work);
+}
+
static const struct drm_bridge_funcs cdn_dp_bridge_funcs = {
.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
@@ -838,6 +948,7 @@ static const struct drm_bridge_funcs cdn_dp_bridge_funcs = {
.atomic_disable = cdn_dp_bridge_atomic_disable,
.mode_valid = cdn_dp_bridge_mode_valid,
.mode_set = cdn_dp_bridge_mode_set,
+ .hpd_notify = cdn_dp_bridge_hpd_notify,
.dp_audio_prepare = cdn_dp_audio_prepare,
.dp_audio_mute_stream = cdn_dp_audio_mute_stream,
@@ -886,7 +997,8 @@ static void cdn_dp_pd_event_work(struct work_struct *work)
{
struct cdn_dp_device *dp = container_of(work, struct cdn_dp_device,
event_work);
- int ret;
+ bool connected;
+ int i, ret;
mutex_lock(&dp->lock);
@@ -945,9 +1057,12 @@ static void cdn_dp_pd_event_work(struct work_struct *work)
out:
mutex_unlock(&dp->lock);
- drm_bridge_hpd_notify(&dp->bridge,
- dp->connected ? connector_status_connected
- : connector_status_disconnected);
+ for (i = 0; i < dp->bridge_count; i++) {
+ connected = dp->connected && dp->bridge_list[i]->connected;
+ drm_bridge_hpd_notify(&dp->bridge_list[i]->bridge,
+ connected ? connector_status_connected
+ : connector_status_disconnected);
+ }
}
static int cdn_dp_pd_event(struct notifier_block *nb,
@@ -967,28 +1082,16 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
return NOTIFY_DONE;
}
-static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
+static int cdn_bridge_add(struct device *dev,
+ struct drm_bridge *bridge,
+ struct drm_bridge *next_bridge,
+ struct drm_encoder *encoder)
{
struct cdn_dp_device *dp = dev_get_drvdata(dev);
- struct drm_encoder *encoder;
+ struct drm_device *drm_dev = dp->drm_dev;
+ struct drm_bridge *last_bridge __free(drm_bridge_put) = NULL;
struct drm_connector *connector;
- struct cdn_dp_port *port;
- struct drm_device *drm_dev = data;
- int ret, i;
-
- ret = cdn_dp_parse_dt(dp);
- if (ret < 0)
- return ret;
-
- dp->drm_dev = drm_dev;
- dp->connected = false;
- dp->active = false;
- dp->active_port = -1;
- dp->fw_loaded = false;
-
- INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
-
- encoder = &dp->encoder.encoder;
+ int ret;
encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
dev->of_node);
@@ -1003,26 +1106,35 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
drm_encoder_helper_add(encoder, &cdn_dp_encoder_helper_funcs);
- dp->bridge.ops =
- DRM_BRIDGE_OP_DETECT |
- DRM_BRIDGE_OP_EDID |
- DRM_BRIDGE_OP_HPD |
- DRM_BRIDGE_OP_DP_AUDIO;
- dp->bridge.of_node = dp->dev->of_node;
- dp->bridge.type = DRM_MODE_CONNECTOR_DisplayPort;
- dp->bridge.hdmi_audio_dev = dp->dev;
- dp->bridge.hdmi_audio_max_i2s_playback_channels = 8;
- dp->bridge.hdmi_audio_spdif_playback = 1;
- dp->bridge.hdmi_audio_dai_port = -1;
-
- ret = devm_drm_bridge_add(dev, &dp->bridge);
+ bridge->ops =
+ DRM_BRIDGE_OP_DETECT |
+ DRM_BRIDGE_OP_EDID |
+ DRM_BRIDGE_OP_HPD |
+ DRM_BRIDGE_OP_DP_AUDIO;
+ bridge->of_node = dp->dev->of_node;
+ bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
+ bridge->hdmi_audio_dev = dp->dev;
+ bridge->hdmi_audio_max_i2s_playback_channels = 8;
+ bridge->hdmi_audio_spdif_playback = 1;
+ bridge->hdmi_audio_dai_port = -1;
+
+ ret = devm_drm_bridge_add(dev, bridge);
if (ret)
return ret;
- ret = drm_bridge_attach(encoder, &dp->bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+ ret = drm_bridge_attach(encoder, bridge, NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR);
if (ret)
return ret;
+ if (next_bridge) {
+ ret = drm_bridge_attach(encoder, next_bridge, bridge,
+ DRM_BRIDGE_ATTACH_NO_CONNECTOR);
+ if (ret)
+ return ret;
+
+ last_bridge = drm_bridge_chain_get_last_bridge(bridge->encoder);
+ }
+
connector = drm_bridge_connector_init(drm_dev, encoder);
if (IS_ERR(connector)) {
ret = PTR_ERR(connector);
@@ -1030,6 +1142,101 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
return ret;
}
+ if (last_bridge) {
+ fwnode_handle_put(connector->fwnode);
+ connector->fwnode = fwnode_handle_get(of_fwnode_handle(last_bridge->of_node));
+ }
+
+ return 0;
+}
+
+static int cdn_dp_parse_next_bridge_dt(struct cdn_dp_device *dp)
+{
+ struct device_node *np = dp->dev->of_node;
+ struct device_node *port __free(device_node) = of_graph_get_port_by_id(np, 1);
+ struct drm_bridge *bridge;
+ int count = 0;
+ int ret = 0;
+ int i;
+
+ /* If device use extcon, do not use hpd bridge */
+ for (i = 0; i < dp->ports; i++) {
+ if (dp->port[i]->extcon) {
+ dp->bridge_count = 1;
+ return 0;
+ }
+ }
+
+ /* One endpoint may correspond to one next bridge. */
+ for_each_of_graph_port_endpoint(port, dp_ep) {
+ struct device_node *next_bridge_node __free(device_node) =
+ of_graph_get_remote_port_parent(dp_ep);
+
+ bridge = of_drm_find_bridge(next_bridge_node);
+ if (!bridge) {
+ ret = -EPROBE_DEFER;
+ goto out;
+ }
+
+ dp->next_bridge_valid = true;
+ dp->next_bridge_list[count] = drm_bridge_get(bridge);
+ count++;
+ }
+
+out:
+ dp->bridge_count = count ? count : 1;
+ return ret;
+}
+
+static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
+{
+ struct cdn_dp_device *dp = dev_get_drvdata(dev);
+ struct drm_bridge *bridge, *next_bridge;
+ struct drm_encoder *encoder;
+ struct cdn_dp_port *port;
+ struct drm_device *drm_dev = data;
+ struct cdn_dp_bridge *dp_bridge;
+ int ret, i;
+
+ ret = cdn_dp_parse_dt(dp);
+ if (ret < 0)
+ return ret;
+
+ ret = cdn_dp_parse_next_bridge_dt(dp);
+ if (ret)
+ goto out;
+
+ dp->drm_dev = drm_dev;
+ dp->connected = false;
+ dp->active = false;
+ dp->active_port = -1;
+ dp->fw_loaded = false;
+
+ for (i = 0; i < dp->bridge_count; i++) {
+ dp_bridge = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
+ &cdn_dp_bridge_funcs);
+ if (IS_ERR(dp_bridge)) {
+ ret = PTR_ERR(dp_bridge);
+ goto out;
+ }
+ dp_bridge->id = i;
+ dp_bridge->parent = dp;
+ if (!dp->next_bridge_valid)
+ dp_bridge->connected = true;
+ dp->bridge_list[i] = dp_bridge;
+ }
+
+ for (i = 0; i < dp->bridge_count; i++) {
+ encoder = &dp->bridge_list[i]->encoder.encoder;
+ bridge = &dp->bridge_list[i]->bridge;
+ next_bridge = dp->next_bridge_list[i];
+ ret = cdn_bridge_add(dev, bridge, next_bridge, encoder);
+ if (ret)
+ goto out;
+ }
+
+ INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
+
for (i = 0; i < dp->ports; i++) {
port = dp->port[i];
@@ -1043,7 +1250,7 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
if (ret) {
DRM_DEV_ERROR(dev,
"register EXTCON_DISP_DP notifier err\n");
- return ret;
+ goto out;
}
}
@@ -1052,15 +1259,29 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
schedule_work(&dp->event_work);
return 0;
+
+out:
+ for (i = 0; i < MAX_PHY; i++)
+ drm_bridge_put(dp->next_bridge_list[i]);
+
+ return ret;
}
static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)
{
struct cdn_dp_device *dp = dev_get_drvdata(dev);
- struct drm_encoder *encoder = &dp->encoder.encoder;
+ struct drm_encoder *encoder;
+ int i;
cancel_work_sync(&dp->event_work);
- encoder->funcs->destroy(encoder);
+ for (i = 0; i < dp->bridge_count; i++) {
+ encoder = &dp->bridge_list[i]->encoder.encoder;
+ encoder->funcs->destroy(encoder);
+ drm_bridge_put(dp->next_bridge_list[i]);
+ }
+
+ for (i = 0; i < MAX_PHY; i++)
+ drm_edid_free(dp->edid_cache[i]);
pm_runtime_disable(dev);
if (dp->fw_loaded)
@@ -1111,10 +1332,10 @@ static int cdn_dp_probe(struct platform_device *pdev)
int ret;
int i;
- dp = devm_drm_bridge_alloc(dev, struct cdn_dp_device, bridge,
- &cdn_dp_bridge_funcs);
- if (IS_ERR(dp))
- return PTR_ERR(dp);
+ dp = devm_kzalloc(dev, sizeof(*dp), GFP_KERNEL);
+ if (!dp)
+ return -ENOMEM;
+
dp->dev = dev;
match = of_match_node(cdn_dp_dt_ids, pdev->dev.of_node);
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
index e9c30b9fd543..c10e423bbf06 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
@@ -38,6 +38,8 @@ enum vic_pxl_encoding_format {
Y_ONLY = 0x10,
};
+struct cdn_dp_device;
+
struct video_info {
bool h_sync_polarity;
bool v_sync_polarity;
@@ -63,16 +65,28 @@ struct cdn_dp_port {
u8 id;
};
+struct cdn_dp_bridge {
+ struct cdn_dp_device *parent;
+ struct drm_bridge bridge;
+ struct rockchip_encoder encoder;
+ bool connected;
+ bool enabled;
+ int id;
+};
+
struct cdn_dp_device {
struct device *dev;
struct drm_device *drm_dev;
- struct drm_bridge bridge;
- struct rockchip_encoder encoder;
+ int bridge_count;
+ struct cdn_dp_bridge *bridge_list[MAX_PHY];
+ struct drm_bridge *next_bridge_list[MAX_PHY];
+ const struct drm_edid *edid_cache[MAX_PHY];
struct drm_display_mode mode;
struct platform_device *audio_pdev;
struct work_struct event_work;
struct mutex lock;
+ bool next_bridge_valid;
bool connected;
bool active;
bool suspended;
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v3 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
From: Chaoyi Chen @ 2026-07-17 7:23 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul,
Chaoyi Chen, Sebastian Reichel, Nicolas Frattaroli
Cc: linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip,
linux-phy
In-Reply-To: <20260717072323.96-1-kernel@airkyi.com>
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge
device.
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
(no changes since v1)
---
drivers/phy/rockchip/Kconfig | 2 ++
drivers/phy/rockchip/phy-rockchip-typec.c | 13 +++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
index 14698571b607..9173d3b4fef4 100644
--- a/drivers/phy/rockchip/Kconfig
+++ b/drivers/phy/rockchip/Kconfig
@@ -119,6 +119,8 @@ config PHY_ROCKCHIP_SNPS_PCIE3
config PHY_ROCKCHIP_TYPEC
tristate "Rockchip TYPEC PHY Driver"
depends on OF && (ARCH_ROCKCHIP || COMPILE_TEST)
+ depends on DRM || DRM=n
+ select DRM_AUX_BRIDGE if DRM_BRIDGE
select EXTCON
select GENERIC_PHY
select RESET_CONTROLLER
diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
index d9701b6106d5..48070b50416e 100644
--- a/drivers/phy/rockchip/phy-rockchip-typec.c
+++ b/drivers/phy/rockchip/phy-rockchip-typec.c
@@ -54,6 +54,7 @@
#include <linux/mfd/syscon.h>
#include <linux/phy/phy.h>
+#include <drm/bridge/aux-bridge.h>
#define CMN_SSM_BANDGAP (0x21 << 2)
#define CMN_SSM_BIAS (0x22 << 2)
@@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
for_each_available_child_of_node(np, child_np) {
struct phy *phy;
+ ret = 0;
- if (of_node_name_eq(child_np, "dp-port"))
+ if (of_node_name_eq(child_np, "dp-port")) {
phy = devm_phy_create(dev, child_np,
&rockchip_dp_phy_ops);
- else if (of_node_name_eq(child_np, "usb3-port"))
+ ret = drm_aux_bridge_register_from_node(dev, child_np);
+ } else if (of_node_name_eq(child_np, "usb3-port"))
phy = devm_phy_create(dev, child_np,
&rockchip_usb3_phy_ops);
else
continue;
+ if (ret) {
+ pm_runtime_disable(dev);
+ of_node_put(child_np);
+ return ret;
+ }
+
if (IS_ERR(phy)) {
dev_err(dev, "failed to create phy: %pOFn\n",
child_np);
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v3 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon
From: Chaoyi Chen @ 2026-07-17 7:23 UTC (permalink / raw)
To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
Sandy Huang, Heiko Stübner, Andy Yan, Vinod Koul,
Chaoyi Chen, Sebastian Reichel, Nicolas Frattaroli
Cc: linux-kernel, dri-devel, linux-arm-kernel, linux-rockchip,
linux-phy
In-Reply-To: <20260717072323.96-1-kernel@airkyi.com>
From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
This patch add support for get PHY lane info without help of extcon.
There is no extcon needed if the Type-C controller is present. In this
case, the lane info can be get from PHY instead of extcon.
The extcon device should still be supported if Type-C controller is
not present.
Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
(no changes since v1)
---
drivers/gpu/drm/rockchip/cdn-dp-core.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
index 190cce9530c8..74e1f5422a6e 100644
--- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
+++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
@@ -157,6 +157,9 @@ static int cdn_dp_get_port_lanes(struct cdn_dp_port *port)
int dptx;
u8 lanes;
+ if (!edev)
+ return phy_get_bus_width(port->phy);
+
dptx = extcon_get_state(edev, EXTCON_DISP_DP);
if (dptx > 0) {
extcon_get_property(edev, EXTCON_DISP_DP,
@@ -220,7 +223,7 @@ static bool cdn_dp_check_sink_connection(struct cdn_dp_device *dp)
* some docks need more time to power up.
*/
while (time_before(jiffies, timeout)) {
- if (!extcon_get_state(port->extcon, EXTCON_DISP_DP))
+ if (port->extcon && !extcon_get_state(port->extcon, EXTCON_DISP_DP))
return false;
if (!cdn_dp_get_sink_count(dp, &sink_count))
@@ -386,11 +389,14 @@ static int cdn_dp_enable_phy(struct cdn_dp_device *dp, struct cdn_dp_port *port)
goto err_power_on;
}
- ret = extcon_get_property(port->extcon, EXTCON_DISP_DP,
- EXTCON_PROP_USB_TYPEC_POLARITY, &property);
- if (ret) {
- DRM_DEV_ERROR(dp->dev, "get property failed\n");
- goto err_power_on;
+ property.intval = 0;
+ if (port->extcon) {
+ ret = extcon_get_property(port->extcon, EXTCON_DISP_DP,
+ EXTCON_PROP_USB_TYPEC_POLARITY, &property);
+ if (ret) {
+ DRM_DEV_ERROR(dp->dev, "get property failed\n");
+ goto err_power_on;
+ }
}
port->lanes = cdn_dp_get_port_lanes(port);
@@ -1027,6 +1033,9 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
for (i = 0; i < dp->ports; i++) {
port = dp->port[i];
+ if (!port->extcon)
+ continue;
+
port->event_nb.notifier_call = cdn_dp_pd_event;
ret = devm_extcon_register_notifier(dp->dev, port->extcon,
EXTCON_DISP_DP,
@@ -1119,14 +1128,14 @@ static int cdn_dp_probe(struct platform_device *pdev)
PTR_ERR(phy) == -EPROBE_DEFER)
return -EPROBE_DEFER;
- if (IS_ERR(extcon) || IS_ERR(phy))
+ if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV)
continue;
port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
if (!port)
return -ENOMEM;
- port->extcon = extcon;
+ port->extcon = IS_ERR(extcon) ? NULL : extcon;
port->phy = phy;
port->dp = dp;
port->id = i;
--
2.53.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH v3 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node()
From: sashiko-bot @ 2026-07-17 7:33 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: vkoul, neil.armstrong, dri-devel, linux-phy, olteanv
In-Reply-To: <20260717072323.96-3-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbalanced `of_node_put()` in `drm_aux_bridge_register_from_node()` and `drm_aux_bridge_release()` causes a reference count underflow for the provided `device_node`.
--
--- Patch [2]: [PATCH v3 2/5] drm/bridge: aux: Add drm_aux_bridge_register_from_node() ---
commit b2b66972200bdc4b6541ca41f8ae44c08d81a964
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
drm/bridge: aux: Add drm_aux_bridge_register_from_node()
The drm_aux_bridge_register() uses the device->of_node as the bridge->of_node.
This patch adds drm_aux_bridge_register_from_node() to allow specifying the
of_node corresponding to the bridge.
> diff --git a/drivers/gpu/drm/bridge/aux-bridge.c b/drivers/gpu/drm/bridge/aux-bridge.c
> index 1ed21a8713bfd..06a1466f49bd2 100644
> --- a/drivers/gpu/drm/bridge/aux-bridge.c
> +++ b/drivers/gpu/drm/bridge/aux-bridge.c
[ ... ]
> @@ -62,7 +63,10 @@ int drm_aux_bridge_register(struct device *parent)
> adev->dev.parent = parent;
> adev->dev.release = drm_aux_bridge_release;
>
> - device_set_of_node_from_dev(&adev->dev, parent);
> + if (np)
> + device_set_node(&adev->dev, of_fwnode_handle(np));
[Severity: High]
Does this cause a reference count underflow for the device node?
Unlike device_set_of_node_from_dev() which safely acquires a reference to the
of_node, device_set_node() just assigns the pointer without calling
of_node_get().
If auxiliary_device_init() fails, the error path below unconditionally calls
of_node_put(adev->dev.of_node). Similarly, the pre-existing
drm_aux_bridge_release() callback drops a reference when the device is
destroyed.
Could this cause of_node_put() to drop a reference that was never acquired?
> + else
> + device_set_of_node_from_dev(&adev->dev, parent);
>
> ret = auxiliary_device_init(adev);
> if (ret) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717072323.96-1-kernel@airkyi.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge
From: sashiko-bot @ 2026-07-17 7:36 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: vkoul, neil.armstrong, olteanv, dri-devel, linux-phy
In-Reply-To: <20260717072323.96-2-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The module registers a bus notifier to catch device additions but fails to iterate over already-registered devices, ignoring any Type-C ports present before the module was loaded.
- [High] The module attaches devm-managed bridges to a foreign USB-C controller device but fails to clean them up on module unload or device removal, leading to resource leaks and an execute-after-free kernel panic.
- [Medium] The return value of `bus_register_notifier()` is ignored.
--
--- Patch [1]: [PATCH v3 1/5] drm/bridge: Implement generic USB Type-C DP HPD bridge ---
commit 5c3c19cbb4491c572fdd386c797dfba9412af2b3
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
drm/bridge: Implement generic USB Type-C DP HPD bridge
This patch implements a generic DP HPD bridge based on aux-hpd-bridge.c.
It monitors Type-C bus events and automatically creates an HPD bridge when
a Type-C port device containing the DP svid is registered.
> diff --git a/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/aux-hpd-typec-dp-bridge.c
> @@ -0,0 +1,54 @@
[ ... ]
> +static int drm_typec_bus_event(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct device *dev = (struct device *)data;
> + struct typec_altmode *alt = to_typec_altmode(dev);
> +
> + if (action != BUS_NOTIFY_ADD_DEVICE)
> + return NOTIFY_OK;
> +
> + /*
> + * alt->dev.parent->parent : USB-C controller device
> + * alt->dev.parent : USB-C connector device
> + */
> + if (is_typec_port_altmode(&alt->dev) && alt->svid == USB_TYPEC_DP_SID)
> + drm_dp_hpd_bridge_register(alt->dev.parent->parent,
> + to_of_node(alt->dev.parent->fwnode));
[Severity: High]
Will this leak devm resources on the controller device when a port is removed
and re-added, since BUS_NOTIFY_DEL_DEVICE is ignored?
Also, because the devm cleanup callbacks are tied to the controller device
rather than this module, could this cause an execute-after-free panic?
If this module and aux-hpd-bridge are unloaded, the devres callbacks on the
controller will point to unmapped memory when the controller is later unbound.
[ ... ]
> +static int __init drm_aux_hpd_typec_dp_bridge_module_init(void)
> +{
> + bus_register_notifier(&typec_bus, &drm_typec_event_nb);
[Severity: Medium]
Is it safe to ignore the return value of bus_register_notifier() here?
If the notifier registration fails, the module initialization will succeed but
remain non-functional.
[Severity: High]
Does this code miss existing Type-C DP ports that were probed before this
module was loaded?
bus_register_notifier() only triggers notifications for devices added after
registration. Since there is no bus_for_each_dev() iteration over existing
devices on the typec_bus after registering the notifier, pre-existing ports
might never get an HPD bridge created.
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717072323.96-1-kernel@airkyi.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
From: sashiko-bot @ 2026-07-17 7:38 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: dri-devel, olteanv, vkoul, linux-phy, neil.armstrong
In-Reply-To: <20260717072323.96-6-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [High] Buffer overflow when parsing device tree endpoints due to missing bounds check.
- [High] Broken hotplug detection for legacy extcon configurations due to unconditional port return.
- [High] NULL pointer dereference and uninitialized memory read if DT endpoints exceed valid PHYs.
- [High] Port switching during EDID read always fails because `cdn_dp_config_video` is called with a zero clock.
- [High] Stale pointers in persistent arrays cause double-free and use-after-free on component re-bind.
- [High] Workqueue `dp->event_work` is scheduled before being initialized, leading to a NULL pointer dereference.
- [Medium] Multiple encoders share a single hardware DP controller but lack an `atomic_check` callback to prevent simultaneous activation.
Pre-existing issues:
- [High] Using `devm_` APIs in the component `bind` callback causes severe resource leaks and active stale notifiers after `unbind`.
--
--- Patch [5]: [PATCH v3 5/5] drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection ---
commit a0d8b7a8a3a01d68ddf971ecfdb92ccb11c60b09
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
drm/rockchip: cdn-dp: Add multiple bridges to support PHY port selection
This patch adds support for multiple bridges to allow selecting between
different PHY ports on the RK3399 CDN-DP controller. It registers a separate
encoder and bridge for each PHY port and parses the DT to handle bridge
chain configurations.
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 74e1f5422a6e2..5e513f6930cb9 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
[ ... ]
> @@ -192,14 +193,27 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
> static struct cdn_dp_port *cdn_dp_connected_port(struct cdn_dp_device *dp)
> {
> struct cdn_dp_port *port;
> - int i, lanes;
> + int i, lanes[MAX_PHY];
>
> for (i = 0; i < dp->ports; i++) {
> port = dp->port[i];
> - lanes = cdn_dp_get_port_lanes(port);
> - if (lanes)
> + lanes[i] = cdn_dp_get_port_lanes(port);
> + if (!dp->next_bridge_valid)
> return port;
[Severity: High]
Does this break hotplug detection for legacy extcon configurations?
By removing the check for lanes > 0, this will unconditionally return the
first port regardless of actual connection status.
> }
> +
> + if (dp->next_bridge_valid) {
> + /* If more than one port is available, pick the last active port */
> + if (dp->active_port >= 0 && lanes[dp->active_port])
> + return dp->port[dp->active_port];
> +
> + /* If the last active port is not available, pick an available port in order */
> + for (i = 0; i < dp->bridge_count; i++) {
> + if (lanes[i])
> + return dp->port[i];
> + }
> + }
[Severity: High]
Could this lead to an uninitialized memory read and NULL pointer
dereference?
If the device tree provides more endpoints than probed PHYs,
dp->bridge_count will exceed dp->ports. This loop will access
uninitialized stack memory in lanes[i] and potentially return a NULL
dp->port[i], causing a crash later in cdn_dp_bridge_edid_read() when
checking port->phy_enabled.
[ ... ]
> @@ -551,6 +599,50 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
> +static int cdn_dp_switch_port(struct cdn_dp_device *dp, struct cdn_dp_port *prev_port,
> + struct cdn_dp_port *port)
> +{
[ ... ]
> + if (!cdn_dp_check_link_status(dp)) {
> + dev_info(dp->dev, "Connected with sink; re-train link\n");
> +
> + ret = cdn_dp_train_link(dp);
> + if (ret) {
> + dev_err(dp->dev, "Training link failed: %d\n", ret);
> + goto out;
> + }
> +
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> + if (ret) {
> + dev_err(dp->dev, "Failed to idle video %d\n", ret);
> + goto out;
> + }
> +
> + ret = cdn_dp_config_video(dp);
> + if (ret)
> + dev_err(dp->dev, "Failed to configure video: %d\n", ret);
> + }
[Severity: High]
Will this port switch fail consistently during EDID read operations?
When cdn_dp_bridge_edid_read() calls this function, dp->mode.clock is
typically 0. A zero clock will cause cdn_dp_config_video() to spin until
timeout and return -EINVAL, thus aborting the EDID read.
[ ... ]
> @@ -828,6 +927,17 @@ static int cdn_dp_audio_mute_stream(struct drm_bridge *bridge,
> static const struct drm_bridge_funcs cdn_dp_bridge_funcs = {
> .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> .atomic_reset = drm_atomic_helper_bridge_reset,
> .detect = cdn_dp_bridge_detect,
> .edid_read = cdn_dp_bridge_edid_read,
> .atomic_enable = cdn_dp_bridge_atomic_enable,
> .atomic_disable = cdn_dp_bridge_atomic_disable,
> .mode_valid = cdn_dp_bridge_mode_valid,
> .mode_set = cdn_dp_bridge_mode_set,
> + .hpd_notify = cdn_dp_bridge_hpd_notify,
[Severity: Medium]
Should there be an atomic_check callback here to prevent simultaneous
activation of multiple CRTCs?
Since all registered encoders share the same possible_crtcs mask for a
single underlying DP controller, userspace could potentially enable
multiple displays at once, leading to hardware register corruption.
[ ... ]
> @@ -1030,6 +1142,101 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> +static int cdn_dp_parse_next_bridge_dt(struct cdn_dp_device *dp)
> +{
[ ... ]
> + for_each_of_graph_port_endpoint(port, dp_ep) {
> + struct device_node *next_bridge_node __free(device_node) =
> + of_graph_get_remote_port_parent(dp_ep);
> +
> + bridge = of_drm_find_bridge(next_bridge_node);
> + if (!bridge) {
> + ret = -EPROBE_DEFER;
> + goto out;
> + }
> +
> + dp->next_bridge_valid = true;
> + dp->next_bridge_list[count] = drm_bridge_get(bridge);
> + count++;
> + }
[Severity: High]
Could this code overflow the dp->next_bridge_list[] array?
The count variable is incremented for each endpoint and used as an index,
but it is never checked against MAX_PHY.
[ ... ]
> @@ -1030,6 +1142,101 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> + for (i = 0; i < dp->bridge_count; i++) {
> + dp_bridge = devm_drm_bridge_alloc(dev, struct cdn_dp_bridge, bridge,
> + &cdn_dp_bridge_funcs);
[Severity: High]
This is a pre-existing issue, but does using devm_ APIs inside a component
bind callback cause memory leaks and use-after-free crashes?
Component unbind does not release devm_ resources, meaning structures like
dp_bridge and the extcon notifiers remain active after unbind and will
leak on subsequent re-binds.
[ ... ]
> + for (i = 0; i < dp->bridge_count; i++) {
> + encoder = &dp->bridge_list[i]->encoder.encoder;
> + bridge = &dp->bridge_list[i]->bridge;
> + next_bridge = dp->next_bridge_list[i];
> + ret = cdn_bridge_add(dev, bridge, next_bridge, encoder);
> + if (ret)
> + goto out;
> + }
> +
> + INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
[Severity: High]
Can this sequence cause a NULL pointer dereference?
By registering the bridges via cdn_bridge_add() before calling INIT_WORK,
an early HPD event can trigger cdn_dp_bridge_hpd_notify() and attempt to
schedule the uninitialized dp->event_work queue.
[ ... ]
> +out:
> + for (i = 0; i < MAX_PHY; i++)
> + drm_bridge_put(dp->next_bridge_list[i]);
> +
> + return ret;
> }
>
> static void cdn_dp_unbind(struct device *dev, struct device *master, void *data)
> {
> struct cdn_dp_device *dp = dev_get_drvdata(dev);
> - struct drm_encoder *encoder = &dp->encoder.encoder;
> + struct drm_encoder *encoder;
> + int i;
>
> cancel_work_sync(&dp->event_work);
> - encoder->funcs->destroy(encoder);
> + for (i = 0; i < dp->bridge_count; i++) {
> + encoder = &dp->bridge_list[i]->encoder.encoder;
> + encoder->funcs->destroy(encoder);
> + drm_bridge_put(dp->next_bridge_list[i]);
> + }
> +
> + for (i = 0; i < MAX_PHY; i++)
> + drm_edid_free(dp->edid_cache[i]);
[Severity: High]
Does this leave dangling pointers in persistent arrays across bind/unbind
cycles?
Because dp is allocated in probe, freeing dp->next_bridge_list and
dp->edid_cache here without setting them to NULL can lead to double-frees
or use-after-free issues if the component is rebound.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717072323.96-1-kernel@airkyi.com?part=5
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
From: sashiko-bot @ 2026-07-17 7:39 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: olteanv, linux-phy, dri-devel, vkoul, neil.armstrong
In-Reply-To: <20260717072323.96-4-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double-free and reference count underflow of the `child_np` device tree node reference due to conflicting ownership with `drm_aux_bridge_register_from_node()`.
--
--- Patch [3]: [PATCH v3 3/5] phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge ---
commit 7121c384753e095d5a040f2bd93021b7ae8110d7
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
phy: rockchip: phy-rockchip-typec: Add DRM AUX bridge
Using the DRM_AUX_BRIDGE helper to create the transparent DRM bridge
device.
> diff --git a/drivers/phy/rockchip/phy-rockchip-typec.c b/drivers/phy/rockchip/phy-rockchip-typec.c
> index d9701b6106d55..48070b50416e5 100644
> --- a/drivers/phy/rockchip/phy-rockchip-typec.c
> +++ b/drivers/phy/rockchip/phy-rockchip-typec.c
> @@ -1162,16 +1163,24 @@ static int rockchip_typec_phy_probe(struct platform_device *pdev)
>
> for_each_available_child_of_node(np, child_np) {
> struct phy *phy;
> + ret = 0;
>
> - if (of_node_name_eq(child_np, "dp-port"))
> + if (of_node_name_eq(child_np, "dp-port")) {
> phy = devm_phy_create(dev, child_np,
> &rockchip_dp_phy_ops);
> - else if (of_node_name_eq(child_np, "usb3-port"))
> + ret = drm_aux_bridge_register_from_node(dev, child_np);
> + } else if (of_node_name_eq(child_np, "usb3-port"))
> phy = devm_phy_create(dev, child_np,
> &rockchip_usb3_phy_ops);
> else
> continue;
>
> + if (ret) {
> + pm_runtime_disable(dev);
> + of_node_put(child_np);
[Severity: High]
Does this cause a reference count underflow for the child_np device node?
drm_aux_bridge_register_from_node() assigns the node to an auxiliary device,
and its error and release paths in drm/bridge/aux-bridge.c unconditionally
call of_node_put() to drop the reference.
However, the for_each_available_child_of_node() iterator owns the reference
to child_np during this loop iteration. Passing it to
drm_aux_bridge_register_from_node() without acquiring an additional reference
via of_node_get() appears to hand off a reference the bridge doesn't own.
If drm_aux_bridge_register_from_node() succeeds, the loop advances and drops
the iterator's reference, leading to a double-put when the bridge is
eventually released.
If drm_aux_bridge_register_from_node() fails, it drops the reference
internally. The explicit of_node_put(child_np) here in the error block would
then cause an immediate double-put.
> + return ret;
> + }
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717072323.96-1-kernel@airkyi.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon
From: sashiko-bot @ 2026-07-17 7:56 UTC (permalink / raw)
To: Chaoyi Chen; +Cc: olteanv, linux-phy, vkoul, dri-devel, neil.armstrong
In-Reply-To: <20260717072323.96-5-kernel@airkyi.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect error checking logic in `cdn_dp_probe` skips port initialization for valid extcon devices, breaking DisplayPort for legacy boards.
- [High] Removing the `extcon` notifier without providing an alternative mechanism completely breaks hotplug detection for Type-C connections without an extcon device.
--
--- Patch [4]: [PATCH v3 4/5] drm/rockchip: cdn-dp: Support handle lane info without extcon ---
commit 2789d7ccb0bb45f82d439538cf9aeaac1da9c5e9
Author: Chaoyi Chen <chaoyi.chen@rock-chips.com>
drm/rockchip: cdn-dp: Support handle lane info without extcon
This patch adds support for getting PHY lane info without the help of extcon.
There is no extcon needed if the Type-C controller is present. The extcon
device should still be supported if the Type-C controller is not present.
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 190cce9530c82..74e1f5422a6e2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
[ ... ]
> @@ -1027,6 +1033,9 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> for (i = 0; i < dp->ports; i++) {
> port = dp->port[i];
>
> + if (!port->extcon)
> + continue;
> +
[Severity: High]
Does skipping the notifier registration when extcon is absent leave the driver
without any hotplug detection mechanism?
The driver relies on the cdn_dp_pd_event notifier (registered via extcon) to
schedule dp->event_work, which probes the hardware, updates the connection
state, and calls drm_bridge_hpd_notify().
Without an alternative HPD mechanism when extcon is absent, won't hotplug
events be silently ignored, causing displays to only be detected if plugged in
during boot or resume?
> port->event_nb.notifier_call = cdn_dp_pd_event;
> ret = devm_extcon_register_notifier(dp->dev, port->extcon,
> EXTCON_DISP_DP,
[ ... ]
> @@ -1119,14 +1128,14 @@ static int cdn_dp_probe(struct platform_device *pdev)
> PTR_ERR(phy) == -EPROBE_DEFER)
> return -EPROBE_DEFER;
>
> - if (IS_ERR(extcon) || IS_ERR(phy))
> + if (IS_ERR(phy) || PTR_ERR(extcon) != -ENODEV)
> continue;
[Severity: High]
Does evaluating PTR_ERR(extcon) != -ENODEV without first checking if extcon
is an error pointer break initialization for valid extcon devices?
When a valid extcon device is successfully found, extcon is a valid kernel
pointer. Casting this pointer to long via PTR_ERR() yields a memory address
that is not equal to -ENODEV.
Since the condition incorrectly evaluates to true, won't the loop continue
and skip port allocation for all valid extcon devices, breaking DisplayPort
on existing platforms that use extcon?
>
> port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> if (!port)
> return -ENOMEM;
>
> - port->extcon = extcon;
> + port->extcon = IS_ERR(extcon) ? NULL : extcon;
> port->phy = phy;
> port->dp = dp;
> port->id = i;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717072323.96-1-kernel@airkyi.com?part=4
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v7 02/13] dt-bindings: clk: zte: Add zx297520v3 top clock and reset controller
From: Krzysztof Kozlowski @ 2026-07-17 8:15 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, Vinod Koul,
Neil Armstrong, Russell King, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, linux-phy
In-Reply-To: <20260717-berserk-faithful-cat-a34ad8@quoll>
On 17/07/2026 08:36, Krzysztof Kozlowski wrote:
> On Fri, Jul 17, 2026 at 12:35:38AM +0300, Stefan Dösinger wrote:
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index cb50c2e3b4ea..5f962b9947ab 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3878,10 +3878,13 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>> S: Odd fixes
>> F: Documentation/arch/arm/zte/
>> F: Documentation/devicetree/bindings/arm/zte.yaml
>> +F: Documentation/devicetree/bindings/clock/zte,zx297520v3-topcrm.yaml
>> F: Documentation/devicetree/bindings/phy/zte,zx297520v3-usb-phy.yaml
>> F: arch/arm/boot/dts/zte/
>> F: arch/arm/mach-zte/
>> +F: include/dt-bindings/clock/zte,zx297520v3-clk.h
>> F: include/dt-bindings/phy/phy-zte-zx297520v3-usb.h
>> +F: include/dt-bindings/reset/zte,zx297520v3-reset.h
>>
>> ARM/ZYNQ ARCHITECTURE
>> M: Michal Simek <michal.simek@amd.com>
>> diff --git a/include/dt-bindings/clock/zte,zx297520v3-clk.h b/include/dt-bindings/clock/zte,zx297520v3-clk.h
>
> Same mistake. Filename MUST match the compatible.
I did not notice that you combine here all the clock IDs from other
clock controllers. The header filename is therefore fine (also reset fine).
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v7 04/13] dt-bindings: clk: zte: Add zx297520v3 LSP clock and reset controller
From: Krzysztof Kozlowski @ 2026-07-17 8:23 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, Vinod Koul,
Neil Armstrong, Russell King, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, linux-phy
In-Reply-To: <20260717-zx29clk-v7-4-408411cfcf36@gmail.com>
On Fri, Jul 17, 2026 at 12:35:40AM +0300, Stefan Dösinger wrote:
> The clock controller of the zx297520v3 Low Speed Peripherals is
> relatively clean. One register per device with gates, muxes and resets
> and for some devices a divider. There are even bits in the matrix
> controller to control propagation of clock lines down to LSP.
>
> The clocks are sorted by register address and I am convinced that the
> device list is complete. There are however a few more registers that are
> likely extra dividers for TDM and I2S devices
>
> Signed-off-by: Stefan Dösinger <stefandoesinger@gmail.com>
>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v5 1/3] dt-bindings: phy: qcom: Add Glymur QMP PCIe multiple link-mode PHY
From: Qiang Yu @ 2026-07-17 9:58 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Manivannan Sadhasivam, Philipp Zabel,
Bjorn Andersson, Konrad Dybcio
Cc: Qiang Yu, linux-arm-msm, linux-phy, devicetree, linux-kernel
In-Reply-To: <20260717-glymur_linkmode_0717-v5-0-4f9e87a61463@oss.qualcomm.com>
Add qcom,glymur-qmp-pcie-multiphy.yaml as a standalone binding
for the Glymur Gen5 PCIe PHY hardware block. This block supports two
link modes, selected at runtime via a TCSR syscon register:
1. x8 - a single 8-lane PHY instance is exposed
2. x4+x4 - two independent 4-lane PHY instances are exposed
Keep this as a separate schema from qcom,sc8280xp-qmp-pcie-phy.yaml
rather than folding it into the shared compatible list there, since the
two PHY instances active in x8 mode require twice as many clocks,
resets, and power-domains as any other entry in that file, and adding
Glymur-specific properties like qcom,link-mode and reg-names there
would only apply to this one compatible.
Document the required clocks, resets, and power-domains for both PHY
instances, and use #phy-cells = <1>, where the cell value is the PHY
index within the active link mode.
Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
.../phy/qcom,glymur-qmp-pcie-multiphy.yaml | 176 +++++++++++++++++++++
1 file changed, 176 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,glymur-qmp-pcie-multiphy.yaml b/Documentation/devicetree/bindings/phy/qcom,glymur-qmp-pcie-multiphy.yaml
new file mode 100644
index 000000000000..73550f77e9b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,glymur-qmp-pcie-multiphy.yaml
@@ -0,0 +1,176 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,glymur-qmp-pcie-multiphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm QMP PCIe multiple link-mode PHY controller (PCIe, Glymur)
+
+maintainers:
+ - Qiang Yu <qiang.yu@oss.qualcomm.com>
+ - Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
+
+description: |
+ The Glymur SoC uses a single PCIe Gen5 PHY hardware block for the
+ PCIe3a/PCIe3b controllers. This block supports two link modes, selected
+ at runtime via a TCSR syscon register:
+
+ 1. x8 - a single 8-lane PHY instance is exposed (PCIe3a only)
+ 2. x4+x4 - two independent 4-lane PHY instances are exposed (PCIe3a and
+ PCIe3b)
+
+ The node always describes both PHY instances ("port a" and "port b"),
+ regardless of which link mode is active on the board.
+
+properties:
+ compatible:
+ enum:
+ - qcom,glymur-qmp-gen5x8-pcie-phy
+
+ reg:
+ minItems: 2
+ maxItems: 2
+
+ reg-names:
+ items:
+ - const: port_a
+ - const: port_b
+
+ clocks:
+ minItems: 10
+ maxItems: 10
+
+ clock-names:
+ items:
+ - const: aux
+ - const: cfg_ahb
+ - const: ref
+ - const: rchng
+ - const: pipe
+ - const: phy_b_aux
+ - const: cfg_ahb_b
+ - const: rchng_b
+ - const: pipe_b
+ - const: pipediv2_b
+
+ power-domains:
+ minItems: 2
+ maxItems: 2
+
+ power-domain-names:
+ items:
+ - const: phy_a_gdsc
+ - const: phy_b_gdsc
+
+ resets:
+ minItems: 4
+ maxItems: 4
+
+ reset-names:
+ items:
+ - const: phy_a
+ - const: phy_a_nocsr
+ - const: phy_b
+ - const: phy_b_nocsr
+
+ vdda-phy-supply: true
+
+ vdda-pll-supply: true
+
+ vdda-refgen0p9-supply: true
+
+ vdda-refgen1p2-supply: true
+
+ qcom,link-mode:
+ description:
+ Reference to a register in the TCSR syscon that reports the link
+ mode the PCIe PHY is currently configured for, either a single x8
+ link or two independent x4 links. The link mode is programmed by
+ firmware before Linux boots; this property is only used to read
+ the active link mode, specified as a phandle to the syscon and
+ the register offset.
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ - items:
+ - description: phandle of TCSR syscon
+ - description: offset of link mode register
+
+ "#clock-cells":
+ const: 1
+
+ clock-output-names:
+ items:
+ - description: Name of the PHY A pipe clock output.
+ - description: Name of the PHY B pipe clock output.
+
+ "#phy-cells":
+ const: 1
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+ - power-domains
+ - power-domain-names
+ - resets
+ - reset-names
+ - vdda-phy-supply
+ - vdda-pll-supply
+ - vdda-refgen0p9-supply
+ - vdda-refgen1p2-supply
+ - qcom,link-mode
+ - "#clock-cells"
+ - clock-output-names
+ - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,glymur-gcc.h>
+ #include <dt-bindings/clock/qcom,glymur-tcsr.h>
+
+ pcie3_phy: phy@f00000 {
+ compatible = "qcom,glymur-qmp-gen5x8-pcie-phy";
+ reg = <0x00f00000 0x10000>, <0x00f10000 0x10000>;
+ reg-names = "port_a", "port_b";
+
+ clocks = <&gcc GCC_PCIE_PHY_3A_AUX_CLK>,
+ <&gcc GCC_PCIE_3A_CFG_AHB_CLK>,
+ <&tcsr TCSR_PCIE_3_CLKREF_EN>,
+ <&gcc GCC_PCIE_3A_PHY_RCHNG_CLK>,
+ <&gcc GCC_PCIE_3A_PIPE_CLK>,
+ <&gcc GCC_PCIE_PHY_3B_AUX_CLK>,
+ <&gcc GCC_PCIE_3B_CFG_AHB_CLK>,
+ <&gcc GCC_PCIE_3B_PHY_RCHNG_CLK>,
+ <&gcc GCC_PCIE_3B_PIPE_CLK>,
+ <&gcc GCC_PCIE_3B_PIPE_DIV2_CLK>;
+ clock-names = "aux", "cfg_ahb", "ref", "rchng", "pipe",
+ "phy_b_aux", "cfg_ahb_b", "rchng_b", "pipe_b",
+ "pipediv2_b";
+
+ resets = <&gcc GCC_PCIE_3A_PHY_BCR>,
+ <&gcc GCC_PCIE_3A_NOCSR_COM_PHY_BCR>,
+ <&gcc GCC_PCIE_3B_PHY_BCR>,
+ <&gcc GCC_PCIE_3B_NOCSR_COM_PHY_BCR>;
+ reset-names = "phy_a", "phy_a_nocsr", "phy_b", "phy_b_nocsr";
+
+ power-domains = <&gcc GCC_PCIE_3A_PHY_GDSC>,
+ <&gcc GCC_PCIE_3B_PHY_GDSC>;
+ power-domain-names = "phy_a_gdsc", "phy_b_gdsc";
+
+ vdda-phy-supply = <&vreg_l3c>;
+ vdda-pll-supply = <&vreg_l2c>;
+
+ vdda-refgen0p9-supply = <&vreg_l3c>;
+ vdda-refgen1p2-supply = <&vreg_l2c>;
+
+ qcom,link-mode = <&tcsr 0x5000>;
+
+ #clock-cells = <1>;
+ clock-output-names = "pcie3a_pipe_clk", "pcie3b_pipe_clk";
+
+ #phy-cells = <1>;
+ };
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 0/3] Add QMP PCIe multiple link-mode PHY support
From: Qiang Yu @ 2026-07-17 9:58 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Manivannan Sadhasivam, Philipp Zabel,
Bjorn Andersson, Konrad Dybcio
Cc: Qiang Yu, linux-arm-msm, linux-phy, devicetree, linux-kernel
Hi all,
Some QMP PCIe PHY hardware blocks can be configured through a link-mode
register into different link topologies, such as a single wide link, or
multiple independent narrower links (e.g. x8 or x4+x4 mode on the Glymur
PCIe3 PHY).
Earlier revisions tried to extend the existing single-instance
phy-qcom-qmp-pcie.c driver to cover this hardware. That added a large
amount of conditional, multi-PHY logic to a driver whose data model
assumes one PHY per node.
This series adds a dedicated PHY provider driver,
phy-qcom-qmp-pcie-multiphy.c:
A single PHY provider node describes the shared hardware block. A new
"qcom,link-mode" property points at the TCSR syscon register that reports
the active topology; the driver reads it once at probe. #phy-cells = <1>
lets consumers pass a logical PHY index to obtain their sub-PHY. Match
data is indexed by link mode, and each link mode has its own array of
per-PHY config tables, so one shared provider exposes a different set of
logical PHYs depending on the active mode. The driver inherits the PHY
settings and link mode already programmed by UEFI, so only the no-CSR
reset is used, and no PHY setting tables or related structures are
defined. Each sub-PHY owns its register regions and power domain, so in a
bifurcated mode the links are brought up, and powered independently.
The driver is implemented and validated on Glymur, and is intended to
be extensible to other multi-mode QMP PCIe PHYs.
This series depends on a prerequisite patch by Krzysztof Kozlowski:
https://lore.kernel.org/r/20260420133616.88740-2-krzysztof.kozlowski@oss.qualcomm.com
Thanks,
Qiang
Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
Changes in v5:
- Split the Glymur multi PHY binding out of the shared qcom,sc8280xp-qmp-pcie-phy.yaml into its own schema.
- Reworked phy-qcom-qmp-pcie-multiphy.c into a standalone driver instead of extending phy-qcom-qmp-pcie.c with conditional multi-PHYlogic.
- Dropped the "link mode value" cell from qcom,link-mode.
- Removed the QMP_PHY_SELECTOR_0/1 dt-bindings macros.
- Link to v4: https://lore.kernel.org/all/20260518-link_mode_0519-v4-0-269cd73cc5d1@oss.qualcomm.com/
Changes in v4:
- Replaced the static bifurcation probe model with a link-mode
architecture: the active topology (x8 or x4+x4) is selected via a
new "qcom,link-mode" DT property and written to a TCSR register at
power-on. This replaces v3 patches 4 and 5 with four new patches
(driver refactor, secondary PHY clock/reset lists, link-mode probe
infrastructure, and Glymur config).
- Updated dt-bindings to describe "qcom,link-mode", #phy-cells = <1>,
per-mode validation rules, and added a new header with
QMP_PHY_SELECTOR_* and QMP_PCIE_GLYMUR_MODE_* macros.
- Patches 2 and 3 (multiple power-domains, multiple nocsr resets) are
unchanged from v3.
- Link to v3: https://lore.kernel.org/r/20260412-glymur_gen5x8_phy_0413-v3-0-affcebc16b8b@oss.qualcomm.com
Changes in v3:
- Add description of each power-domain.
- Add 64bit prefetchable memory range required by some EPs eg. AI100 ultra.
- Move PCIe3a after PCIe3b and move PCIe3a PHY before PCIe3b PHY.
- Link to v2: https://lore.kernel.org/all/20260323-glymur_gen5x8_phy_0323-v2-0-ce0fc07f0e52@oss.qualcomm.com/
Changes in v2:
- Remove pd_list from qmp_pcie struct as it is not used in phy driver.
- align clk-names on "
- Link to v1: https://lore.kernel.org/all/20260304-glymur_gen5x8_phy-v1-0-849e9a72e125@oss.qualcomm.com/
To: Vinod Koul <vkoul@kernel.org>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Qiang Yu <qiang.yu@oss.qualcomm.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
To: Philipp Zabel <p.zabel@pengutronix.de>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Qiang Yu (3):
dt-bindings: phy: qcom: Add Glymur QMP PCIe multiple link-mode PHY
phy: qcom: qmp-pcie: Add QMP PCIe Multi-PHY driver
arm64: dts: qcom: glymur: Wire PCIe3a/3b to shared Gen5x8 PHY
.../phy/qcom,glymur-qmp-pcie-multiphy.yaml | 176 +++++
arch/arm64/boot/dts/qcom/glymur-crd.dtsi | 8 +
arch/arm64/boot/dts/qcom/glymur.dtsi | 336 ++++++++-
drivers/phy/qualcomm/Kconfig | 11 +
drivers/phy/qualcomm/Makefile | 1 +
drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c | 770 +++++++++++++++++++++
6 files changed, 1300 insertions(+), 2 deletions(-)
---
base-commit: 59ccadbf59aedee486fce5b4a5486a07c3f4b5d3
change-id: 20260716-glymur_linkmode_0717-70eacf08a5c5
Best regards,
--
Qiang Yu <qiang.yu@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox