Re: [PATCH v3 1/5] dt-bindings: phy: qcom,sc8280xp-qmp-pcie-phy: Add support for glymur Gen5 x8 bifurcation mode

[Qiang Yu <qiang.yu@oss.qualcomm.com>]

On Mon, Apr 20, 2026 at 03:23:43PM +0200, Krzysztof Kozlowski wrote:
> On Mon, Apr 20, 2026 at 12:23:19AM -0700, Qiang Yu wrote:
> > On Fri, Apr 17, 2026 at 11:18:08AM +0200, Krzysztof Kozlowski wrote:
> > > On Wed, Apr 15, 2026 at 07:58:13PM -0700, Qiang Yu wrote:
> > > > On Wed, Apr 15, 2026 at 09:50:28AM +0200, Krzysztof Kozlowski wrote:
> > > > > On Sun, Apr 12, 2026 at 11:25:56PM -0700, Qiang Yu wrote:
> > > > > > The Glymur SoC has pcie3a and pcie3b PHYs that can operate in two modes:
> > > > > > 
> > > > > > 1. Independent 4-lane mode: Each PHY operates as a separate PCIe Gen5
> > > > > >    4-lane interface, compatible with qcom,glymur-qmp-gen5x4-pcie-phy
> > > > > > 2. Bifurcation mode (8-lane): pcie3a phy acts as leader and pcie3b phy as
> > > > > >    follower to form a single 8-lane PCIe Gen5 interface
> > > > > > 
> > > > > > In bifurcation mode, the hardware design requires controlling additional
> > > > > > resources beyond the standard pcie3a PHY configuration:
> > > > > > 
> > > > > > - pcie3b's aux_clk (phy_b_aux)
> > > > > > - pcie3b's phy_gdsc power domain
> > > > > > - pcie3b's bcr/nocsr reset
> > > > > > 
> > > > > > Add qcom,glymur-qmp-gen5x8-pcie-phy compatible string to document this
> > > > > > 8-lane bifurcation configuration.
> > > > > 
> > > > > Do you describe PCI3A or PCI3B or something combined PCI3?
> > > > 
> > > > I describe a single x8 PHY with resources from both the pcie3a and pcie3b
> > > > PHY blocks for x8 operation.
> > > > 
> > > > > 
> > > > > > 
> > > > > > The phy_b_aux clock is used as the 6th clock instead of pipediv2,
> > > > > > requiring the clock-names enum to be extended to support both
> > > > > > [phy_b_aux, pipediv2] options at index 5. This follows the existing
> > > > > > pattern used for [rchng, refgen] clocks at index 3.
> > > > > > 
> > > > > > Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> > > > > > ---
> > > > > >  .../bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml   | 53 ++++++++++++++++++----
> > > > > >  1 file changed, 45 insertions(+), 8 deletions(-)
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > > > > > index 3a35120a77ec0ceb814a1cdcacff32fef32b4f7b..14eba5d705b1956c1bb00cc8c95171ed6488299b 100644
> > > > > > --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-pcie-phy.yaml
> > > > > > @@ -18,6 +18,7 @@ properties:
> > > > > >      enum:
> > > > > >        - qcom,glymur-qmp-gen4x2-pcie-phy
> > > > > >        - qcom,glymur-qmp-gen5x4-pcie-phy
> > > > > > +      - qcom,glymur-qmp-gen5x8-pcie-phy
> > > > > 
> > > > > That's the same device as 5x4, no? One device, one compatible and this
> > > > > suggests you will have three PCI phys in the DT - two 5x4 and one 5x8?
> > > > > 
> > > > 
> > > > It is not the same as the 5x4 PHY. In DT, we model three PHY nodes:
> > > > phy_3a (1x4), phy_3b (1x4), and a separate phy_1x8 node for x8 mode.
> > > 
> > > OK, that's what I wanted to hear. And that's what should not be done,
> > > 
> > > You should not have a separate node for the same hardware. First, DTC
> > > will give you a W=1 warning, although warning itself should be moved to
> > > W=2.
> > > 
> > > Second, the warning tells important story - same hardware is described
> > > twice.
> > > 
> > > You only need phy_3a and phy_3b, so only two in total.
> > 
> > We can keep only phy_3a and phy_3b, but still add new compatible
> > qcom,glymur-qmp-gen5x8-pcie-phy in binding, right?
> > 
> > For boards that support pcie3a(1x4) + pcie3b(1x4), DTS would be:
> > 
> > pcie3a_phy { compatible = "qcom,glymur-qmp-gen5x4-pcie-phy"; };
> > pcie3b_phy { compatible = "qcom,glymur-qmp-gen5x4-pcie-phy"; };
> > 
> > For boards that support 1x8, we would override pcie3a_phy with:
> > 
> > pcie3a_phy { compatible = "qcom,glymur-qmp-gen5x8-pcie-phy"; /* additional resources */ };
> > pcie3b_phy { compatible = "qcom,glymur-qmp-gen5x4-pcie-phy"; };
> > 
> > This still uses only two PHY nodes and DTC will not report warning.
> 
> IMO, you do not need another compatible. Device is exactly the same. If
> wiring on the board differs, e.g. you have 8x instead of 4x, you:
> 1. disable unused 3B phy.
> 2. Add to 3A missing resources or the phandle to companion node.
> 
> At least that is what I tought till now, when I opened the HPG/manual
> for Glymur phy.  Someone skipped important information when PCIe PHY was
> upstreamed first and glymur.dtsi already got PHY 3B described.
> 
> Reminder: writing bindings asks you explicitly to post COMPLETE
> bindings.
> 
> If you posted COMPLETE bindings we would question all this and you would
> have to check in user manual that this is actually ONE device.
> 
> There is no 5x4 phy 3A and 3B, at least HPG is pretty clear here.

PHY_A and PHY_B are two sub PHYs that can act independently. I thought we
can describe them. And for previous target eg Hamoa, we also descibed like
this.

> 
> And you should start with that.
> 
> But you posted first incomplete binding, hiding the rest and now you
> have 5x4 merged into DTSI.
> 
> So let's rephrase based on manual:
> You have only one PCIE phy3. Not 3A + 3B. That one phy3 can be
> configured by consumers (board) differently, e.g. by requesting 8-lane
> or twice 4-lane phys.
> 
> Let me send correction note for glymur.dtsi.
> 

So we can have only one compatible "qcom,glymur-qmp-gen5x8-pcie-phy" and
one phy dts node?

- Qiang Yu

> Best regards,
> Krzysztof
>