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

AngeloGioacchino Del Regno
2023-01-02T14:15:59Z

Il 02/01/23 14:38, Guillaume Ranquet ha scritto:
> On Mon, 26 Dec 2022 06:18, CK Hu (胡俊光) <ck.hu@mediatek.com> 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@baylibre.com>
>>> ---
>>>
>>
>> [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@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
>>> +
>>> + 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