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@xxxxxxxxxxxxxx> >>>>>>> >>>>>>> Add device tree bindings for display clock controller for >>>>>>> Qualcomm Technology Inc's SM6125 SoC. >>>>>>> >>>>>>> Signed-off-by: Martin Botka <martin.botka@xxxxxxxxxxxxxx> >>>>>>> --- >>>>>>> .../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@xxxxxxxxxxxxxx> >>>>>>> + >>>>>>> +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
