| 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: <f2856b8f-9465-2638-aabf-d2dda842766b@collabora.com>
Date: Wed, 25 May 2022 17:30:33 +0200
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Guillaume Ranquet <granquet@...libre.com>,
Chun-Kuang Hu <chunkuang.hu@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Matthias Brugger <matthias.bgg@...il.com>,
Chunfeng Yun <chunfeng.yun@...iatek.com>,
Kishon Vijay Abraham I <kishon@...com>,
Vinod Koul <vkoul@...nel.org>, Helge Deller <deller@....de>,
CK Hu <ck.hu@...iatek.com>, Jitao shi <jitao.shi@...iatek.com>
Cc: Markus Schneider-Pargmann <msp@...libre.com>,
dri-devel@...ts.freedesktop.org,
linux-mediatek@...ts.infradead.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-phy@...ts.infradead.org, linux-fbdev@...r.kernel.org
Subject: Re: [PATCH v10 02/21] dt-bindings: mediatek,dp: Add Display Port
binding
Il 23/05/22 12:47, Guillaume Ranquet ha scritto:
> From: Markus Schneider-Pargmann <msp@...libre.com>
>
> This controller is present on several mediatek hardware. Currently
> mt8195 and mt8395 have this controller without a functional difference,
> so only one compatible field is added.
>
> The controller can have two forms, as a normal display port and as an
> embedded display port.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@...libre.com>
> Signed-off-by: Guillaume Ranquet <granquet@...libre.com>
> ---
> .../display/mediatek/mediatek,dp.yaml | 99 +++++++++++++++++++
> 1 file changed, 99 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
> new file mode 100644
> index 000000000000..36ae0a6df299
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/mediatek/mediatek,dp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Display Port Controller
> +
> +maintainers:
> + - CK Hu <ck.hu@...iatek.com>
> + - Jitao shi <jitao.shi@...iatek.com>
> +
> +description: |
> + Device tree bindings for the MediaTek (embedded) Display Port controller
> + present on some MediaTek SoCs.
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt8195-dp-tx
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + items:
> + - description: faxi clock
> +
> + clock-names:
> + items:
> + - const: faxi
> +
> + power-domains:
> + maxItems: 1
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> + properties:
> + port@0:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Input endpoint of the controller, usually dp_intf
> +
> + port@1:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Output endpoint of the controller
> +
You should add port@0 (and port@1, probably) as required... with what you've done
here, you're saying that "ports" is required, but you're allowing it to be empty..
ports:
$ref: /schemas/graph.yaml#/properties/ports
properties:
port@0:
$ref: /schemas/graph.yaml#/properties/port
description: Input endpoint of the controller, usually dp_intf
port@1:
$ref: /schemas/graph.yaml#/properties/port
description: Output endpoint of the controller
required:
- port@0
- port@1
^^^ that's how it should look.
> + max-lanes:
> + maxItems: 1
> + description: maximum number of lanes supported by the hardware
> +
> + max-linkrate:
> + maxItems: 1
> + description: maximum link rate supported by the hardware
As you've put it (in the example below), the max-linkrate property wants a value
that corresponds to what you find in the HW registers... this is wrong.
Devicetree bindings should be generic and devicetrees shouldn't have hardware
specific bits inside, hence, please change this property to accept a link rate
specified in Mbps and also specify that in the description.
Thanks,
Angelo
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - ports
> + - max-lanes
> + - max-linkrate
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/power/mt8195-power.h>
> + edp_tx: edp_tx@...00000 {
> + compatible = "mediatek,mt8195-dp-tx";
> + reg = <0 0x1c500000 0 0x8000>;
> + interrupts = <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH 0>;
> + power-domains = <&spm MT8195_POWER_DOMAIN_EPD_TX>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&edp_pin>;
> + max-lanes = /bits/ 8 <4>;
> + max-linkrate = /bits/ 8 <0x1e>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + edp_in: endpoint {
> + remote-endpoint = <&dp_intf0_out>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + edp_out: endpoint {
> + remote-endpoint = <&panel_in>;
> + };
> + };
> + };
> + };
Powered by blists - more mailing lists