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

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

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