On Thu, 22 Sep 2022 09:18, Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> wrote: >On 19/09/2022 18:56, Guillaume Ranquet wrote: >> Add mt8195 SoC bindings for hdmi and hdmi-ddc >> >> Make port1 optional for mt8195 as it only supports HDMI tx for now. >> Requires a ddc-i2c-bus phandle. >> Requires a power-domains phandle. >> >> Signed-off-by: Guillaume Ranquet <granquet@xxxxxxxxxxxx> >> >> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml >> index bdaf0b51e68c..abb231a0694b 100644 >> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,hdmi.yaml >> @@ -21,6 +21,10 @@ properties: >> - mediatek,mt7623-hdmi >> - mediatek,mt8167-hdmi >> - mediatek,mt8173-hdmi >> + - mediatek,mt8195-hdmi >> + >> + clocks: true >> + clock-names: true > >???? >Why is this moved? > >> >> reg: >> maxItems: 1 >> @@ -28,20 +32,6 @@ properties: >> interrupts: >> maxItems: 1 >> >> - 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 > >Clock definition with constraints should stay here. You just customize >it per variant. > Clocks are different between the two hardwares, so I've tried moving everything inside the if/else block. Is there a better way to express this? >> - >> phys: >> maxItems: 1 >> >> @@ -58,6 +48,16 @@ properties: >> description: | >> phandle link and register offset to the system configuration registers. >> >> + ddc-i2c-bus: >> + $ref: '/schemas/types.yaml#/definitions/phandle' > >Drop quotes > >> + description: Phandle to the ddc-i2c device > >Isn't this property of panel? > It's a property used in panels and connectors. But since this IP doesn't use a connector per say, I've added the property here. Which doesn't sound reasonnable when I'm explaining it like this... I'll see what I can do to fit a connector and have things look a bit more standard. >> + >> + power-domains: >> + description: >> + A phandle and PM domain specifier as defined by bindings >> + of the power controller specified by phandle. See >> + Documentation/devicetree/bindings/power/power-domain.yaml for details. > >No need for this text. This is standard property. You miss maxItems. > > >> + >> ports: >> $ref: /schemas/graph.yaml#/properties/ports >> >> @@ -76,7 +76,6 @@ properties: >> >> required: >> - port@0 >> - - port@1 >> >> required: >> - compatible >> @@ -86,9 +85,55 @@ 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 > >Clocks are entirely different. I am not sure there is benefit in keeping >these devices in one bindings. > I agree with that, but it was requested by CK that the driver and bindings be as common as possible. >> + >> + required: >> + - power-domains >> + - ddc-i2c-bus > >Blank line, > >> + 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 >> + >> + ports: >> + required: >> + - port@1 >> + >> + 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..3c80bcebe6d3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml >> @@ -0,0 +1,45 @@ >> +# 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 Device Tree Bindings for mt8195 > >Drop Device Tree Bindings > >> + >> +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. > >Why is this different than existing ddc bindings? > This ddc is actually part of the MT8195 hdmi IP. So it is a bit simpler than the mediatek,hdmi-ddc.yaml As it has only one clock, no reg, no interrupts. >> + >> +properties: >> + compatible: >> + enum: >> + - mediatek,mt8195-hdmi-ddc >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-names: >> + items: >> + - const: ddc-i2c >> + >> +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 { > >No underscores in node names. Generic node names. > > >Best regards, >Krzysztof >
