| Message ID | 20200402000234.226466-3-helen.koike@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | move Rockchip ISP bindings out of staging / add ISP DT nodes for RK3399 | expand |
Hi Helen, > # SPDX-License-Identifier: (GPL-2.0+ OR MIT) > %YAML 1.2 > --- > $id: http://devicetree.org/schemas/media/rockchip-isp1.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > title: Rockchip SoC Image Signal Processing unit v1 Where do we need 'v1' for? Is there a 'v2'? > > maintainers: > - Helen Koike <helen.koike@collabora.com> > > description: | > Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs > which contains image processing, scaling, and compression functions. > > properties: > compatible: > const: rockchip,rk3399-cif-isp > > reg: > maxItems: 1 > > interrupts: > maxItems: 1 > > iommus: > maxItems: 1 > > power-domains: > maxItems: 1 > > phys: > maxItems: 1 > description: phandle for the PHY port > > phy-names: > const: dphy > > clocks: > items: > - description: ISP clock > - description: ISP AXI clock clock > - description: ISP AXI clock wrapper clock > - description: ISP AHB clock clock > - description: ISP AHB wrapper clock > > clock-names: > items: > - const: clk_isp > - const: aclk_isp > - const: aclk_isp_wrap > - const: hclk_isp > - const: hclk_isp_wrap > > # See ./video-interfaces.txt for details > ports: > type: object > additionalProperties: false > > properties: > "#address-cells": > const: 1 > > "#size-cells": > const: 0 > > port@0: > type: object > description: connection point for sensors at MIPI-DPHY RX0 > additionalProperties: false Nothing required here? > > properties: > "#address-cells": > const: 1 > > "#size-cells": > const: 0 > > reg: > const: 0 > > patternProperties: > endpoint: > type: object > additionalProperties: false > > properties: > reg: > maxItems: 1 > > data-lanes: > minItems: 1 > maxItems: 4 > > remote-endpoint: true > > required: > - port@0 The use of '@0' makes "#address-cells" and "#size-cells" also a requirement. - "#address-cells" - "#size-cells" > > required: > - compatible How about 'reg'? - reg > - interrupts > - clocks > - clock-names > - power-domains > - iommus > - phys > - phy-names > - ports > > additionalProperties: false > > examples: > - | > > #include <dt-bindings/clock/rk3399-cru.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/power/rk3399-power.h> > > parent0: parent@0 { > #address-cells = <2>; > #size-cells = <2>; > > isp0: isp0@ff910000 { > compatible = "rockchip,rk3399-cif-isp"; > reg = <0x0 0xff910000 0x0 0x4000>; > interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>; > clocks = <&cru SCLK_ISP0>, > <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>, > <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>; > clock-names = "clk_isp", > "aclk_isp", "aclk_isp_wrap", > "hclk_isp", "hclk_isp_wrap"; > power-domains = <&power RK3399_PD_ISP0>; > iommus = <&isp0_mmu>; > phys = <&dphy>; > phy-names = "dphy"; > > ports { > #address-cells = <1>; > #size-cells = <0>; > > port@0 { > #address-cells = <1>; > #size-cells = <0>; > reg = <0>; > > mipi_in_wcam: endpoint@0 { > reg = <0>; > remote-endpoint = <&wcam_out>; > data-lanes = <1 2>; > }; > > mipi_in_ucam: endpoint@1 { > reg = <1>; > remote-endpoint = <&ucam_out>; > data-lanes = <1>; > }; > }; > }; > }; > > i2c7: i2c@ff160000 { > clock-frequency = <400000>; > #address-cells = <1>; > #size-cells = <0>; Incomplete example. From i2c-rk3x.yaml: required: - compatible - reg - interrupts - clocks - clock-names > > wcam: camera@36 { > compatible = "ovti,ov5695"; > reg = <0x36>; > > port { > wcam_out: endpoint { > remote-endpoint = <&mipi_in_wcam>; > data-lanes = <1 2>; > }; > }; > }; > > ucam: camera@3c { > compatible = "ovti,ov2685"; > reg = <0x3c>; > > port { > ucam_out: endpoint { > remote-endpoint = <&mipi_in_ucam>; > data-lanes = <1>; > }; > }; > }; > }; > };
Hi Johan, Thanks for your review. On 4/2/20 8:35 AM, Johan Jonker wrote: > Hi Helen, > >> # SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> %YAML 1.2 >> --- >> $id: http://devicetree.org/schemas/media/rockchip-isp1.yaml# >> $schema: http://devicetree.org/meta-schemas/core.yaml# >> > >> title: Rockchip SoC Image Signal Processing unit v1 > > Where do we need 'v1' for? Is there a 'v2'? ISPv1 is the Rockchip's name for the IP block. > >> >> maintainers: >> - Helen Koike <helen.koike@collabora.com> >> >> description: | >> Rockchip ISP1 is the Camera interface for the Rockchip series of SoCs >> which contains image processing, scaling, and compression functions. >> >> properties: >> compatible: >> const: rockchip,rk3399-cif-isp >> >> reg: >> maxItems: 1 >> >> interrupts: >> maxItems: 1 >> >> iommus: >> maxItems: 1 >> >> power-domains: >> maxItems: 1 >> >> phys: >> maxItems: 1 >> description: phandle for the PHY port >> >> phy-names: >> const: dphy >> >> clocks: >> items: >> - description: ISP clock >> - description: ISP AXI clock clock >> - description: ISP AXI clock wrapper clock >> - description: ISP AHB clock clock >> - description: ISP AHB wrapper clock >> >> clock-names: >> items: >> - const: clk_isp >> - const: aclk_isp >> - const: aclk_isp_wrap >> - const: hclk_isp >> - const: hclk_isp_wrap >> >> # See ./video-interfaces.txt for details >> ports: >> type: object >> additionalProperties: false >> >> properties: >> "#address-cells": >> const: 1 >> >> "#size-cells": >> const: 0 >> >> port@0: >> type: object >> description: connection point for sensors at MIPI-DPHY RX0 > >> additionalProperties: false > > Nothing required here? I was thinking that if there is no endpoint, then nothing is required. But if there is, then #address-cells, #size-cells and reg are. I guess I can just add them as required. I'll add it in the patchseries. > >> >> properties: >> "#address-cells": >> const: 1 >> >> "#size-cells": >> const: 0 >> >> reg: >> const: 0 >> >> patternProperties: >> endpoint: >> type: object >> additionalProperties: false >> >> properties: >> reg: >> maxItems: 1 >> >> data-lanes: >> minItems: 1 >> maxItems: 4 >> >> remote-endpoint: true >> >> required: > >> - port@0 > > The use of '@0' makes "#address-cells" and "#size-cells" also a requirement. > > - "#address-cells" > - "#size-cells" Ok, I'll add it. > >> >> required: >> - compatible > > How about 'reg'? > > - reg ack, I'll add another patch in the series fixing this. > >> - interrupts >> - clocks >> - clock-names >> - power-domains >> - iommus >> - phys >> - phy-names >> - ports >> >> additionalProperties: false >> >> examples: >> - | >> >> #include <dt-bindings/clock/rk3399-cru.h> >> #include <dt-bindings/interrupt-controller/arm-gic.h> >> #include <dt-bindings/power/rk3399-power.h> >> >> parent0: parent@0 { >> #address-cells = <2>; >> #size-cells = <2>; >> >> isp0: isp0@ff910000 { >> compatible = "rockchip,rk3399-cif-isp"; >> reg = <0x0 0xff910000 0x0 0x4000>; >> interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH 0>; >> clocks = <&cru SCLK_ISP0>, >> <&cru ACLK_ISP0>, <&cru ACLK_ISP0_WRAPPER>, >> <&cru HCLK_ISP0>, <&cru HCLK_ISP0_WRAPPER>; >> clock-names = "clk_isp", >> "aclk_isp", "aclk_isp_wrap", >> "hclk_isp", "hclk_isp_wrap"; >> power-domains = <&power RK3399_PD_ISP0>; >> iommus = <&isp0_mmu>; >> phys = <&dphy>; >> phy-names = "dphy"; >> >> ports { >> #address-cells = <1>; >> #size-cells = <0>; >> >> port@0 { >> #address-cells = <1>; >> #size-cells = <0>; >> reg = <0>; >> >> mipi_in_wcam: endpoint@0 { >> reg = <0>; >> remote-endpoint = <&wcam_out>; >> data-lanes = <1 2>; >> }; >> >> mipi_in_ucam: endpoint@1 { >> reg = <1>; >> remote-endpoint = <&ucam_out>; >> data-lanes = <1>; >> }; >> }; >> }; >> }; >> > >> i2c7: i2c@ff160000 { >> clock-frequency = <400000>; >> #address-cells = <1>; >> #size-cells = <0>; > > Incomplete example. > From i2c-rk3x.yaml: > > required: > - compatible > - reg > - interrupts > - clocks > - clock-names The idea was to exemplify how to connect to the sensor nodes below. But I don't see a problem adding a complete i2c example, I'll add it. Thanks Helen > >> >> wcam: camera@36 { >> compatible = "ovti,ov5695"; >> reg = <0x36>; >> >> port { >> wcam_out: endpoint { >> remote-endpoint = <&mipi_in_wcam>; >> data-lanes = <1 2>; >> }; >> }; >> }; >> >> ucam: camera@3c { >> compatible = "ovti,ov2685"; >> reg = <0x3c>; >> >> port { >> ucam_out: endpoint { >> remote-endpoint = <&mipi_in_ucam>; >> data-lanes = <1>; >> }; >> }; >> }; >> }; >> };
diff --git a/drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml b/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
similarity index 100%
rename from drivers/staging/media/rkisp1/Documentation/devicetree/bindings/media/rockchip-isp1.yaml
rename to Documentation/devicetree/bindings/media/rockchip-isp1.yaml
Move rkisp1 bindings to Documentation/devicetree/bindings/media Signed-off-by: Helen Koike <helen.koike@collabora.com> --- .../devicetree/bindings/media/rockchip-isp1.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {drivers/staging/media/rkisp1/Documentation => Documentation}/devicetree/bindings/media/rockchip-isp1.yaml (100%)