On Thu, 03 Nov 2022 13:45, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: >On 02/11/2022 09:31, Guillaume Ranquet wrote: >> On Fri, 14 Oct 2022 18:08, Krzysztof Kozlowski >> <krzysztof.kozlowski@xxxxxxxxxx> wrote: >>> On 14/10/2022 11:15, Guillaume Ranquet wrote: >>>> Add mt8195 SoC bindings for hdmi and hdmi-ddc >>>> >>>> Signed-off-by: Guillaume Ranquet <granquet@xxxxxxxxxxxx> >>>> --- >>>> .../bindings/display/mediatek/mediatek,hdmi.yaml | 67 +++++++++++++++++----- >>>> .../display/mediatek/mediatek,mt8195-hdmi-ddc.yaml | 51 ++++++++++++++++ >>>> 2 files changed, 104 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml >>>> index bdaf0b51e68c..955026cd7ca5 100644 >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml >>>> @@ -21,26 +21,21 @@ properties: >>>> - mediatek,mt7623-hdmi >>>> - mediatek,mt8167-hdmi >>>> - mediatek,mt8173-hdmi >>>> + - mediatek,mt8195-hdmi >>>> >>>> reg: >>>> maxItems: 1 >>>> >>>> - interrupts: >>>> - maxItems: 1 >>>> - >>> >>> This change is not really explained in commit msg... >>> >>>> clocks: >>>> - items: >>>> - - description: Pixel Clock >>>> - - description: HDMI PLL >>>> - - description: Bit Clock >>>> - - description: S/PDIF Clock >>>> + minItems: 4 >>>> + maxItems: 4 >>>> >>>> clock-names: >>>> - items: >>>> - - const: pixel >>>> - - const: pll >>>> - - const: bclk >>>> - - const: spdif >>>> + minItems: 4 >>>> + maxItems: 4 >>>> + >>>> + interrupts: >>>> + maxItems: 1 >>>> >>>> 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..0fe0a2a2f17f >>>> --- /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@xxxxxxxxxxxx> >>>> + - Jitao shi <jitao.shi@xxxxxxxxxxxx> >>>> + >>>> +description: | >>>> + The HDMI DDC i2c controller is used to interface with the HDMI DDC pins. >>>> + >>>> +properties: >>>> + compatible: >>>> + enum: >>>> + - mediatek,mt8195-hdmi-ddc >>> >>> I think I wrote it - you already have bindings for HDMI DDC. I doubt >>> that these are different and it looks like you model the bindings >>> according to your driver. That's not the way. >> >> Hi Krzysztof, >> >> I've made a separate binding as this new IP is integrated into the >> HDMI hw block. >> The difference it makes is that the hw is slightly simpler to describe >> as the IP doesn't >> have it's own range of registers or an interrupt line. >> >> I can use the "legacy mediatek mtk ddc binding" if I modify it to have >> the reg and >> interrupt properties not being required for mt8195. > >OK, it is reasonable - such stuff should be in commit msg, so we won't >keep asking. > I'll sum this up in the commit msg for V3 then. >> >> Would that work better for you? >> >>> >>>> + >>>> + clocks: >>>> + maxItems: 1 >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: ddc >>>> + >>>> + mediatek,hdmi: >>>> + $ref: /schemas/types.yaml#/definitions/phandle >>>> + description: >>>> + A phandle to the mt8195 hdmi controller >>>> + >>>> +required: >>>> + - compatible >>>> + - clocks >>>> + - clock-names >>>> + >>>> +additionalProperties: false >>>> + >>>> +examples: >>>> + - | >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h> >>>> + #include <dt-bindings/interrupt-controller/irq.h> >>>> + hdmiddc0: ddc_i2c { >>> >>> Node names should be generic - ddc. >>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation >>> >>> No underscores in node names. >>> >>> Additionally I2C devices have addresses on the bus. Why this one doesn't? >>> >> >> This is an i2c adapter, not a device. >> And as it lives inside the HDMI hw block, I've omitted using an address here. >> >> Is this valid? or should this be expressed differently? > >What is an I2C adapter? Did you mean I2C controller (master)? Yes, a controller. This is an I2C controller connected to the HDMI connector, it is used to exchange data on the Display Data Channel with the display (such as EDID). Thx, Guillaume. > >Best regards, >Krzysztof >
