linux-kernel - Re: [PATCH v4 2/5] dt-bindings: media: add mediatek ISP3.0 camsv

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <feb4e04f-8282-4b52-b785-07bf15008e06@linaro.org>
Date: Fri, 12 Jan 2024 08:34:45 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Julien Stephan <jstephan@...libre.com>
Cc: Phi-bang Nguyen <pnguyen@...libre.com>,
 Andy Hsieh <andy.hsieh@...iatek.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
 Florian Sylvestre <fsylvestre@...libre.com>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-mediatek@...ts.infradead.org, linux-media@...r.kernel.org,
 Louis Kuo <louis.kuo@...iatek.com>, Matthias Brugger
 <matthias.bgg@...il.com>, Mauro Carvalho Chehab <mchehab@...nel.org>,
 Paul Elder <paul.elder@...asonboard.com>, Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v4 2/5] dt-bindings: media: add mediatek ISP3.0 camsv

On 10/01/2024 15:14, Julien Stephan wrote:
> From: Phi-bang Nguyen <pnguyen@...libre.com>
> 
> This adds the bindings, for the ISP3.0 camsv module embedded in
> some Mediatek SoC, such as the mt8365
> 
> Signed-off-by: Phi-bang Nguyen <pnguyen@...libre.com>
> Signed-off-by: Julien Stephan <jstephan@...libre.com>
> Link: https://lore.kernel.org/r/20230807094940.329165-4-jstephan@baylibre.com
> ---
>  .../bindings/media/mediatek,mt8365-camsv.yaml | 109 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 110 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8365-camsv.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8365-camsv.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8365-camsv.yaml
> new file mode 100644
> index 000000000000..097b1ab6bc72
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mt8365-camsv.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2023 MediaTek, BayLibre
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/mediatek,mt8365-camsv.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek CAMSV 3.0
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart@...asonboard.com>
> +  - Julien Stephan <jstephan@...libre.com>
> +  - Andy Hsieh <andy.hsieh@...iatek.com>
> +
> +description:
> +  The CAMSV is a set of DMA engines connected to the SENINF CSI-2
> +  receivers. The number of CAMSVs depend on the SoC model.

DMA should not go to media, but to dma

> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8365-camsv
> +
> +  reg:
> +    items:
> +      - description: camsv base
> +      - description: img0 base
> +      - description: tg base
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: cam clock
> +      - description: camtg clock
> +      - description: camsv clock
> +
> +  clock-names:
> +    items:
> +      - const: cam
> +      - const: camtg
> +      - const: camsv
> +
> +  iommus:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: connection point for camsv0

This explains me nothing. What type of connection point? How does it fit
the pipeline going to the display?

It seems you represented DMA as some other device to make your drivers
easier... That's not how it works.

> +
> +    required:
> +      - port@0
> +
> +required:
> +  - compatible
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - iommus
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/clock/mediatek,mt8365-clk.h>
> +    #include <dt-bindings/memory/mediatek,mt8365-larb-port.h>
> +    #include <dt-bindings/power/mediatek,mt8365-power.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        camsv1: camsv@...50000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation



Best regards,
Krzysztof