Re: [PATCH v3 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings

Guillaume Ranquet
2022-11-07T14:56:46Z

On Mon, 07 Nov 2022 11:02, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>On 04/11/2022 15:09, Guillaume Ranquet wrote:
>> Add mt8195 SoC bindings for hdmi and hdmi-ddc
>>
>> On mt8195 the ddc i2c controller is part of the hdmi IP block and thus has no
>> specific register range, power domain or interrupt, making it simpler
>> than its the legacy "mediatek,hdmi-ddc" binding.
>>
>> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
>> ---
>> .../bindings/display/mediatek/mediatek,hdmi.yaml | 61 ++++++++++++++++++----
>> .../display/mediatek/mediatek,mt8195-hdmi-ddc.yaml | 51 ++++++++++++++++++
>> 2 files changed, 101 insertions(+), 11 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
>> index bdaf0b51e68c..9710b7b6e9bf 100644
>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml
>> @@ -21,6 +21,7 @@ properties:
>> - mediatek,mt7623-hdmi
>> - mediatek,mt8167-hdmi
>> - mediatek,mt8173-hdmi
>> + - mediatek,mt8195-hdmi
>>
>> reg:
>> maxItems: 1
>> @@ -29,18 +30,12 @@ properties:
>> maxItems: 1
>>
>> clocks:
>> - items:
>> - - description: Pixel Clock
>> - - description: HDMI PLL
>> - - description: Bit Clock
>> - - description: S/PDIF Clock
>> + minItems: 4
>
>Drop minItems, it's not needed when equal to maxItems.
>
>> + maxItems: 4
>>
>> clock-names:
>> - items:
>> - - const: pixel
>> - - const: pll
>> - - const: bclk
>> - - const: spdif
>> + minItems: 4
>
>Drop minItems, it's not needed when equal to maxItems.
>
>
>> + maxItems: 4
>>
>> phys:
>> maxItems: 1
>> @@ -58,6 +53,9 @@ properties:
>> description: |
>> phandle link and register offset to the system configuration registers.
>>
>> + power-domains:
>> + maxItems: 1
>> +
>> ports:
>> $ref: /schemas/graph.yaml#/properties/ports
>>
>> @@ -86,9 +84,50 @@ required:
>> - clock-names
>> - phys
>> - phy-names
>> - - mediatek,syscon-hdmi
>> - ports
>>
>> +allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: mediatek,mt8195-hdmi
>> + then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: APB
>> + - description: HDCP
>> + - description: HDCP 24M
>> + - description: Split HDMI
>> + clock-names:
>> + items:
>> + - const: hdmi_apb_sel
>> + - const: hdcp_sel
>> + - const: hdcp24_sel
>> + - const: split_hdmi
>> +
>> + required:
>> + - power-domains
>> + else:
>> + properties:
>> + clocks:
>> + items:
>> + - description: Pixel Clock
>> + - description: HDMI PLL
>> + - description: Bit Clock
>> + - description: S/PDIF Clock
>> +
>> + clock-names:
>> + items:
>> + - const: pixel
>> + - const: pll
>> + - const: bclk
>> + - const: spdif
>> +
>> + required:
>> + - mediatek,syscon-hdmi
>> +
>> additionalProperties: false
>>
>> examples:
>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
>> new file mode 100644
>> index 000000000000..2dc273689584
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Mediatek HDMI DDC for mt8195
>> +
>> +maintainers:
>> + - CK Hu <ck.hu@mediatek.com>
>> + - Jitao shi <jitao.shi@mediatek.com>
>> +
>> +description: |
>> + The HDMI DDC i2c controller is used to interface with the HDMI DDC pins.
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - mediatek,mt8195-hdmi-ddc
>> +
>> + clocks:
>> + maxItems: 1
>> +
>> + clock-names:
>> + items:
>> + - const: ddc
>
>Unless you expect it to grow, I propose to drop clock-names. It's not
>useful when the name is the same as name of hardware.
>
>Best regards,
>Krzysztof
>

Thx for your suggestions, I'll apply them in v4.

Thx,
Guillaume.