diff mbox series

[v4,1/5] dt-bindings: media: add mediatek ISP3.0 sensor interface

Message ID 20240110141443.364655-2-jstephan@baylibre.com (mailing list archive)
State New, archived
Headers show
Series Add Mediatek ISP3.0 | expand

Commit Message

Julien Stephan Jan. 10, 2024, 2:14 p.m. UTC
From: Louis Kuo <louis.kuo@mediatek.com>

This adds the bindings, for the mediatek ISP3.0 SENINF module embedded in
some Mediatek SoC, such as the mt8365

Signed-off-by: Louis Kuo <louis.kuo@mediatek.com>
Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
Signed-off-by: Julien Stephan <jstephan@baylibre.com>
Link: https://lore.kernel.org/r/20230807094940.329165-2-jstephan@baylibre.com
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../media/mediatek,mt8365-seninf.yaml         | 259 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 266 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml

Comments

Laurent Pinchart Jan. 11, 2024, 8:15 a.m. UTC | #1
Hi Julien,

Thank you for the patch.

On Wed, Jan 10, 2024 at 03:14:38PM +0100, Julien Stephan wrote:
> From: Louis Kuo <louis.kuo@mediatek.com>
> 
> This adds the bindings, for the mediatek ISP3.0 SENINF module embedded in
> some Mediatek SoC, such as the mt8365
> 
> Signed-off-by: Louis Kuo <louis.kuo@mediatek.com>
> Signed-off-by: Phi-Bang Nguyen <pnguyen@baylibre.com>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> Link: https://lore.kernel.org/r/20230807094940.329165-2-jstephan@baylibre.com
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../media/mediatek,mt8365-seninf.yaml         | 259 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 266 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
> new file mode 100644
> index 000000000000..0a7b7d949df7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
> @@ -0,0 +1,259 @@
> +# 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-seninf.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Sensor Interface 3.0
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +  - Julien Stephan <jstephan@baylibre.com>
> +  - Andy Hsieh <andy.hsieh@mediatek.com>
> +
> +description:
> +  The ISP3.0 SENINF is the CSI-2 and parallel camera sensor interface found in
> +  multiple MediaTek SoCs. It can support up to three physical CSI-2
> +  input ports, configured in DPHY (2 or 4 data lanes) or CPHY depending on the soc.

s/soc/SoC/

And while at it, you can reflow the text to 80 columns.

> +  On the output side, SENINF can be connected either to CAMSV instance or
> +  to the internal ISP. CAMSV is used to bypass the internal ISP processing
> +  in order to connect either an external ISP, or a sensor (RAW, YUV).
> +
> +properties:
> +  compatible:
> +    const: mediatek,mt8365-seninf
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Seninf camsys clock
> +      - description: Seninf top mux clock
> +
> +  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
> +
> +  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>;
> +
> +          seninf: seninf@15040000 {
> +                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>;

Indentation issue, you need two more spaces here.

With these small issues fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +                            };
> +                      };
> +
> +                      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>;
> +                      };
> +
> +                };
> +          };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 41e0862cfa7d..4444e719cf8e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13611,6 +13611,13 @@ M:	Sean Wang <sean.wang@mediatek.com>
>  S:	Maintained
>  F:	drivers/char/hw_random/mtk-rng.c
>  
> +MEDIATEK ISP3.0 DRIVER
> +M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +M:	Julien Stephan <jstephan@baylibre.com>
> +M:	Andy Hsieh <andy.hsieh@mediatek.com>
> +S:	Supported
> +F:	Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
> +
>  MEDIATEK SMI DRIVER
>  M:	Yong Wu <yong.wu@mediatek.com>
>  L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)
Krzysztof Kozlowski Jan. 12, 2024, 7:32 a.m. UTC | #2
On 10/01/2024 15:14, Julien Stephan wrote:
> From: Louis Kuo <louis.kuo@mediatek.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?

