[2/4] dt-bindings: media: rkisp1: move rockchip-isp1 bindings out of staging
diff mbox series

Message ID 20200402000234.226466-3-helen.koike@collabora.com
State New
Headers show
Series
  • move Rockchip ISP bindings out of staging / add ISP DT nodes for RK3399
Related show

Commit Message

Helen Koike April 2, 2020, 12:02 a.m. UTC
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%)

Comments

Johan Jonker April 2, 2020, 11:35 a.m. UTC | #1
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>;
>                       };
>                   };
>             };
>         };
>     };
Helen Koike April 2, 2020, 2:42 p.m. UTC | #2
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>;
>>                       };
>>                   };
>>             };
>>         };
>>     };

Patch
diff mbox series

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