According to documentation, the DP PHY on x1e80100 has another clock called refclk. Rework the driver to allow different number of clocks. Fix the dt-bindings schema and add the clock to the DT node as well.
Signed-off-by: Abel Vesa abel.vesa@linaro.org --- Changes in v2: - Fix schema by adding the minItems, as suggested by Krzysztof. - Use devm_clk_bulk_get_all, as suggested by Konrad. - Rephrase the commit messages to reflect the flexible number of clocks. - Link to v1: https://lore.kernel.org/r/20250730-phy-qcom-edp-add-missing-refclk-v1-0-6f78...
--- Abel Vesa (3): dt-bindings: phy: qcom-edp: Add missing clock for X Elite phy: qcom: edp: Make the number of clocks flexible arm64: dts: qcom: Add missing TCSR refclk to the DP PHYs
.../devicetree/bindings/phy/qcom,edp-phy.yaml | 28 +++++++++++++++++++++- arch/arm64/boot/dts/qcom/x1e80100.dtsi | 12 ++++++---- drivers/phy/qualcomm/phy-qcom-edp.c | 18 +++++++------- 3 files changed, 45 insertions(+), 13 deletions(-) --- base-commit: 5d50cf9f7cf20a17ac469c20a2e07c29c1f6aab7 change-id: 20250730-phy-qcom-edp-add-missing-refclk-5ab82828f8e7
Best regards,
On X Elite platform, the eDP PHY uses one more clock called refclk. Add it to the schema.
Cc: stable@vger.kernel.org # v6.10 Fixes: 5d5607861350 ("dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles") Signed-off-by: Abel Vesa abel.vesa@linaro.org --- .../devicetree/bindings/phy/qcom,edp-phy.yaml | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml index eb97181cbb9579893b4ee26a39c3559ad87b2fba..a8ba0aa9ff9d83f317bd897a7d564f7e13f6a1e2 100644 --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml @@ -37,12 +37,15 @@ properties: - description: PLL register block
clocks: - maxItems: 2 + minItems: 2 + maxItems: 3
clock-names: + minItems: 2 items: - const: aux - const: cfg_ahb + - const: refclk
"#clock-cells": const: 1 @@ -64,6 +67,29 @@ required: - "#clock-cells" - "#phy-cells"
+allOf: + - if: + properties: + compatible: + enum: + - qcom,x1e80100-dp-phy + then: + properties: + clocks: + minItems: 3 + maxItems: 3 + clock-names: + minItems: 3 + maxItems: 3 + else: + properties: + clocks: + minItems: 2 + maxItems: 2 + clock-names: + minItems: 2 + maxItems: 2 + additionalProperties: false
examples:
On 9/3/25 2:37 PM, Abel Vesa wrote:
On X Elite platform, the eDP PHY uses one more clock called refclk. Add it to the schema.
Cc: stable@vger.kernel.org # v6.10 Fixes: 5d5607861350 ("dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles") Signed-off-by: Abel Vesa abel.vesa@linaro.org
.../devicetree/bindings/phy/qcom,edp-phy.yaml | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml index eb97181cbb9579893b4ee26a39c3559ad87b2fba..a8ba0aa9ff9d83f317bd897a7d564f7e13f6a1e2 100644 --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml @@ -37,12 +37,15 @@ properties: - description: PLL register block clocks:
- maxItems: 2
- minItems: 2
- maxItems: 3
clock-names:
- minItems: 2 items:
- const: aux
- const: cfg_ahb
- const: refclk
"ref"?
Konrad
On Wed, Sep 03, 2025 at 03:37:25PM +0200, Konrad Dybcio wrote:
On 9/3/25 2:37 PM, Abel Vesa wrote:
On X Elite platform, the eDP PHY uses one more clock called refclk. Add it to the schema.
Cc: stable@vger.kernel.org # v6.10 Fixes: 5d5607861350 ("dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles") Signed-off-by: Abel Vesa abel.vesa@linaro.org
.../devicetree/bindings/phy/qcom,edp-phy.yaml | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml index eb97181cbb9579893b4ee26a39c3559ad87b2fba..a8ba0aa9ff9d83f317bd897a7d564f7e13f6a1e2 100644 --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml @@ -37,12 +37,15 @@ properties: - description: PLL register block clocks:
- maxItems: 2
- minItems: 2
- maxItems: 3
clock-names:
- minItems: 2 items:
- const: aux
- const: cfg_ahb
- const: refclk"ref"?
Certainly more consistent with other QCom phy bindings.
On 9/4/25 1:51 AM, Rob Herring wrote:
On Wed, Sep 03, 2025 at 03:37:25PM +0200, Konrad Dybcio wrote:
On 9/3/25 2:37 PM, Abel Vesa wrote:
On X Elite platform, the eDP PHY uses one more clock called refclk. Add it to the schema.
Cc: stable@vger.kernel.org # v6.10 Fixes: 5d5607861350 ("dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles") Signed-off-by: Abel Vesa abel.vesa@linaro.org
.../devicetree/bindings/phy/qcom,edp-phy.yaml | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml index eb97181cbb9579893b4ee26a39c3559ad87b2fba..a8ba0aa9ff9d83f317bd897a7d564f7e13f6a1e2 100644 --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml @@ -37,12 +37,15 @@ properties: - description: PLL register block clocks:
- maxItems: 2
- minItems: 2
- maxItems: 3
clock-names:
- minItems: 2 items:
- const: aux
- const: cfg_ahb
- const: refclk"ref"?
Certainly more consistent with other QCom phy bindings.
That, and the name of a clock-names entry ending in 'clk' is simply superfluous
Konrad
On 25-09-04 10:11:26, Konrad Dybcio wrote:
On 9/4/25 1:51 AM, Rob Herring wrote:
On Wed, Sep 03, 2025 at 03:37:25PM +0200, Konrad Dybcio wrote:
On 9/3/25 2:37 PM, Abel Vesa wrote:
On X Elite platform, the eDP PHY uses one more clock called refclk. Add it to the schema.
Cc: stable@vger.kernel.org # v6.10 Fixes: 5d5607861350 ("dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles") Signed-off-by: Abel Vesa abel.vesa@linaro.org
.../devicetree/bindings/phy/qcom,edp-phy.yaml | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml index eb97181cbb9579893b4ee26a39c3559ad87b2fba..a8ba0aa9ff9d83f317bd897a7d564f7e13f6a1e2 100644 --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml @@ -37,12 +37,15 @@ properties: - description: PLL register block clocks:
- maxItems: 2
- minItems: 2
- maxItems: 3
clock-names:
- minItems: 2 items:
- const: aux
- const: cfg_ahb
- const: refclk"ref"?
Certainly more consistent with other QCom phy bindings.
That, and the name of a clock-names entry ending in 'clk' is simply superfluous
Yep. Will fix in v3.
Thanks for reviewing.
On 03/09/2025 14:37, Abel Vesa wrote:
On X Elite platform, the eDP PHY uses one more clock called refclk. Add it to the schema.
Cc: stable@vger.kernel.org # v6.10 Fixes: 5d5607861350 ("dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles") Signed-off-by: Abel Vesa abel.vesa@linaro.org
.../devicetree/bindings/phy/qcom,edp-phy.yaml | 28 +++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml index eb97181cbb9579893b4ee26a39c3559ad87b2fba..a8ba0aa9ff9d83f317bd897a7d564f7e13f6a1e2 100644 --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml @@ -37,12 +37,15 @@ properties: - description: PLL register block clocks:
- maxItems: 2
- minItems: 2
- maxItems: 3
clock-names:
- minItems: 2 items:
- const: aux
- const: cfg_ahb
- const: refclk
Name is: "ref"
"#clock-cells": const: 1 @@ -64,6 +67,29 @@ required:
- "#clock-cells"
- "#phy-cells"
+allOf:
- if:
properties:compatible:enum:- qcom,x1e80100-dp-phy- then:
properties:clocks:minItems: 3
That's an ABI break, so you need to explain it and mention the impact. Reason that there is one more clock, but everything was working fine, is not usually enough.
Best regards, Krzysztof
On 04/09/2025 08:50, Krzysztof Kozlowski wrote:
+allOf:
- if:
properties:compatible:enum:- qcom,x1e80100-dp-phy- then:
properties:clocks:minItems: 3That's an ABI break, so you need to explain it and mention the impact. Reason that there is one more clock, but everything was working fine, is not usually enough.
Heh, I already asked for that at v1 and nothing improved.
Best regards, Krzysztof
On 25-09-04 08:51:49, Krzysztof Kozlowski wrote:
On 04/09/2025 08:50, Krzysztof Kozlowski wrote:
+allOf:
- if:
properties:compatible:enum:- qcom,x1e80100-dp-phy- then:
properties:clocks:minItems: 3That's an ABI break, so you need to explain it and mention the impact. Reason that there is one more clock, but everything was working fine, is not usually enough.
Heh, I already asked for that at v1 and nothing improved.
I missed that comment. Sorry about that.
Will address in v3.
Thanks for reviewing.
On X Elite, the DP PHY needs another clock called refclk, while all other platforms do not. So get all the clocks regardless of how many there are provided.
Cc: stable@vger.kernel.org # v6.10 Fixes: db83c107dc29 ("phy: qcom: edp: Add v6 specific ops and X1E80100 platform support") Signed-off-by: Abel Vesa abel.vesa@linaro.org --- drivers/phy/qualcomm/phy-qcom-edp.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c index f1b51018683d51df064f60440864c6031638670c..116b7f7b4f8be93e5128c3cc6f382ce7576accbc 100644 --- a/drivers/phy/qualcomm/phy-qcom-edp.c +++ b/drivers/phy/qualcomm/phy-qcom-edp.c @@ -103,7 +103,9 @@ struct qcom_edp {
struct phy_configure_opts_dp dp_opts;
- struct clk_bulk_data clks[2]; + struct clk_bulk_data *clks; + int num_clks; + struct regulator_bulk_data supplies[2];
bool is_edp; @@ -218,7 +220,7 @@ static int qcom_edp_phy_init(struct phy *phy) if (ret) return ret;
- ret = clk_bulk_prepare_enable(ARRAY_SIZE(edp->clks), edp->clks); + ret = clk_bulk_prepare_enable(edp->num_clks, edp->clks); if (ret) goto out_disable_supplies;
@@ -885,7 +887,7 @@ static int qcom_edp_phy_exit(struct phy *phy) { struct qcom_edp *edp = phy_get_drvdata(phy);
- clk_bulk_disable_unprepare(ARRAY_SIZE(edp->clks), edp->clks); + clk_bulk_disable_unprepare(edp->num_clks, edp->clks); regulator_bulk_disable(ARRAY_SIZE(edp->supplies), edp->supplies);
return 0; @@ -1092,11 +1094,11 @@ static int qcom_edp_phy_probe(struct platform_device *pdev) if (IS_ERR(edp->pll)) return PTR_ERR(edp->pll);
- edp->clks[0].id = "aux"; - edp->clks[1].id = "cfg_ahb"; - ret = devm_clk_bulk_get(dev, ARRAY_SIZE(edp->clks), edp->clks); - if (ret) - return ret; + edp->num_clks = devm_clk_bulk_get_all(dev, &edp->clks); + if (edp->num_clks < 0) { + dev_err(dev, "Failed to get clocks\n"); + return edp->num_clks; + }
edp->supplies[0].supply = "vdda-phy"; edp->supplies[1].supply = "vdda-pll";
On 9/3/25 2:37 PM, Abel Vesa wrote:
On X Elite, the DP PHY needs another clock called refclk, while all other platforms do not. So get all the clocks regardless of how many there are provided.
Cc: stable@vger.kernel.org # v6.10 Fixes: db83c107dc29 ("phy: qcom: edp: Add v6 specific ops and X1E80100 platform support") Signed-off-by: Abel Vesa abel.vesa@linaro.org
[...]
@@ -1092,11 +1094,11 @@ static int qcom_edp_phy_probe(struct platform_device *pdev) if (IS_ERR(edp->pll)) return PTR_ERR(edp->pll);
- edp->clks[0].id = "aux";
- edp->clks[1].id = "cfg_ahb";
- ret = devm_clk_bulk_get(dev, ARRAY_SIZE(edp->clks), edp->clks);
- if (ret)
return ret;
- edp->num_clks = devm_clk_bulk_get_all(dev, &edp->clks);
- if (edp->num_clks < 0) {
dev_err(dev, "Failed to get clocks\n");return edp->num_clks;
return dev_err_probe(), also please print the ret code
Konrad
The DP PHYs on X1E80100 need the refclk which is provided by the TCSR CC. So add it to the PHYs.
Cc: stable@vger.kernel.org # v6.9 Fixes: 1940c25eaa63 ("arm64: dts: qcom: x1e80100: Add display nodes") Signed-off-by: Abel Vesa abel.vesa@linaro.org --- arch/arm64/boot/dts/qcom/x1e80100.dtsi | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi index 737c5dbd1c808300041cc8897ca1f7450e16e019..495356a7ebe662c68385a19ee0657033e44e0c7a 100644 --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi @@ -5670,9 +5670,11 @@ mdss_dp2_phy: phy@aec2a00 { <0 0x0aec2000 0 0x1c8>;
clocks = <&dispcc DISP_CC_MDSS_DPTX2_AUX_CLK>, - <&dispcc DISP_CC_MDSS_AHB_CLK>; + <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&tcsr TCSR_EDP_CLKREF_EN>; clock-names = "aux", - "cfg_ahb"; + "cfg_ahb", + "refclk";
power-domains = <&rpmhpd RPMHPD_MX>;
@@ -5690,9 +5692,11 @@ mdss_dp3_phy: phy@aec5a00 { <0 0x0aec5000 0 0x1c8>;
clocks = <&dispcc DISP_CC_MDSS_DPTX3_AUX_CLK>, - <&dispcc DISP_CC_MDSS_AHB_CLK>; + <&dispcc DISP_CC_MDSS_AHB_CLK>, + <&tcsr TCSR_EDP_CLKREF_EN>; clock-names = "aux", - "cfg_ahb"; + "cfg_ahb", + "refclk";
power-domains = <&rpmhpd RPMHPD_MX>;
On 9/3/25 2:37 PM, Abel Vesa wrote:
The DP PHYs on X1E80100 need the refclk which is provided by the TCSR CC. So add it to the PHYs.
Cc: stable@vger.kernel.org # v6.9
You want to backport this to 6.9, but you also want to backport the driver patch to 6.10, "meh"
I'm not sure it makes sense to backport functionally, as this would only exhibit issues if:
a) the UEFI did no work to enable the refclk or: b) unused cleanup would happen
but the board would not survive booting with b) in v6.9, at least it wouldn't have display - see Commit b60521eff227 ("clk: qcom: gcc-x1e80100: Unregister GCC_GPU_CFG_AHB_CLK/GCC_DISP_XO_CLK")
and a) is not something we'd hit on any of the upstream-supported targets
Konrad
On 25-09-04 10:40:36, Konrad Dybcio wrote:
On 9/3/25 2:37 PM, Abel Vesa wrote:
The DP PHYs on X1E80100 need the refclk which is provided by the TCSR CC. So add it to the PHYs.
Cc: stable@vger.kernel.org # v6.9
You want to backport this to 6.9, but you also want to backport the driver patch to 6.10, "meh"
I'm not sure it makes sense to backport functionally, as this would only exhibit issues if:
a) the UEFI did no work to enable the refclk or: b) unused cleanup would happen
but the board would not survive booting with b) in v6.9, at least it wouldn't have display - see Commit b60521eff227 ("clk: qcom: gcc-x1e80100: Unregister GCC_GPU_CFG_AHB_CLK/GCC_DISP_XO_CLK")
and a) is not something we'd hit on any of the upstream-supported targets
You are correct.
However, HW-wise, this clock is there and is needed, regardless if UEFI leaves it enabled or not. So it makes sense to go all the way back to 6.9 and fix it.
linux-stable-mirror@lists.linaro.org
-
Abel Vesa -
Konrad Dybcio -
Krzysztof Kozlowski -
Rob Herring