> +
> +  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@15040000 {

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
Julien Stephan June 7, 2024, 8:52 a.m. UTC | #3
Le ven. 12 janv. 2024 à 08:32, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> a écrit :
>
> On 10/01/2024 15:14, Julien Stephan wrote:
> > From: Louis Kuo <louis.kuo@mediatek.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@15040000 {
>
> 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
>
Laurent Pinchart June 7, 2024, 2:41 p.m. UTC | #4
Hi Krzysztof,

On Fri, Jun 07, 2024 at 10:52:33AM +0200, Julien Stephan wrote:
> Le ven. 12 janv. 2024 à 08:32, Krzysztof Kozlowski a écrit :
> > On 10/01/2024 15:14, Julien Stephan wrote:
> > > From: Louis Kuo <louis.kuo@mediatek.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@15040000 {
> >
> > 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

The seninf is (mostly) a set of MIPI CSI-2 receivers. Would you prefer
'csi', 'mipi-csi', 'csi-2' or any other name ?

There's also the camsv IP in the same series that needs a generic name.
I really don't know what to propose for it. Could you recommend
something that would make you happy ?

On a side note, that document lacks appropriate generic names for lots
of building blocks found in recent SoCs, it would be nice to get it
updated. You will eventually get better quality DT patches then :-)

> > > +                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
> >
> > > +                };
Krzysztof Kozlowski June 10, 2024, 7:39 a.m. UTC | #5
On 07/06/2024 10:52, Julien Stephan wrote:
> Le ven. 12 janv. 2024 à 08:32, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> a écrit :
>>
>> On 10/01/2024 15:14, Julien Stephan wrote:
>>> From: Louis Kuo <louis.kuo@mediatek.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

So it is fixed per soc? Then make it fixed per soc.

Best regards,
Krzysztof
Krzysztof Kozlowski June 10, 2024, 7:54 a.m. UTC | #6
On 07/06/2024 16:41, Laurent Pinchart wrote:
> Hi Krzysztof,
> 
> On Fri, Jun 07, 2024 at 10:52:33AM +0200, Julien Stephan wrote:
>> Le ven. 12 janv. 2024 à 08:32, Krzysztof Kozlowski a écrit :
>>> On 10/01/2024 15:14, Julien Stephan wrote:

Eeeh? January?

...

>>>
>>>> +
>>>> +          seninf: seninf@15040000 {
>>>
>>> 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
> 
> The seninf is (mostly) a set of MIPI CSI-2 receivers. Would you prefer
> 'csi', 'mipi-csi', 'csi-2' or any other name ?

csi@ works for me

> 
> There's also the camsv IP in the same series that needs a generic name.
> I really don't know what to propose for it. Could you recommend
> something that would make you happy ?

Sorry,that's almost half year old thread. Not present in my inbox.

> 
> On a side note, that document lacks appropriate generic names for lots
> of building blocks found in recent SoCs, it would be nice to get it
> updated. You will eventually get better quality DT patches then :-)


The list grew recently, so just add something there. But it is okay if
some name is not really generic, it's just recommendation.


Best regards,
Krzysztof
Laurent Pinchart June 10, 2024, 8:54 a.m. UTC | #7
Hi Krzysztof,

On Mon, Jun 10, 2024 at 09:54:03AM +0200, Krzysztof Kozlowski wrote:
> On 07/06/2024 16:41, Laurent Pinchart wrote:
> > On Fri, Jun 07, 2024 at 10:52:33AM +0200, Julien Stephan wrote:
> >> Le ven. 12 janv. 2024 à 08:32, Krzysztof Kozlowski a écrit :
> >>> On 10/01/2024 15:14, Julien Stephan wrote:
> 
> Eeeh? January?
> 
> ...
> 
> >>>
> >>>> +
> >>>> +          seninf: seninf@15040000 {
> >>>
> >>> 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
> > 
> > The seninf is (mostly) a set of MIPI CSI-2 receivers. Would you prefer
> > 'csi', 'mipi-csi', 'csi-2' or any other name ?
> 
> csi@ works for me
> 
> > There's also the camsv IP in the same series that needs a generic name.
> > I really don't know what to propose for it. Could you recommend
> > something that would make you happy ?
> 
> Sorry,that's almost half year old thread. Not present in my inbox.

I remember someone presenting a talk titled "Beginner Linux kernel
maintainer's toolbox" in Prague last year. The talk mentioned a tool
call b4. I highly recommend it ;-)

> > On a side note, that document lacks appropriate generic names for lots
> > of building blocks found in recent SoCs, it would be nice to get it
> > updated. You will eventually get better quality DT patches then :-)
> 
> The list grew recently, so just add something there. But it is okay if
> some name is not really generic, it's just recommendation.

OK, then I think we can go for 'csi' for seninf, and keep 'camsv'.
Krzysztof Kozlowski June 10, 2024, 10:18 a.m. UTC | #8
On 10/06/2024 10:54, Laurent Pinchart wrote:
>>
>>> There's also the camsv IP in the same series that needs a generic name.
>>> I really don't know what to propose for it. Could you recommend
>>> something that would make you happy ?
>>
>> Sorry,that's almost half year old thread. Not present in my inbox.
> 
> I remember someone presenting a talk titled "Beginner Linux kernel
> maintainer's toolbox" in Prague last year. The talk mentioned a tool
> call b4. I highly recommend it ;-)

