| Message ID | 20230624-sm6125-dpu-v1-2-1d5a638cebf2@somainline.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
| Series | drm/msm: Add SM6125 MDSS/DPU hardware and enable Sony Xperia 10 II panel | expand |
On 24/06/2023 02:41, Marijn Suijten wrote: > The downsteam driver for dispcc only ever gets and puts this clock > without ever using it in the clocktree; this unnecessary workaround was > never ported to mainline, hence the driver doesn't consume this clock > and shouldn't be required by the bindings. > > Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings") > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > --- In perfect would we would like to know whether hardware needs this clock enabled/controlled, not whether some driver needs it. I understand though that with lack of proper docs we rely on drivers, so: Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 2023-06-24 11:08:30, Krzysztof Kozlowski wrote: > On 24/06/2023 02:41, Marijn Suijten wrote: > > The downsteam driver for dispcc only ever gets and puts this clock > > without ever using it in the clocktree; this unnecessary workaround was > > never ported to mainline, hence the driver doesn't consume this clock > > and shouldn't be required by the bindings. > > > > Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings") > > Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> > > --- > > In perfect would we would like to know whether hardware needs this clock > enabled/controlled, not whether some driver needs it. I understand > though that with lack of proper docs we rely on drivers, so: It might only use this to figure out if those clocks have already probed or are available. The logic goes as follows: clk = devm_clk_get(&pdev->dev, "cfg_ahb_clk"); if (IS_ERR(clk)) { if (PTR_ERR(clk) != -EPROBE_DEFER) dev_err(&pdev->dev, "Unable to get ahb clock handle\n"); return PTR_ERR(clk); } devm_clk_put(&pdev->dev, clk); Nothing else uses/parents cfg_ahb_clk. Maybe with clk_ignore_unused or similar, it gets turned on but never off again? Indeed, a lack of documentation and comment from the manufacturer makes it impossible to know, and ignoring it (as the driver already does) works just fine. > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Thanks! - Marijn > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml index 8a210c4c5f82..2acf487d8a2f 100644 --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml @@ -28,7 +28,6 @@ properties: - description: Pixel clock from DSI PHY1 - description: Link clock from DP PHY - description: VCO DIV clock from DP PHY - - description: AHB config clock from GCC clock-names: items: @@ -38,7 +37,6 @@ properties: - const: dsi1_phy_pll_out_dsiclk - const: dp_phy_pll_link_clk - const: dp_phy_pll_vco_div_clk - - const: cfg_ahb_clk '#clock-cells': const: 1 @@ -71,15 +69,13 @@ examples: <&dsi0_phy 1>, <&dsi1_phy 1>, <&dp_phy 0>, - <&dp_phy 1>, - <&gcc GCC_DISP_AHB_CLK>; + <&dp_phy 1>; clock-names = "bi_tcxo", "dsi0_phy_pll_out_byteclk", "dsi0_phy_pll_out_dsiclk", "dsi1_phy_pll_out_dsiclk", "dp_phy_pll_link_clk", - "dp_phy_pll_vco_div_clk", - "cfg_ahb_clk"; + "dp_phy_pll_vco_div_clk"; #clock-cells = <1>; #power-domain-cells = <1>; };
The downsteam driver for dispcc only ever gets and puts this clock without ever using it in the clocktree; this unnecessary workaround was never ported to mainline, hence the driver doesn't consume this clock and shouldn't be required by the bindings. Fixes: 8397c9c0c26b ("dt-bindings: clock: add QCOM SM6125 display clock bindings") Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org> --- Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)