Re: [PATCH 0/5] phy: qcom: qmp-pcie: Add PCIe Gen5 8-lane bifurcation support for Glymur

[Neil Armstrong <neil.armstrong@linaro.org>]

On 3/6/26 10:26, Qiang Yu wrote:
> On Thu, Mar 05, 2026 at 10:14:05AM +0100, Konrad Dybcio wrote:
>> On 3/4/26 9:21 AM, Qiang Yu wrote:
>>> This patch series adds support for PCIe Gen5 8-lane bifurcation mode on
>>> the Glymur SoC's third PCIe controller. In this configuration, pcie3a PHY
>>> acts as leader and pcie3b PHY as follower to form a single 8-lane PCIe
>>> Gen5 interface.
>>>
>>> To support 8-lanes mode, this patch series add multiple power domain and
>>> multi nocsr reset infrastructure as the hardware programming guide
>>> specifies a strict initialization sequence for bifurcation mode that
>>> requires coordinated multi-PHY resource management:
>>>
>>> 1. Turn on both pcie3a_phy_gdsc and pcie3b_phy_gdsc power domains
>>> 2. Assert both pcie3a and pcie3b nocsr resets, then deassert them together
>>> 3. Enable all pcie3a PHY clocks and pcie3b PHY aux clock (bifur_aux)
>>> 4. Poll for PHY ready status
>>
>> I think we never concluded the discussion where I suggested the
>> bifurcated PHY may be better expressed as a single node with
>> #phy-cells = <1>, removing the need for duplicated resource references

DT requires strict hardware description, no abstraction for HW, so if there's
2 PHYs, then add 2 separate phys and reference them from the PCie controller.

On platforms where you want 2x4, then add 2 pcie_ports using 2 phys, on platforms
with 1x8 a single pcie_port with 2 phys.

Neil

>>
> I understand your suggestion would look like below. I agree that the
> unified PHY approach being more elegant from a device tree perspective,
> provide better DT flexibility and eliminate the need for different
> compatibles and dupicated resources between 1x8 and 2x4 modes.
> 
> However, this will include implementation complexity to phy driver.
> The driver would need conditional logic to selectively enable different
> clocks/resets based on the PHY parameter and maintain mode-specific
> resource arrays. There's also the issue that assigned-clocks
> GCC_PCIE_3A_PHY_RCHNG_CLK and GCC_PCIE_3B_PHY_RCHNG_CLK will be set before
> probe no matter which mode is used, even though in 1x8 mode or only one of
> them is actually needed. For pipe clock outputs, only pcie3a_pipe_clk would
> be needed in 1x8 mode while pcie3b_pipe_clk would be unused. For
> powerdomain, we also need to add additional logic to attach and turn
> on/off them.
> 
> While these challenges could be resolved, I'm not sure the benefits
> justify the added complexity.
> 
> pcie3_unified_phy {
>      compatible = "qcom,glymur-qmp-gen5-pcie-phy";
>      reg = <0 0x00f00000 0 0x10000>, <0 0x00f10000 0 0x10000>;  /* Both PHY ranges */
> 
>      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>;
> 
>      power-domains = <&gcc GCC_PCIE_3A_PHY_GDSC>,
>                      <&gcc GCC_PCIE_3B_PHY_GDSC>;
> 
>      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>;
> 
> 	#clock-cells = <1>;
>      clock-output-names = "pcie3a_pipe_clk", "pcie3b_pipe_clk";
>      assigned-clocks = <&gcc GCC_PCIE_3A_PHY_RCHNG_CLK>,
>                        <&gcc GCC_PCIE_3B_PHY_RCHNG_CLK>;
>      assigned-clock-rates = <100000000>, <100000000>;
> 
>      #phy-cells = <1>;  /* Parameter: 0=PHY_A, 1=PHY_B, 2=UNIFIED_8LANE */
> };
> 
> For 2x4 mode (independent 4-lane PHYs):
> &pcie3a {
>      phys = <&pcie3_unified_phy PHY_A>;  /* PHY A only */
>      status = "okay";
> };
> 
> &pcie3b {
>      phys = <&pcie3_unified_phy PHY_B>;  /* PHY B only */
>      status = "okay";
> };
> 
> For 1x8 mode (unified 8-lane PHY):
> 
> &pcie3a {
>      phys = <&pcie3_unified_phy PHY_AB>;
>      num-lanes = <8>;
>      status = "okay";
> };
> 
> &pcie3b {
>      status = "disabled";
> };
> 
> - Qiang Yu