| 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
| ||
|
Date: Wed, 2 Nov 2022 13:31:26 +0000
From: Guillaume Ranquet <granquet@...libre.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
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>,
Daniel Vetter <daniel@...ll.ch>
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 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.
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?
Thx,
Guillaume.
>> + compatible = "mediatek,mt8195-hdmi-ddc";
>> + mediatek,hdmi = <&hdmi0>;
>> + clocks = <&clk26m>;
>> + clock-names = "ddc";
>> + };
>> +
>> +...
>>
>
>Best regards,
>Krzysztof
>
Powered by blists - more mailing lists