diff mbox series

[1/6] dt-bindings: phy: qcom,hdmi-phy-other: use pxo clock

Message ID 20220909132010.3814817-2-dmitry.baryshkov@linaro.org
State Superseded
Headers show
Series [1/6] dt-bindings: phy: qcom,hdmi-phy-other: use pxo clock | expand

Commit Message

Dmitry Baryshkov Sept. 9, 2022, 1:20 p.m. UTC
Add pxo clock to the 8960 bindings (used by the HDMI PLL)

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../bindings/phy/qcom,hdmi-phy-other.yaml     | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Krzysztof Kozlowski Sept. 10, 2022, 7:53 a.m. UTC | #1
On 09/09/2022 22:29, Dmitry Baryshkov wrote:
> 
> 
> On 9 September 2022 19:44:11 GMT+03:00, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>> On 09/09/2022 17:03, Dmitry Baryshkov wrote:
>>>>> +    then:
>>>>> +      properties:
>>>>> +        clocks:
>>>>> +          minItems: 1
>>>>> +          maxItems: 2
>>>>> +        clock-names:
>>>>> +          minItems: 1
>>>>> +          items:
>>>>> +            - const: slave_iface
>>>>> +            - const: pxo
>>>>
>>>> Why pxo is optional? Commit msg does not say much here.
>>>
>>> It's optional as it is not present in current DT files. The driver will 
>>> fallback to 'pxo_board' if the clock is not present.
>>>
>>>> It seems you also miss the DTS change adding the clock.
>>>
>>> Oh, I'll add it to v2.
>>
>> How about adding it to DTS and making it required in the bindings? I did
>> not check the driver, but isn't the driver fail if clock is missing thus
>> the clock is really required?
> 
> I had the impression that we cannot make a clock mandatory of it wasn't present before. Please correct me if I'm wrong.

We cannot break the ABI which means implementation must accept old DTS.
As you wrote below, the implementation will handle this case. Whether we
can require new DTS properties (if implementation respects ABI) is a
different problem and I believe that we can. Bindings can grow, even
with necessary changes, because no ABI is actually broken here.

> 
> No, the driver will not fail. It will fallback to the lookup of the `pxo_board' clock from the system clock list.

Ah, good!


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-other.yaml b/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-other.yaml
index fdb277edebeb..2c21e120ff8d 100644
--- a/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-other.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,hdmi-phy-other.yaml
@@ -53,7 +53,6 @@  allOf:
           contains:
             enum:
               - qcom,hdmi-phy-8660
-              - qcom,hdmi-phy-8960
     then:
       properties:
         clocks:
@@ -63,6 +62,24 @@  allOf:
             - const: slave_iface
         vddio-supply: false
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,hdmi-phy-8960
+    then:
+      properties:
+        clocks:
+          minItems: 1
+          maxItems: 2
+        clock-names:
+          minItems: 1
+          items:
+            - const: slave_iface
+            - const: pxo
+        vddio-supply: false
+
   - if:
       properties:
         compatible:
@@ -98,7 +115,7 @@  examples:
             <0x4a00500 0x100>;
       #phy-cells = <0>;
       power-domains = <&mmcc 1>;
-      clock-names = "slave_iface";
-      clocks = <&clk 21>;
+      clock-names = "slave_iface", "pxo";
+      clocks = <&clk 21>, <&pxo_board>;
       core-vdda-supply = <&pm8921_hdmi_mvs>;
     };