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

[AngeloGioacchino Del Regno]

Il 02/01/23 14:38, Guillaume Ranquet ha scritto:
> On Mon, 26 Dec 2022 06:18, CK Hu (胡俊光) <ck.hu@xxxxxxxxxxxx> wrote:
> > Hi, Guillaume:
> >
> > On Fri, 2022-11-04 at 15:09 +0100, 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@xxxxxxxxxxxx>
> > > ---
> >
> > [snip]
> >
> > > 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:
> > > https://urldefense.com/v3/__http://devicetree.org/schemas/display/mediatek/mediatek,mt8195-hdmi-ddc.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WfpOlq57w$
> > >
> > > +$schema:
> > > https://urldefense.com/v3/__http://devicetree.org/meta-schemas/core.yaml*__;Iw!!CTRNKA9wMg0ARbw!wwVQuq5lzW0lvUFUkVXPWT8cIu96xdkn4tMams1E55qyxEZmgV1i0WdSGOSxzw$
> > >
> > > +
> > > +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
> > > +
> > > +  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: i2c {
> > > +      compatible = "mediatek,mt8195-hdmi-ddc";
> > > +      mediatek,hdmi = <&hdmi0>;
> > > +      clocks = <&clk26m>;
> > > +      clock-names = "ddc";
> > > +    };
> >
> > I think we should not have a virtual device. This ddc is part of
> > mt8195-hdmi device, so just keep mt8195-hdmi, and let mt8195-hdmi
> > driver to probe the sub driver of ddc driver.
> >
> > Regards,
> > CK
>
> Hi CK,
>
> Thx for your input.
> Though I would strongly prefer to keep the ddc as a separate "virtual device".
>
> It aligns better with the goal of reusing as much code as possible
> from the HDMI V1 IP,
> which is something you have been advocating since V1 of this patch
> quite some time ago
> and has shaped this patch.
>
> To me we are in a state that is clean and avoids branching in the hdmi
> common code.
> Would you reconsider and allow the use of that virtual device?
>
> Thx,
> Guillaume.

You can as well keep the DDC as a separated driver, but register in the HDMI v1 and
v2 driver at probe time.

Doing that, you won't need any devicetree node specific to any virtual device :-)

Cheers,
Angelo