| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABnWg9uDki0ZtkxU1BPZq0ZU1mi4zFjasw+e3pQYb+Nv1MThLA@mail.gmail.com>
Date: Thu, 3 Nov 2022 08:17:12 -0700
From: Guillaume Ranquet <granquet@...libre.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Daniel Vetter <daniel@...ll.ch>,
Chun-Kuang Hu <chunkuang.hu@...nel.org>,
Chunfeng Yun <chunfeng.yun@...iatek.com>,
David Airlie <airlied@...il.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Jitao shi <jitao.shi@...iatek.com>, CK Hu <ck.hu@...iatek.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Vinod Koul <vkoul@...nel.org>,
Kishon Vijay Abraham I <kishon@...com>,
Matthias Brugger <matthias.bgg@...il.com>
Cc: stuart.lee@...iatek.com, linux-phy@...ts.infradead.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>,
linux-kernel@...r.kernel.org, mac.shen@...iatek.com,
linux-mediatek@...ts.infradead.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 02/12] dt-bindings: display: mediatek: add MT8195 hdmi bindings
On Thu, 03 Nov 2022 13:45, Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
>On 02/11/2022 09:31, Guillaume Ranquet wrote:
>> On Fri, 14 Oct 2022 18:08, Krzysztof Kozlowski
>> <krzysztof.kozlowski@...aro.org> 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@...libre.com>
>>>> ---
>>>> .../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@...iatek.com>
>>>> + - Jitao shi <jitao.shi@...iatek.com>
>>>> +
>>>> +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
>
Powered by blists - more mailing lists