Wouldn't solve it. I would need to download the thread and import to
mailbox (several clicks needed) or open in some other tool just to see
the email. Or find it on lore.kernel.org - anyway just not convenient.

Best regards,
Krzysztof
Conor Dooley June 10, 2024, 4:10 p.m. UTC | #9
On Mon, Jun 10, 2024 at 12:18:12PM +0200, Krzysztof Kozlowski wrote:
> On 10/06/2024 10:54, Laurent Pinchart wrote:
> >>
> >>> There's also the camsv IP in the same series that needs a generic name.
> >>> I really don't know what to propose for it. Could you recommend
> >>> something that would make you happy ?
> >>
> >> Sorry,that's almost half year old thread. Not present in my inbox.
> > 
> > I remember someone presenting a talk titled "Beginner Linux kernel
> > maintainer's toolbox" in Prague last year. The talk mentioned a tool
> > call b4. I highly recommend it ;-)
> 
> Wouldn't solve it. I would need to download the thread and import to
> mailbox (several clicks needed) or open in some other tool just to see
> the email. Or find it on lore.kernel.org - anyway just not convenient.

Apparently you can use b4 from within mutt to populate threads, but I am
yet to figure out if it works with non-local maildirs:
https://b4.docs.kernel.org/en/latest/maintainer/mbox.html#using-with-mutt
Conor Dooley June 10, 2024, 4:19 p.m. UTC | #10
On Mon, Jun 10, 2024 at 05:10:51PM +0100, Conor Dooley wrote:
> On Mon, Jun 10, 2024 at 12:18:12PM +0200, Krzysztof Kozlowski wrote:
> > On 10/06/2024 10:54, Laurent Pinchart wrote:
> > >>
> > >>> There's also the camsv IP in the same series that needs a generic name.
> > >>> I really don't know what to propose for it. Could you recommend
> > >>> something that would make you happy ?
> > >>
> > >> Sorry,that's almost half year old thread. Not present in my inbox.
> > > 
> > > I remember someone presenting a talk titled "Beginner Linux kernel
> > > maintainer's toolbox" in Prague last year. The talk mentioned a tool
> > > call b4. I highly recommend it ;-)
> > 
> > Wouldn't solve it. I would need to download the thread and import to
> > mailbox (several clicks needed) or open in some other tool just to see
> > the email. Or find it on lore.kernel.org - anyway just not convenient.
> 
> Apparently you can use b4 from within mutt to populate threads, but I am
> yet to figure out if it works with non-local maildirs:
> https://b4.docs.kernel.org/en/latest/maintainer/mbox.html#using-with-mutt

That should read "how to make it work", I know it doesn't work like
documented there with non-local mailboxes.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml b/Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
new file mode 100644
index 000000000000..0a7b7d949df7
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
@@ -0,0 +1,259 @@ 
+# 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-seninf.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Sensor Interface 3.0
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+  - Julien Stephan <jstephan@baylibre.com>
+  - Andy Hsieh <andy.hsieh@mediatek.com>
+
+description:
+  The ISP3.0 SENINF is the CSI-2 and parallel camera sensor interface found in
+  multiple MediaTek SoCs. It can support up to three physical CSI-2
+  input ports, configured in DPHY (2 or 4 data lanes) or CPHY depending on the soc.
+  On the output side, SENINF can be connected either to CAMSV instance or
+  to the internal ISP. CAMSV is used to bypass the internal ISP processing
+  in order to connect either an external ISP, or a sensor (RAW, YUV).
+
+properties:
+  compatible:
+    const: mediatek,mt8365-seninf
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Seninf camsys clock
+      - description: Seninf top mux clock
+
+  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
+
+  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>;
+
+          seninf: seninf@15040000 {
+                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>;
+                      };
+
+                };
+          };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 41e0862cfa7d..4444e719cf8e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13611,6 +13611,13 @@  M:	Sean Wang <sean.wang@mediatek.com>
 S:	Maintained
 F:	drivers/char/hw_random/mtk-rng.c
 
+MEDIATEK ISP3.0 DRIVER
+M:	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
+M:	Julien Stephan <jstephan@baylibre.com>
+M:	Andy Hsieh <andy.hsieh@mediatek.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml
+
 MEDIATEK SMI DRIVER
 M:	Yong Wu <yong.wu@mediatek.com>
 L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)