diff mbox series

[v6,1/3] media: dt-bindings: media: add rockchip-vip

Message ID 6fa90df50c201dec70165c5138bc837f5a8829b5.1695981374.git.mehdi.djait@bootlin.com (mailing list archive)
State New, archived
Headers show
Series media: rockchip: Add a driver for Rockchip's camera interface | expand

Commit Message

Mehdi Djait Sept. 29, 2023, 10:08 a.m. UTC 
Add a documentation for the Rockchip Camera Interface controller
binding.

This controller can be found on platforms such as the PX30 or
RK1808, RK3128 and RK3288. The PX30 is the only platform
supported so far.

Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
---
 .../bindings/media/rockchip-vip.yaml          | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/rockchip-vip.yaml

Comments

Rob Herring Oct. 2, 2023, 5:18 p.m. UTC | #1 
On Fri, Sep 29, 2023 at 12:08:00PM +0200, Mehdi Djait wrote:
> Add a documentation for the Rockchip Camera Interface controller
> binding.
> 
> This controller can be found on platforms such as the PX30 or
> RK1808, RK3128 and RK3288. The PX30 is the only platform
> supported so far.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> ---
>  .../bindings/media/rockchip-vip.yaml          | 91 +++++++++++++++++++

filename should match compatible.

>  1 file changed, 91 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-vip.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/rockchip-vip.yaml b/Documentation/devicetree/bindings/media/rockchip-vip.yaml
> new file mode 100644
> index 000000000000..33c603209c39
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip-vip.yaml
> @@ -0,0 +1,91 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/rockchip-vip.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip VIP Camera Interface
> +
> +maintainers:
> +  - Mehdi Djait <mehdi.djait@bootlin.com>
> +
> +description: |-

Don't need '|-'.

> +  Rockchip Video Input Processor present on PX30, RK1808, RK3128 and RK3288

Write complete sentences.



> +
> +properties:
> +  compatible:
> +    const: rockchip,px30-vip

I see 4 SoCs listed, but only 1 compatible.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: ACLK
> +      - description: HCLK
> +      - description: PCLK
> +
> +  clock-names:
> +    items:
> +      - const: aclk
> +      - const: hclk
> +      - const: pclk
> +
> +  resets:
> +    items:
> +      - description: AXI
> +      - description: AHB
> +      - description: PCLK IN
> +
> +  reset-names:
> +    items:
> +      - const: axi
> +      - const: ahb
> +      - const: pclkin
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base

/schemas/graph.yaml#/properties/port

if there are no extra properties (such as defined in 
video-interfaces.yaml).

> +    description: A connection to a sensor or decoder
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/px30-cru.h>
> +    #include <dt-bindings/power/px30-power.h>
> +
> +    parent0: parent {

Drop unused labels.

> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        vip: vip@ff490000 {
> +            compatible = "rockchip,px30-vip";
> +            reg = <0x0 0xff490000 0x0 0x200>;
> +            interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&cru ACLK_CIF>, <&cru HCLK_CIF>, <&cru PCLK_CIF>;
> +            clock-names = "aclk", "hclk", "pclk";
> +            resets = <&cru SRST_CIF_A>, <&cru SRST_CIF_H>, <&cru SRST_CIF_PCLKIN>;
> +            reset-names = "axi", "ahb", "pclkin";
> +            power-domains = <&power PX30_PD_VI>;
> +            port {
> +                vip_in: endpoint {
> +                    remote-endpoint = <&tw9900_out>;
> +                };
> +            };
> +        };
> +    };
> +...
> -- 
> 2.41.0
>
Mehdi Djait Oct. 3, 2023, 8:40 a.m. UTC | #2 
Hello Rob,

Thank you for the review!

