* [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
[not found] <20220226200911.230030-1-marijn.suijten@somainline.org>
@ 2022-02-26 20:09 ` Marijn Suijten
2022-02-26 22:41 ` Marijn Suijten
2022-02-27 10:03 ` Krzysztof Kozlowski
0 siblings, 2 replies; 9+ messages in thread
From: Marijn Suijten @ 2022-02-26 20:09 UTC (permalink / raw)
To: phone-devel
Cc: ~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-clk,
devicetree, linux-kernel
From: Martin Botka <martin.botka@somainline.org>
Add device tree bindings for display clock controller for
Qualcomm Technology Inc's SM6125 SoC.
Signed-off-by: Martin Botka <martin.botka@somainline.org>
---
.../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
.../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
2 files changed, 128 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
new file mode 100644
index 000000000000..3465042d0d9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Display Clock Controller Binding for SM6125
+
+maintainers:
+ - Martin Botka <martin.botka@somainline.org>
+
+description: |
+ Qualcomm display clock control module which supports the clocks and
+ power domains on SM6125.
+
+ See also:
+ dt-bindings/clock/qcom,dispcc-sm6125.h
+
+properties:
+ compatible:
+ enum:
+ - qcom,sm6125-dispcc
+
+ clocks:
+ items:
+ - description: Board XO source
+ - description: Byte clock from DSI PHY0
+ - description: Pixel clock from DSI PHY0
+ - 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:
+ - const: bi_tcxo
+ - const: dsi0_phy_pll_out_byteclk
+ - const: dsi0_phy_pll_out_dsiclk
+ - 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
+
+ '#power-domain-cells':
+ const: 1
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - '#clock-cells'
+ - '#power-domain-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/qcom,rpmcc.h>
+ #include <dt-bindings/clock/qcom,gcc-sm6125.h>
+ clock-controller@5f00000 {
+ compatible = "qcom,sm6125-dispcc";
+ reg = <0x5f00000 0x20000>;
+ clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
+ <&dsi0_phy 0>,
+ <&dsi0_phy 1>,
+ <0>,
+ <&dp_phy 0>,
+ <&dp_phy 1>,
+ <&gcc GCC_DISP_AHB_CLK>;
+ 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";
+ #clock-cells = <1>;
+ #power-domain-cells = <1>;
+ };
+...
diff --git a/include/dt-bindings/clock/qcom,dispcc-sm6125.h b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
new file mode 100644
index 000000000000..4ff974f4fcc3
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
+#define _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
+
+#define DISP_CC_PLL0 0
+#define DISP_CC_MDSS_AHB_CLK 1
+#define DISP_CC_MDSS_AHB_CLK_SRC 2
+#define DISP_CC_MDSS_BYTE0_CLK 3
+#define DISP_CC_MDSS_BYTE0_CLK_SRC 4
+#define DISP_CC_MDSS_BYTE0_INTF_CLK 5
+#define DISP_CC_MDSS_DP_AUX_CLK 6
+#define DISP_CC_MDSS_DP_AUX_CLK_SRC 7
+#define DISP_CC_MDSS_DP_CRYPTO_CLK 8
+#define DISP_CC_MDSS_DP_CRYPTO_CLK_SRC 9
+#define DISP_CC_MDSS_DP_LINK_CLK 10
+#define DISP_CC_MDSS_DP_LINK_CLK_SRC 11
+#define DISP_CC_MDSS_DP_LINK_INTF_CLK 12
+#define DISP_CC_MDSS_DP_PIXEL_CLK 13
+#define DISP_CC_MDSS_DP_PIXEL_CLK_SRC 14
+#define DISP_CC_MDSS_ESC0_CLK 15
+#define DISP_CC_MDSS_ESC0_CLK_SRC 16
+#define DISP_CC_MDSS_MDP_CLK 17
+#define DISP_CC_MDSS_MDP_CLK_SRC 18
+#define DISP_CC_MDSS_MDP_LUT_CLK 19
+#define DISP_CC_MDSS_NON_GDSC_AHB_CLK 20
+#define DISP_CC_MDSS_PCLK0_CLK 21
+#define DISP_CC_MDSS_PCLK0_CLK_SRC 22
+#define DISP_CC_MDSS_ROT_CLK 23
+#define DISP_CC_MDSS_ROT_CLK_SRC 24
+#define DISP_CC_MDSS_VSYNC_CLK 25
+#define DISP_CC_MDSS_VSYNC_CLK_SRC 26
+#define DISP_CC_XO_CLK 27
+
+/* DISP_CC GDSCR */
+#define MDSS_GDSC 0
+
+#endif
--
2.35.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-02-26 20:09 ` [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings Marijn Suijten
@ 2022-02-26 22:41 ` Marijn Suijten
2022-02-27 10:03 ` Krzysztof Kozlowski
1 sibling, 0 replies; 9+ messages in thread
From: Marijn Suijten @ 2022-02-26 22:41 UTC (permalink / raw)
To: phone-devel
Cc: ~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, linux-arm-msm, linux-clk,
devicetree, linux-kernel
On 2022-02-26 21:09:10, Marijn Suijten wrote:
> From: Martin Botka <martin.botka@somainline.org>
>
> Add device tree bindings for display clock controller for
> Qualcomm Technology Inc's SM6125 SoC.
>
> Signed-off-by: Martin Botka <martin.botka@somainline.org>
This of course lacks the mandatory sign-off after getting permission to
apply my own review and mailing-list review, and resending the patch:
Signed-off-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
> 2 files changed, 128 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> new file mode 100644
> index 000000000000..3465042d0d9f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Display Clock Controller Binding for SM6125
> +
> +maintainers:
> + - Martin Botka <martin.botka@somainline.org>
> +
> +description: |
> + Qualcomm display clock control module which supports the clocks and
> + power domains on SM6125.
> +
> + See also:
> + dt-bindings/clock/qcom,dispcc-sm6125.h
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,sm6125-dispcc
> +
> + clocks:
> + items:
> + - description: Board XO source
> + - description: Byte clock from DSI PHY0
> + - description: Pixel clock from DSI PHY0
> + - 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:
> + - const: bi_tcxo
> + - const: dsi0_phy_pll_out_byteclk
> + - const: dsi0_phy_pll_out_dsiclk
> + - 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
> +
> + '#power-domain-cells':
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - '#clock-cells'
> + - '#power-domain-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,rpmcc.h>
> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> + clock-controller@5f00000 {
> + compatible = "qcom,sm6125-dispcc";
> + reg = <0x5f00000 0x20000>;
> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> + <&dsi0_phy 0>,
> + <&dsi0_phy 1>,
> + <0>,
> + <&dp_phy 0>,
> + <&dp_phy 1>,
> + <&gcc GCC_DISP_AHB_CLK>;
> + 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";
> + #clock-cells = <1>;
> + #power-domain-cells = <1>;
> + };
> +...
> diff --git a/include/dt-bindings/clock/qcom,dispcc-sm6125.h b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
> new file mode 100644
> index 000000000000..4ff974f4fcc3
> --- /dev/null
> +++ b/include/dt-bindings/clock/qcom,dispcc-sm6125.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
> +#define _DT_BINDINGS_CLK_QCOM_DISP_CC_SM6125_H
> +
> +#define DISP_CC_PLL0 0
> +#define DISP_CC_MDSS_AHB_CLK 1
> +#define DISP_CC_MDSS_AHB_CLK_SRC 2
> +#define DISP_CC_MDSS_BYTE0_CLK 3
> +#define DISP_CC_MDSS_BYTE0_CLK_SRC 4
> +#define DISP_CC_MDSS_BYTE0_INTF_CLK 5
> +#define DISP_CC_MDSS_DP_AUX_CLK 6
> +#define DISP_CC_MDSS_DP_AUX_CLK_SRC 7
> +#define DISP_CC_MDSS_DP_CRYPTO_CLK 8
> +#define DISP_CC_MDSS_DP_CRYPTO_CLK_SRC 9
> +#define DISP_CC_MDSS_DP_LINK_CLK 10
> +#define DISP_CC_MDSS_DP_LINK_CLK_SRC 11
> +#define DISP_CC_MDSS_DP_LINK_INTF_CLK 12
> +#define DISP_CC_MDSS_DP_PIXEL_CLK 13
> +#define DISP_CC_MDSS_DP_PIXEL_CLK_SRC 14
> +#define DISP_CC_MDSS_ESC0_CLK 15
> +#define DISP_CC_MDSS_ESC0_CLK_SRC 16
> +#define DISP_CC_MDSS_MDP_CLK 17
> +#define DISP_CC_MDSS_MDP_CLK_SRC 18
> +#define DISP_CC_MDSS_MDP_LUT_CLK 19
> +#define DISP_CC_MDSS_NON_GDSC_AHB_CLK 20
> +#define DISP_CC_MDSS_PCLK0_CLK 21
> +#define DISP_CC_MDSS_PCLK0_CLK_SRC 22
> +#define DISP_CC_MDSS_ROT_CLK 23
> +#define DISP_CC_MDSS_ROT_CLK_SRC 24
> +#define DISP_CC_MDSS_VSYNC_CLK 25
> +#define DISP_CC_MDSS_VSYNC_CLK_SRC 26
> +#define DISP_CC_XO_CLK 27
> +
> +/* DISP_CC GDSCR */
> +#define MDSS_GDSC 0
> +
> +#endif
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-02-26 20:09 ` [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings Marijn Suijten
2022-02-26 22:41 ` Marijn Suijten
@ 2022-02-27 10:03 ` Krzysztof Kozlowski
2022-02-27 21:43 ` Dmitry Baryshkov
1 sibling, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-02-27 10:03 UTC (permalink / raw)
To: Marijn Suijten, phone-devel
Cc: ~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, linux-arm-msm, linux-clk, devicetree, linux-kernel
On 26/02/2022 21:09, Marijn Suijten wrote:
> From: Martin Botka <martin.botka@somainline.org>
>
> Add device tree bindings for display clock controller for
> Qualcomm Technology Inc's SM6125 SoC.
>
> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> ---
> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
> 2 files changed, 128 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> new file mode 100644
> index 000000000000..3465042d0d9f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Display Clock Controller Binding for SM6125
> +
> +maintainers:
> + - Martin Botka <martin.botka@somainline.org>
> +
> +description: |
> + Qualcomm display clock control module which supports the clocks and
> + power domains on SM6125.
> +
> + See also:
> + dt-bindings/clock/qcom,dispcc-sm6125.h
> +
> +properties:
> + compatible:
> + enum:
> + - qcom,sm6125-dispcc
> +
> + clocks:
> + items:
> + - description: Board XO source
> + - description: Byte clock from DSI PHY0
> + - description: Pixel clock from DSI PHY0
> + - 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:
> + - const: bi_tcxo
> + - const: dsi0_phy_pll_out_byteclk
> + - const: dsi0_phy_pll_out_dsiclk
> + - 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
> +
> + '#power-domain-cells':
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - '#clock-cells'
> + - '#power-domain-cells'
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/qcom,rpmcc.h>
> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> + clock-controller@5f00000 {
> + compatible = "qcom,sm6125-dispcc";
> + reg = <0x5f00000 0x20000>;
> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> + <&dsi0_phy 0>,
> + <&dsi0_phy 1>,
> + <0>,
This does not look like a valid phandle. This clock is required, isn't it?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-02-27 10:03 ` Krzysztof Kozlowski
@ 2022-02-27 21:43 ` Dmitry Baryshkov
2022-02-28 9:23 ` Krzysztof Kozlowski
0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Baryshkov @ 2022-02-27 21:43 UTC (permalink / raw)
To: Krzysztof Kozlowski, Marijn Suijten, phone-devel
Cc: ~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, linux-arm-msm, linux-clk, devicetree, linux-kernel
On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
> On 26/02/2022 21:09, Marijn Suijten wrote:
>> From: Martin Botka <martin.botka@somainline.org>
>>
>> Add device tree bindings for display clock controller for
>> Qualcomm Technology Inc's SM6125 SoC.
>>
>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>> ---
>> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
>> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
>> 2 files changed, 128 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>> new file mode 100644
>> index 000000000000..3465042d0d9f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>> @@ -0,0 +1,87 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Display Clock Controller Binding for SM6125
>> +
>> +maintainers:
>> + - Martin Botka <martin.botka@somainline.org>
>> +
>> +description: |
>> + Qualcomm display clock control module which supports the clocks and
>> + power domains on SM6125.
>> +
>> + See also:
>> + dt-bindings/clock/qcom,dispcc-sm6125.h
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - qcom,sm6125-dispcc
>> +
>> + clocks:
>> + items:
>> + - description: Board XO source
>> + - description: Byte clock from DSI PHY0
>> + - description: Pixel clock from DSI PHY0
>> + - 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:
>> + - const: bi_tcxo
>> + - const: dsi0_phy_pll_out_byteclk
>> + - const: dsi0_phy_pll_out_dsiclk
>> + - 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
>> +
>> + '#power-domain-cells':
>> + const: 1
>> +
>> + reg:
>> + maxItems: 1
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> + - clock-names
>> + - '#clock-cells'
>> + - '#power-domain-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,rpmcc.h>
>> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>> + clock-controller@5f00000 {
>> + compatible = "qcom,sm6125-dispcc";
>> + reg = <0x5f00000 0x20000>;
>> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>> + <&dsi0_phy 0>,
>> + <&dsi0_phy 1>,
>> + <0>,
>
> This does not look like a valid phandle. This clock is required, isn't it?
Not, it's not required for general dispcc support.
dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc
clocks. However if support for DP is not enabled, the dispcc can work
w/o DP phy clock. Thus we typically add 0 phandles as placeholders for
DSI/DP clock sources and populate them as support for respective
interfaces gets implemented.
>
>
> Best regards,
> Krzysztof
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-02-27 21:43 ` Dmitry Baryshkov
@ 2022-02-28 9:23 ` Krzysztof Kozlowski
2022-03-02 12:54 ` Marijn Suijten
0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-02-28 9:23 UTC (permalink / raw)
To: Dmitry Baryshkov, Marijn Suijten, phone-devel
Cc: ~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, linux-arm-msm, linux-clk, devicetree, linux-kernel
On 27/02/2022 22:43, Dmitry Baryshkov wrote:
> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
>> On 26/02/2022 21:09, Marijn Suijten wrote:
>>> From: Martin Botka <martin.botka@somainline.org>
>>>
>>> Add device tree bindings for display clock controller for
>>> Qualcomm Technology Inc's SM6125 SoC.
>>>
>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>> ---
>>> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
>>> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
>>> 2 files changed, 128 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> new file mode 100644
>>> index 000000000000..3465042d0d9f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>> @@ -0,0 +1,87 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Qualcomm Display Clock Controller Binding for SM6125
>>> +
>>> +maintainers:
>>> + - Martin Botka <martin.botka@somainline.org>
>>> +
>>> +description: |
>>> + Qualcomm display clock control module which supports the clocks and
>>> + power domains on SM6125.
>>> +
>>> + See also:
>>> + dt-bindings/clock/qcom,dispcc-sm6125.h
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - qcom,sm6125-dispcc
>>> +
>>> + clocks:
>>> + items:
>>> + - description: Board XO source
>>> + - description: Byte clock from DSI PHY0
>>> + - description: Pixel clock from DSI PHY0
>>> + - 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:
>>> + - const: bi_tcxo
>>> + - const: dsi0_phy_pll_out_byteclk
>>> + - const: dsi0_phy_pll_out_dsiclk
>>> + - 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
>>> +
>>> + '#power-domain-cells':
>>> + const: 1
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - clocks
>>> + - clock-names
>>> + - '#clock-cells'
>>> + - '#power-domain-cells'
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/clock/qcom,rpmcc.h>
>>> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>>> + clock-controller@5f00000 {
>>> + compatible = "qcom,sm6125-dispcc";
>>> + reg = <0x5f00000 0x20000>;
>>> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>>> + <&dsi0_phy 0>,
>>> + <&dsi0_phy 1>,
>>> + <0>,
>>
>> This does not look like a valid phandle. This clock is required, isn't it?
>
> Not, it's not required for general dispcc support.
> dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc
> clocks. However if support for DP is not enabled, the dispcc can work
> w/o DP phy clock. Thus we typically add 0 phandles as placeholders for
> DSI/DP clock sources and populate them as support for respective
> interfaces gets implemented.
>
Then the clock is optional, isn't it? While not modeling it as optional?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-02-28 9:23 ` Krzysztof Kozlowski
@ 2022-03-02 12:54 ` Marijn Suijten
2022-03-02 13:51 ` Krzysztof Kozlowski
0 siblings, 1 reply; 9+ messages in thread
From: Marijn Suijten @ 2022-03-02 12:54 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Dmitry Baryshkov, phone-devel, ~postmarketos/upstreaming,
AngeloGioacchino Del Regno, Konrad Dybcio, Martin Botka,
Jami Kettunen, Pavel Dubrova, Andy Gross, Bjorn Andersson,
Michael Turquette, Stephen Boyd, Rob Herring, linux-arm-msm,
linux-clk, devicetree, linux-kernel
On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
> > On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
> >> On 26/02/2022 21:09, Marijn Suijten wrote:
> >>> From: Martin Botka <martin.botka@somainline.org>
> >>>
> >>> Add device tree bindings for display clock controller for
> >>> Qualcomm Technology Inc's SM6125 SoC.
> >>>
> >>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> >>> ---
> >>> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
> >>> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
> >>> 2 files changed, 128 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> new file mode 100644
> >>> index 000000000000..3465042d0d9f
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>> @@ -0,0 +1,87 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Qualcomm Display Clock Controller Binding for SM6125
> >>> +
> >>> +maintainers:
> >>> + - Martin Botka <martin.botka@somainline.org>
> >>> +
> >>> +description: |
> >>> + Qualcomm display clock control module which supports the clocks and
> >>> + power domains on SM6125.
> >>> +
> >>> + See also:
> >>> + dt-bindings/clock/qcom,dispcc-sm6125.h
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - qcom,sm6125-dispcc
> >>> +
> >>> + clocks:
> >>> + items:
> >>> + - description: Board XO source
> >>> + - description: Byte clock from DSI PHY0
> >>> + - description: Pixel clock from DSI PHY0
> >>> + - 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:
> >>> + - const: bi_tcxo
> >>> + - const: dsi0_phy_pll_out_byteclk
> >>> + - const: dsi0_phy_pll_out_dsiclk
> >>> + - 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
> >>> +
> >>> + '#power-domain-cells':
> >>> + const: 1
> >>> +
> >>> + reg:
> >>> + maxItems: 1
> >>> +
> >>> +required:
> >>> + - compatible
> >>> + - reg
> >>> + - clocks
> >>> + - clock-names
> >>> + - '#clock-cells'
> >>> + - '#power-domain-cells'
> >>> +
> >>> +additionalProperties: false
> >>> +
> >>> +examples:
> >>> + - |
> >>> + #include <dt-bindings/clock/qcom,rpmcc.h>
> >>> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> >>> + clock-controller@5f00000 {
> >>> + compatible = "qcom,sm6125-dispcc";
> >>> + reg = <0x5f00000 0x20000>;
> >>> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> >>> + <&dsi0_phy 0>,
> >>> + <&dsi0_phy 1>,
> >>> + <0>,
> >>
> >> This does not look like a valid phandle. This clock is required, isn't it?
I remember it being used like this before, though upon closer inspection
only qcom,gcc-msm8998.yaml uses it as example.
The clock should be optional, in that case it is perhaps desired to omit
it from clock-names instead, or pretend there's a `dsi1_phy 1`?
> >
> > Not, it's not required for general dispcc support.
> > dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc
> > clocks. However if support for DP is not enabled, the dispcc can work
> > w/o DP phy clock. Thus we typically add 0 phandles as placeholders for
Is there any semantic difference between omitting the clock from DT (in
clocks= /and/ clock-names=) or setting it to a 0 phandle?
> > DSI/DP clock sources and populate them as support for respective
> > interfaces gets implemented.
> >
>
> Then the clock is optional, isn't it? While not modeling it as optional?
It looks like this should be modelled using minItems: then, and
"optional" text/comment? Other clocks are optional as well, we don't
have DSI 1 in downstream SM6125 DT sources and haven't added the DP PLL
in our to-be-upstreamed mainline tree yet.
- Marijn
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-03-02 12:54 ` Marijn Suijten
@ 2022-03-02 13:51 ` Krzysztof Kozlowski
2022-03-02 14:48 ` Bjorn Andersson
0 siblings, 1 reply; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-02 13:51 UTC (permalink / raw)
To: Marijn Suijten, Dmitry Baryshkov, phone-devel,
~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, linux-arm-msm, linux-clk, devicetree, linux-kernel
On 02/03/2022 13:54, Marijn Suijten wrote:
> On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
>> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
>>> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
>>>> On 26/02/2022 21:09, Marijn Suijten wrote:
>>>>> From: Martin Botka <martin.botka@somainline.org>
>>>>>
>>>>> Add device tree bindings for display clock controller for
>>>>> Qualcomm Technology Inc's SM6125 SoC.
>>>>>
>>>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>>>> ---
>>>>> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
>>>>> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
>>>>> 2 files changed, 128 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..3465042d0d9f
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>> @@ -0,0 +1,87 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Qualcomm Display Clock Controller Binding for SM6125
>>>>> +
>>>>> +maintainers:
>>>>> + - Martin Botka <martin.botka@somainline.org>
>>>>> +
>>>>> +description: |
>>>>> + Qualcomm display clock control module which supports the clocks and
>>>>> + power domains on SM6125.
>>>>> +
>>>>> + See also:
>>>>> + dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>> +
>>>>> +properties:
>>>>> + compatible:
>>>>> + enum:
>>>>> + - qcom,sm6125-dispcc
>>>>> +
>>>>> + clocks:
>>>>> + items:
>>>>> + - description: Board XO source
>>>>> + - description: Byte clock from DSI PHY0
>>>>> + - description: Pixel clock from DSI PHY0
>>>>> + - 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:
>>>>> + - const: bi_tcxo
>>>>> + - const: dsi0_phy_pll_out_byteclk
>>>>> + - const: dsi0_phy_pll_out_dsiclk
>>>>> + - 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
>>>>> +
>>>>> + '#power-domain-cells':
>>>>> + const: 1
>>>>> +
>>>>> + reg:
>>>>> + maxItems: 1
>>>>> +
>>>>> +required:
>>>>> + - compatible
>>>>> + - reg
>>>>> + - clocks
>>>>> + - clock-names
>>>>> + - '#clock-cells'
>>>>> + - '#power-domain-cells'
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> + - |
>>>>> + #include <dt-bindings/clock/qcom,rpmcc.h>
>>>>> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>>>>> + clock-controller@5f00000 {
>>>>> + compatible = "qcom,sm6125-dispcc";
>>>>> + reg = <0x5f00000 0x20000>;
>>>>> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>>>>> + <&dsi0_phy 0>,
>>>>> + <&dsi0_phy 1>,
>>>>> + <0>,
>>>>
>>>> This does not look like a valid phandle. This clock is required, isn't it?
>
> I remember it being used like this before, though upon closer inspection
> only qcom,gcc-msm8998.yaml uses it as example.
>
> The clock should be optional, in that case it is perhaps desired to omit
> it from clock-names instead, or pretend there's a `dsi1_phy 1`?
I propose to omit it.
>
>>>
>>> Not, it's not required for general dispcc support.
>>> dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc
>>> clocks. However if support for DP is not enabled, the dispcc can work
>>> w/o DP phy clock. Thus we typically add 0 phandles as placeholders for
>
> Is there any semantic difference between omitting the clock from DT (in
> clocks= /and/ clock-names=) or setting it to a 0 phandle?
Yes, there is. The DT validation does not check the meaning behind
values, so there is no difference between valid phandle/ID and 0. While
not having a clock at all is spotted by validation.
>
>>> DSI/DP clock sources and populate them as support for respective
>>> interfaces gets implemented.
>>>
>>
>> Then the clock is optional, isn't it? While not modeling it as optional?
>
> It looks like this should be modelled using minItems: then, and
> "optional" text/comment? Other clocks are optional as well, we don't
> have DSI 1 in downstream SM6125 DT sources and haven't added the DP PLL
> in our to-be-upstreamed mainline tree yet.
Are they really optional? Or maybe they should not even be provided?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-03-02 13:51 ` Krzysztof Kozlowski
@ 2022-03-02 14:48 ` Bjorn Andersson
2022-03-02 17:15 ` Krzysztof Kozlowski
0 siblings, 1 reply; 9+ messages in thread
From: Bjorn Andersson @ 2022-03-02 14:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Marijn Suijten, Dmitry Baryshkov, phone-devel,
~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring,
linux-arm-msm, linux-clk, devicetree, linux-kernel
On Wed 02 Mar 05:51 PST 2022, Krzysztof Kozlowski wrote:
> On 02/03/2022 13:54, Marijn Suijten wrote:
> > On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
> >> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
> >>> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
> >>>> On 26/02/2022 21:09, Marijn Suijten wrote:
> >>>>> From: Martin Botka <martin.botka@somainline.org>
> >>>>>
> >>>>> Add device tree bindings for display clock controller for
> >>>>> Qualcomm Technology Inc's SM6125 SoC.
> >>>>>
> >>>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> >>>>> ---
> >>>>> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
> >>>>> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
> >>>>> 2 files changed, 128 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>>>> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..3465042d0d9f
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
> >>>>> @@ -0,0 +1,87 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: Qualcomm Display Clock Controller Binding for SM6125
> >>>>> +
> >>>>> +maintainers:
> >>>>> + - Martin Botka <martin.botka@somainline.org>
> >>>>> +
> >>>>> +description: |
> >>>>> + Qualcomm display clock control module which supports the clocks and
> >>>>> + power domains on SM6125.
> >>>>> +
> >>>>> + See also:
> >>>>> + dt-bindings/clock/qcom,dispcc-sm6125.h
> >>>>> +
> >>>>> +properties:
> >>>>> + compatible:
> >>>>> + enum:
> >>>>> + - qcom,sm6125-dispcc
> >>>>> +
> >>>>> + clocks:
> >>>>> + items:
> >>>>> + - description: Board XO source
> >>>>> + - description: Byte clock from DSI PHY0
> >>>>> + - description: Pixel clock from DSI PHY0
> >>>>> + - 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:
> >>>>> + - const: bi_tcxo
> >>>>> + - const: dsi0_phy_pll_out_byteclk
> >>>>> + - const: dsi0_phy_pll_out_dsiclk
> >>>>> + - 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
> >>>>> +
> >>>>> + '#power-domain-cells':
> >>>>> + const: 1
> >>>>> +
> >>>>> + reg:
> >>>>> + maxItems: 1
> >>>>> +
> >>>>> +required:
> >>>>> + - compatible
> >>>>> + - reg
> >>>>> + - clocks
> >>>>> + - clock-names
> >>>>> + - '#clock-cells'
> >>>>> + - '#power-domain-cells'
> >>>>> +
> >>>>> +additionalProperties: false
> >>>>> +
> >>>>> +examples:
> >>>>> + - |
> >>>>> + #include <dt-bindings/clock/qcom,rpmcc.h>
> >>>>> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
> >>>>> + clock-controller@5f00000 {
> >>>>> + compatible = "qcom,sm6125-dispcc";
> >>>>> + reg = <0x5f00000 0x20000>;
> >>>>> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
> >>>>> + <&dsi0_phy 0>,
> >>>>> + <&dsi0_phy 1>,
> >>>>> + <0>,
> >>>>
> >>>> This does not look like a valid phandle. This clock is required, isn't it?
> >
> > I remember it being used like this before, though upon closer inspection
> > only qcom,gcc-msm8998.yaml uses it as example.
> >
> > The clock should be optional, in that case it is perhaps desired to omit
> > it from clock-names instead, or pretend there's a `dsi1_phy 1`?
>
> I propose to omit it.
>
The wire is there, it's only optional because we don't have the other
side represented in DT yet.
I believe we started filling out 0s like this because omitting elements
that are not yet possible to fill out means that the order will change
as we add more functions, something Rob has objected to. Further more as
we add more functions the existing dts will fail validation, even though
the hardware hasn't changed.
That said, even though we don't have the other piece on this particular
platform we do know where this signal comes from. So we should be able
to have a valid (or at least strongly plausible) example in the binding
- and then fill out the dts with 0s to keep validation happy until the
other pieces are filled out.
Regards,
Bjorn
> >
> >>>
> >>> Not, it's not required for general dispcc support.
> >>> dispcc uses DSI and DP PHY clocks to provide respective pixel/byte/etc
> >>> clocks. However if support for DP is not enabled, the dispcc can work
> >>> w/o DP phy clock. Thus we typically add 0 phandles as placeholders for
> >
> > Is there any semantic difference between omitting the clock from DT (in
> > clocks= /and/ clock-names=) or setting it to a 0 phandle?
>
> Yes, there is. The DT validation does not check the meaning behind
> values, so there is no difference between valid phandle/ID and 0. While
> not having a clock at all is spotted by validation.
>
> >
> >>> DSI/DP clock sources and populate them as support for respective
> >>> interfaces gets implemented.
> >>>
> >>
> >> Then the clock is optional, isn't it? While not modeling it as optional?
> >
> > It looks like this should be modelled using minItems: then, and
> > "optional" text/comment? Other clocks are optional as well, we don't
> > have DSI 1 in downstream SM6125 DT sources and haven't added the DP PLL
> > in our to-be-upstreamed mainline tree yet.
>
> Are they really optional? Or maybe they should not even be provided?
>
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings
2022-03-02 14:48 ` Bjorn Andersson
@ 2022-03-02 17:15 ` Krzysztof Kozlowski
0 siblings, 0 replies; 9+ messages in thread
From: Krzysztof Kozlowski @ 2022-03-02 17:15 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Marijn Suijten, Dmitry Baryshkov, phone-devel,
~postmarketos/upstreaming, AngeloGioacchino Del Regno,
Konrad Dybcio, Martin Botka, Jami Kettunen, Pavel Dubrova,
Andy Gross, Michael Turquette, Stephen Boyd, Rob Herring,
linux-arm-msm, linux-clk, devicetree, linux-kernel
On 02/03/2022 15:48, Bjorn Andersson wrote:
> On Wed 02 Mar 05:51 PST 2022, Krzysztof Kozlowski wrote:
>
>> On 02/03/2022 13:54, Marijn Suijten wrote:
>>> On 2022-02-28 10:23:19, Krzysztof Kozlowski wrote:
>>>> On 27/02/2022 22:43, Dmitry Baryshkov wrote:
>>>>> On 27/02/2022 13:03, Krzysztof Kozlowski wrote:
>>>>>> On 26/02/2022 21:09, Marijn Suijten wrote:
>>>>>>> From: Martin Botka <martin.botka@somainline.org>
>>>>>>>
>>>>>>> Add device tree bindings for display clock controller for
>>>>>>> Qualcomm Technology Inc's SM6125 SoC.
>>>>>>>
>>>>>>> Signed-off-by: Martin Botka <martin.botka@somainline.org>
>>>>>>> ---
>>>>>>> .../bindings/clock/qcom,dispcc-sm6125.yaml | 87 +++++++++++++++++++
>>>>>>> .../dt-bindings/clock/qcom,dispcc-sm6125.h | 41 +++++++++
>>>>>>> 2 files changed, 128 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>>>> create mode 100644 include/dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..3465042d0d9f
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm6125.yaml
>>>>>>> @@ -0,0 +1,87 @@
>>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>>> +%YAML 1.2
>>>>>>> +---
>>>>>>> +$id: http://devicetree.org/schemas/clock/qcom,dispcc-sm6125.yaml#
>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>> +
>>>>>>> +title: Qualcomm Display Clock Controller Binding for SM6125
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> + - Martin Botka <martin.botka@somainline.org>
>>>>>>> +
>>>>>>> +description: |
>>>>>>> + Qualcomm display clock control module which supports the clocks and
>>>>>>> + power domains on SM6125.
>>>>>>> +
>>>>>>> + See also:
>>>>>>> + dt-bindings/clock/qcom,dispcc-sm6125.h
>>>>>>> +
>>>>>>> +properties:
>>>>>>> + compatible:
>>>>>>> + enum:
>>>>>>> + - qcom,sm6125-dispcc
>>>>>>> +
>>>>>>> + clocks:
>>>>>>> + items:
>>>>>>> + - description: Board XO source
>>>>>>> + - description: Byte clock from DSI PHY0
>>>>>>> + - description: Pixel clock from DSI PHY0
>>>>>>> + - 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:
>>>>>>> + - const: bi_tcxo
>>>>>>> + - const: dsi0_phy_pll_out_byteclk
>>>>>>> + - const: dsi0_phy_pll_out_dsiclk
>>>>>>> + - 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
>>>>>>> +
>>>>>>> + '#power-domain-cells':
>>>>>>> + const: 1
>>>>>>> +
>>>>>>> + reg:
>>>>>>> + maxItems: 1
>>>>>>> +
>>>>>>> +required:
>>>>>>> + - compatible
>>>>>>> + - reg
>>>>>>> + - clocks
>>>>>>> + - clock-names
>>>>>>> + - '#clock-cells'
>>>>>>> + - '#power-domain-cells'
>>>>>>> +
>>>>>>> +additionalProperties: false
>>>>>>> +
>>>>>>> +examples:
>>>>>>> + - |
>>>>>>> + #include <dt-bindings/clock/qcom,rpmcc.h>
>>>>>>> + #include <dt-bindings/clock/qcom,gcc-sm6125.h>
>>>>>>> + clock-controller@5f00000 {
>>>>>>> + compatible = "qcom,sm6125-dispcc";
>>>>>>> + reg = <0x5f00000 0x20000>;
>>>>>>> + clocks = <&rpmcc RPM_SMD_XO_CLK_SRC>,
>>>>>>> + <&dsi0_phy 0>,
>>>>>>> + <&dsi0_phy 1>,
>>>>>>> + <0>,
>>>>>>
>>>>>> This does not look like a valid phandle. This clock is required, isn't it?
>>>
>>> I remember it being used like this before, though upon closer inspection
>>> only qcom,gcc-msm8998.yaml uses it as example.
>>>
>>> The clock should be optional, in that case it is perhaps desired to omit
>>> it from clock-names instead, or pretend there's a `dsi1_phy 1`?
>>
>> I propose to omit it.
>>
>
> The wire is there, it's only optional because we don't have the other
> side represented in DT yet.
>
> I believe we started filling out 0s like this because omitting elements
> that are not yet possible to fill out means that the order will change
> as we add more functions, something Rob has objected to. Further more as
> we add more functions the existing dts will fail validation, even though
> the hardware hasn't changed.
>
>
> That said, even though we don't have the other piece on this particular
> platform we do know where this signal comes from. So we should be able
> to have a valid (or at least strongly plausible) example in the binding
> - and then fill out the dts with 0s to keep validation happy until the
> other pieces are filled out.
So based on this, this clock is not actually optional and the bindings
should stay like this. The example should be more-or-less complete, so
there is not much sense to have there clock "0". DTS is of course different.
BR,
Krzysztof
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-03-02 17:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220226200911.230030-1-marijn.suijten@somainline.org>
2022-02-26 20:09 ` [PATCH v2 2/3] dt-bindings: clock: add QCOM SM6125 display clock bindings Marijn Suijten
2022-02-26 22:41 ` Marijn Suijten
2022-02-27 10:03 ` Krzysztof Kozlowski
2022-02-27 21:43 ` Dmitry Baryshkov
2022-02-28 9:23 ` Krzysztof Kozlowski
2022-03-02 12:54 ` Marijn Suijten
2022-03-02 13:51 ` Krzysztof Kozlowski
2022-03-02 14:48 ` Bjorn Andersson
2022-03-02 17:15 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).