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

[Krzysztof Kozlowski <krzk@kernel.org>]

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.

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.

Best regards,
Krzysztof