On Mon, Oct 02, 2023 at 12:18:01PM -0500, Rob Herring wrote:
> On Fri, Sep 29, 2023 at 12:08:00PM +0200, Mehdi Djait wrote:
> > Add a documentation for the Rockchip Camera Interface controller
> > binding.
> > 
> > This controller can be found on platforms such as the PX30 or
> > RK1808, RK3128 and RK3288. The PX30 is the only platform
> > supported so far.
> > 
> > Signed-off-by: Mehdi Djait <mehdi.djait@bootlin.com>
> > ---
> >  .../bindings/media/rockchip-vip.yaml          | 91 +++++++++++++++++++
> 
> filename should match compatible.
> 
> >  1 file changed, 91 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/rockchip-vip.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/rockchip-vip.yaml b/Documentation/devicetree/bindings/media/rockchip-vip.yaml
> > new file mode 100644
> > index 000000000000..33c603209c39
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/rockchip-vip.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/rockchip-vip.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Rockchip VIP Camera Interface
> > +
> > +maintainers:
> > +  - Mehdi Djait <mehdi.djait@bootlin.com>
> > +
> > +description: |-
> 
> Don't need '|-'.
> 
> > +  Rockchip Video Input Processor present on PX30, RK1808, RK3128 and RK3288
> 
> Write complete sentences.
> 
> 
> 
> > +
> > +properties:
> > +  compatible:
> > +    const: rockchip,px30-vip
> 
> I see 4 SoCs listed, but only 1 compatible.
> 

PX30 is the only SoC I have used to test the driver. What is the best
way to proceed here ? Change the description ?

ack for all the other comments. 

--
Kind Regards
Mehdi Djait
Krzysztof Kozlowski Oct. 3, 2023, 9:17 a.m. UTC | #3 
On 03/10/2023 10:40, Mehdi Djait wrote:

>>> +maintainers:
>>> +  - Mehdi Djait <mehdi.djait@bootlin.com>
>>> +
>>> +description: |-
>>
>> Don't need '|-'.
>>
>>> +  Rockchip Video Input Processor present on PX30, RK1808, RK3128 and RK3288
>>
>> Write complete sentences.
>>
>>
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: rockchip,px30-vip
>>
>> I see 4 SoCs listed, but only 1 compatible.
>>
> 
> PX30 is the only SoC I have used to test the driver. What is the best
> way to proceed here ? Change the description ?
> 

The best is to have bindings complete for all of the four SoCs, even if
you did not test the driver on them. The "complete" means
hardware-complete, so what the datasheet/spec is saying.

If anything is missing, could be added later.

You can change the description and drop other SoCs as well.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/rockchip-vip.yaml b/Documentation/devicetree/bindings/media/rockchip-vip.yaml
new file mode 100644
index 000000000000..33c603209c39
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rockchip-vip.yaml
@@ -0,0 +1,91 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/rockchip-vip.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip VIP Camera Interface
+
+maintainers:
+  - Mehdi Djait <mehdi.djait@bootlin.com>
+
+description: |-
+  Rockchip Video Input Processor present on PX30, RK1808, RK3128 and RK3288
+
+properties:
+  compatible:
+    const: rockchip,px30-vip
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: ACLK
+      - description: HCLK
+      - description: PCLK
+
+  clock-names:
+    items:
+      - const: aclk
+      - const: hclk
+      - const: pclk
+
+  resets:
+    items:
+      - description: AXI
+      - description: AHB
+      - description: PCLK IN
+
+  reset-names:
+    items:
+      - const: axi
+      - const: ahb
+      - const: pclkin
+
+  power-domains:
+    maxItems: 1
+
+  port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    description: A connection to a sensor or decoder
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/px30-cru.h>
+    #include <dt-bindings/power/px30-power.h>
+
+    parent0: parent {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        vip: vip@ff490000 {
+            compatible = "rockchip,px30-vip";
+            reg = <0x0 0xff490000 0x0 0x200>;
+            interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&cru ACLK_CIF>, <&cru HCLK_CIF>, <&cru PCLK_CIF>;
+            clock-names = "aclk", "hclk", "pclk";
+            resets = <&cru SRST_CIF_A>, <&cru SRST_CIF_H>, <&cru SRST_CIF_PCLKIN>;
+            reset-names = "axi", "ahb", "pclkin";
+            power-domains = <&power PX30_PD_VI>;
+            port {
+                vip_in: endpoint {
+                    remote-endpoint = <&tw9900_out>;
+                };
+            };
+        };
+    };
+...