Add a documentation for the Rockchip Camera Interface binding.
Signed-off-by: Mehdi Djait <[email protected]>
---
.../bindings/media/rockchip,px30-vip.yaml | 173 ++++++++++++++++++
1 file changed, 173 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
diff --git a/Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml b/Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
new file mode 100644
index 000000000000..580ed654000c
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
@@ -0,0 +1,173 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/rockchip,px30-vip.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip CIF Camera Interface
+
+maintainers:
+ - Mehdi Djait <[email protected]>
+
+description:
+ CIF is a camera interface present on some rockchip SoCs. It receives the data
+ from Camera sensor or CCIR656 encoder and transfers it into system main memory
+ by AXI bus.
+
+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
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description: input port on the parallel interface
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ bus-type:
+ enum: [5, 6]
+
+ required:
+ - bus-type
+
+ required:
+ - port@0
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/px30-cru.h>
+ #include <dt-bindings/display/sdtv-standards.h>
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/media/video-interfaces.h>
+ #include <dt-bindings/power/px30-power.h>
+
+ parent {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ video-capture@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>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ cif_in: endpoint {
+ remote-endpoint = <&tw9900_out>;
+ bus-type = <MEDIA_BUS_TYPE_BT656>;
+ };
+ };
+ };
+ };
+
+ composite_connector {
+ compatible = "composite-video-connector";
+ label = "tv";
+ sdtv-standards = <(SDTV_STD_PAL | SDTV_STD_NTSC)>;
+
+ port {
+ composite_to_tw9900: endpoint {
+ remote-endpoint = <&tw9900_to_composite>;
+ };
+ };
+ };
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ video-decoder@44 {
+ compatible = "techwell,tw9900";
+ reg = <0x44>;
+
+ vdd-supply = <&tw9900_supply>;
+ reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <0>;
+ tw9900_to_composite: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&composite_to_tw9900>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ tw9900_out: endpoint {
+ remote-endpoint = <&cif_in>;
+ };
+ };
+ };
+ };
+ };
+ };
+...
--
2.41.0
On Thu, 16 Nov 2023 12:04:38 +0100, Mehdi Djait wrote:
> Add a documentation for the Rockchip Camera Interface binding.
>
> Signed-off-by: Mehdi Djait <[email protected]>
> ---
> .../bindings/media/rockchip,px30-vip.yaml | 173 ++++++++++++++++++
> 1 file changed, 173 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/media/rockchip,px30-vip.example.dtb: /example-0/parent/i2c/video-decoder@44: failed to match any schema with compatible: ['techwell,tw9900']
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/a0af1d30e79fb1f2567297c951781996836d6db6.1700132457.git.mehdi.djait@bootlin.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On Thu, Nov 16, 2023 at 06:26:53AM -0600, Rob Herring wrote:
>
> On Thu, 16 Nov 2023 12:04:38 +0100, Mehdi Djait wrote:
> > Add a documentation for the Rockchip Camera Interface binding.
> >
> > Signed-off-by: Mehdi Djait <[email protected]>
> > ---
> > .../bindings/media/rockchip,px30-vip.yaml | 173 ++++++++++++++++++
> > 1 file changed, 173 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Documentation/devicetree/bindings/media/rockchip,px30-vip.example.dtb: /example-0/parent/i2c/video-decoder@44: failed to match any schema with compatible: ['techwell,tw9900']
There's a reviewed binding for this on the list at present:
https://lore.kernel.org/all/[email protected]/
Perhaps a poor choice however for an example in a another (unrelated?)
series.
Cheers,
Conor.
On Thu, Nov 16, 2023 at 12:04:38PM +0100, Mehdi Djait wrote:
> Add a documentation for the Rockchip Camera Interface binding.
>
> Signed-off-by: Mehdi Djait <[email protected]>
Provided the undocumented compatible in the example is resolved either
by sequencing, or by swapping out for another device:
Reviewed-by: Conor Dooley <[email protected]>
Cheers,
Conor.
Hi Mehdi,
On 11/16/23 12:04, Mehdi Djait wrote:
> Add a documentation for the Rockchip Camera Interface binding.
>
> Signed-off-by: Mehdi Djait <[email protected]>
> ---
> .../bindings/media/rockchip,px30-vip.yaml | 173 ++++++++++++++++++
> 1 file changed, 173 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml b/Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
> new file mode 100644
> index 000000000000..580ed654000c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
> @@ -0,0 +1,173 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/rockchip,px30-vip.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip CIF Camera Interface
May I suggest "Rockchip Camera Interface (CIF)"?
> +
> +maintainers:
> + - Mehdi Djait <[email protected]>
> +
> +description:
> + CIF is a camera interface present on some rockchip SoCs. It receives the data
s/rockchip/Rockchip
> + from Camera sensor or CCIR656 encoder and transfers it into system main memory
> + by AXI bus.
> +
> +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
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + properties:
> + port@0:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description: input port on the parallel interface
In more recent Rockchip SoCs this seems to be described as "DVP
interface (digital parallel input)". Maybe we could use this description
here as well?
> +
> + properties:
> + endpoint:
> + $ref: video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + bus-type:
> + enum: [5, 6]
Not sure whether this is possible, but if we could use the
MEDIA_BUS_TYPE_PARALLEL and MEDIA_BUS_TYPE_BT656 defines here it would
be way more descriptive.
> +
> + required:
> + - bus-type
> +
> + required:
> + - port@0
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/px30-cru.h>
> + #include <dt-bindings/display/sdtv-standards.h>
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/media/video-interfaces.h>
> + #include <dt-bindings/power/px30-power.h>
> +
> + parent {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + video-capture@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>;
Sort alphabetically: power-domains before resets.
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + cif_in: endpoint {
> + remote-endpoint = <&tw9900_out>;
> + bus-type = <MEDIA_BUS_TYPE_BT656>;
> + };
> + };
> + };
> + };
> +
> + composite_connector {
> + compatible = "composite-video-connector";
> + label = "tv";
> + sdtv-standards = <(SDTV_STD_PAL | SDTV_STD_NTSC)>;
> +
> + port {
> + composite_to_tw9900: endpoint {
> + remote-endpoint = <&tw9900_to_composite>;
> + };
> + };
> + };
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + video-decoder@44 {
> + compatible = "techwell,tw9900";
> + reg = <0x44>;
> +
> + vdd-supply = <&tw9900_supply>;
> + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
This goes before vdd-supply (alphabetical sorting). No need for blank
lines between the properties.
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0>;
I think reg property goes first, then the others. No blank lines between
properties, but one blank line between properties and nodes.
> + tw9900_to_composite: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&composite_to_tw9900>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
Same here.
> + tw9900_out: endpoint {
> + remote-endpoint = <&cif_in>;
> + };
> + };
> + };
> + };
> + };
> + };
> +...
With the inline comments addressed,
Reviewed-by: Michael Riesch <[email protected]>
Thanks for your efforts!
Best regards,
Michael