[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEHHSvYt-aqFahi=B_si=duJH8xDgy_9nndgR-P0+U5THX69uw@mail.gmail.com>
Date: Fri, 7 Jun 2024 10:52:33 +0200
From: Julien Stephan <jstephan@...libre.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: Louis Kuo <louis.kuo@...iatek.com>, Phi-Bang Nguyen <pnguyen@...libre.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.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>, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
linux-media@...r.kernel.org, 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 1/5] dt-bindings: media: add mediatek ISP3.0 sensor interface
Le ven. 12 janv. 2024 à 08:32, Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> a écrit :
>
> On 10/01/2024 15:14, Julien Stephan wrote:
> > From: Louis Kuo <louis.kuo@...iatek.com>
> >
> > This adds the bindings, for the mediatek ISP3.0 SENINF module embedded in
> > some Mediatek SoC, such as the mt8365
> >
>
> ...
>
> > + clock-names:
> > + items:
> > + - const: camsys
> > + - const: top_mux
> > +
> > + phys:
> > + minItems: 1
> > + maxItems: 4
> > + description:
> > + phandle to the PHYs connected to CSI0/A, CSI1, CSI2 and CSI0B
> > +
> > + phy-names:
> > + minItems: 1
> > + items:
> > + - const: csi0
> > + - const: csi1
> > + - const: csi2
> > + - const: csi0b
>
> Why one hardware has flexible number of phys?
Hi Krzysztof,
seninf can have multiple port depending on the soc, each requiring its own phy
>
> > +
> > + ports:
> > + $ref: /schemas/graph.yaml#/properties/ports
> > +
> > + properties:
> > + port@0:
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > + unevaluatedProperties: false
> > + description: CSI0 or CSI0A port
> > +
> > + properties:
> > + endpoint:
> > + $ref: video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + maxItems: 1
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 4
> > +
> > + port@1:
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > + unevaluatedProperties: false
> > + description: CSI1 port
> > +
> > + properties:
> > + endpoint:
> > + $ref: video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + maxItems: 1
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 4
> > +
> > + port@2:
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > + unevaluatedProperties: false
> > + description: CSI2 port
> > +
> > + properties:
> > + endpoint:
> > + $ref: video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + maxItems: 1
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 4
> > +
> > + port@3:
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > + unevaluatedProperties: false
> > + description: CSI0B port
> > +
> > + properties:
> > + endpoint:
> > + $ref: video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + clock-lanes:
> > + maxItems: 1
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 2
> > +
> > + port@4:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: connection point for cam0
> > +
> > + port@5:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: connection point for cam1
> > +
> > + port@6:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: connection point for camsv0
> > +
> > + port@7:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: connection point for camsv1
> > +
> > + port@8:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: connection point for camsv2
> > +
> > + port@9:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: connection point for camsv3
> > +
> > + required:
> > + - port@0
> > + - port@1
> > + - port@2
> > + - port@3
> > + - port@4
> > + - port@5
> > + - port@6
> > + - port@7
> > + - port@8
> > + - port@9
> > +
> > +required:
> > + - compatible
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - power-domains
> > + - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/mediatek,mt8365-clk.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/phy/phy.h>
> > + #include <dt-bindings/power/mediatek,mt8365-power.h>
> > +
> > + soc {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
>
> Use 4 spaces for example indentation.
>
> > +
> > + seninf: seninf@...40000 {
>
> 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
>
>
> > + compatible = "mediatek,mt8365-seninf";
> > + reg = <0 0x15040000 0 0x6000>;
> > + interrupts = <GIC_SPI 210 IRQ_TYPE_LEVEL_LOW>;
> > + clocks = <&camsys CLK_CAM_SENIF>,
> > + <&topckgen CLK_TOP_SENIF_SEL>;
> > + clock-names = "camsys", "top_mux";
> > +
> > + power-domains = <&spm MT8365_POWER_DOMAIN_CAM>;
> > +
> > + phys = <&mipi_csi0 PHY_TYPE_DPHY>;
> > + phy-names = "csi0";
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + reg = <0>;
> > + seninf_in1: endpoint {
> > + clock-lanes = <2>;
> > + data-lanes = <1 3 0 4>;
> > + remote-endpoint = <&isp1_out>;
> > + };
> > + };
> > +
> > + port@1 {
> > + reg = <1>;
> > + };
> > +
> > + port@2 {
> > + reg = <2>;
> > + };
> > +
> > + port@3 {
> > + reg = <3>;
> > + };
> > +
> > + port@4 {
> > + reg = <4>;
> > + seninf_camsv1_endpoint: endpoint {
> > + remote-endpoint = <&camsv1_endpoint>;
> > + };
> > + };
> > +
> > + port@5 {
> > + reg = <5>;
> > + };
> > +
> > + port@6 {
> > + reg = <6>;
> > + };
> > +
> > + port@7 {
> > + reg = <7>;
> > + };
> > +
> > + port@8 {
> > + reg = <8>;
> > + };
> > +
> > + port@9 {
> > + reg = <9>;
> > + };
> > +
>
> Stray blank line
>
> > + };
>
>
>
> Best regards,
> Krzysztof
>
Powered by blists - more mailing lists