Message ID | 20240110141443.364655-3-jstephan@baylibre.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Add Mediatek ISP3.0 | expand |
On 10/01/2024 15:14, Julien Stephan wrote: > From: Phi-bang Nguyen <pnguyen@baylibre.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@baylibre.com> > Signed-off-by: Julien Stephan <jstephan@baylibre.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@ideasonboard.com> > + - Julien Stephan <jstephan@baylibre.com> > + - Andy Hsieh <andy.hsieh@mediatek.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@15050000 { 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
On Fri, Jan 12, 2024 at 08:34:45AM +0100, Krzysztof Kozlowski wrote: > On 10/01/2024 15:14, Julien Stephan wrote: > > From: Phi-bang Nguyen <pnguyen@baylibre.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@baylibre.com> > > Signed-off-by: Julien Stephan <jstephan@baylibre.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@ideasonboard.com> > > + - Julien Stephan <jstephan@baylibre.com> > > + - Andy Hsieh <andy.hsieh@mediatek.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 They're not generic DMA engines. The CAMSV is a video capture device that includes a DMA engine, much like pretty much all the other video capture devices. > > + > > +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? The description seems wrong, it should state description: Connection to the SENINF output or something similar. > 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@15050000 { > > 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
On 12/01/2024 08:41, Laurent Pinchart wrote: > On Fri, Jan 12, 2024 at 08:34:45AM +0100, Krzysztof Kozlowski wrote: >> On 10/01/2024 15:14, Julien Stephan wrote: >>> From: Phi-bang Nguyen <pnguyen@baylibre.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@baylibre.com> >>> Signed-off-by: Julien Stephan <jstephan@baylibre.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@ideasonboard.com> >>> + - Julien Stephan <jstephan@baylibre.com> >>> + - Andy Hsieh <andy.hsieh@mediatek.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 > > They're not generic DMA engines. The CAMSV is a video capture device > that includes a DMA engine, much like pretty much all the other video > capture devices. OK, some more explanation would be useful in description. > >>> + >>> +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? > > The description seems wrong, it should state > > description: Connection to the SENINF output > > or something similar. I am still not sure whether DMA engine should be connected via graphs. Best regards, Krzysztof
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@ideasonboard.com> + - Julien Stephan <jstephan@baylibre.com> + - Andy Hsieh <andy.hsieh@mediatek.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. + +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 + + 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@15050000 { + compatible = "mediatek,mt8365-camsv"; + reg = <0 0x15050000 0 0x0040>, + <0 0x15050208 0 0x0020>, + <0 0x15050400 0 0x0100>; + interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_LOW>; + clocks = <&camsys CLK_CAM>, + <&camsys CLK_CAMTG>, + <&camsys CLK_CAMSV0>; + clock-names = "cam", "camtg", "camsv"; + iommus = <&iommu M4U_PORT_CAM_IMGO>; + power-domains = <&spm MT8365_POWER_DOMAIN_CAM>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@0 { + reg = <0>; + camsv1_endpoint: endpoint { + remote-endpoint = <&seninf_camsv1_endpoint>; + }; + }; + }; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 4444e719cf8e..3ea2158864e1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13616,6 +13616,7 @@ 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-camsv.yaml F: Documentation/devicetree/bindings/media/mediatek,mt8365-seninf.yaml MEDIATEK SMI DRIVER