[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0b19c544-f773-435e-9829-aaaa1c6daf7a@wolfvision.net>
Date: Mon, 24 Feb 2025 11:21:41 +0100
From: Michael Riesch <michael.riesch@...fvision.net>
To: Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc: Mehdi Djait <mehdi.djait@...ux.intel.com>,
Maxime Chevallier <maxime.chevallier@...tlin.com>,
Théo Lebrun <theo.lebrun@...tlin.com>,
Gerald Loacker <gerald.loacker@...fvision.net>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Mauro Carvalho Chehab <mchehab@...nel.org>, Rob Herring
<robh+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Heiko Stuebner <heiko@...ech.de>,
Kever Yang <kever.yang@...k-chips.com>,
Nicolas Dufresne <nicolas.dufresne@...labora.com>,
Sebastian Fricke <sebastian.fricke@...labora.com>,
Sebastian Reichel <sebastian.reichel@...labora.com>,
Paul Kocialkowski <paulk@...-base.io>,
Alexander Shiyan <eagle.alexander923@...il.com>,
Val Packett <val@...kett.cool>, Rob Herring <robh@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v4 03/11] media: dt-bindings: media: add bindings for
rockchip rk3568 vicap
Hi Sakari,
Thanks for the review.
On 2/21/25 15:10, Sakari Ailus wrote:
> Hi Michael,
>
> Thanks for the update.
>
> On Wed, Feb 19, 2025 at 11:16:34AM +0100, Michael Riesch wrote:
>> Add documentation for the Rockchip RK3568 Video Capture (VICAP) unit.
>>
>> Signed-off-by: Michael Riesch <michael.riesch@...fvision.net>
>> ---
>> .../bindings/media/rockchip,rk3568-vicap.yaml | 168 +++++++++++++++++++++
>> MAINTAINERS | 1 +
>> 2 files changed, 169 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
>> new file mode 100644
>> index 000000000000..3dc15efeb32e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
>> @@ -0,0 +1,168 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/media/rockchip,rk3568-vicap.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip RK3568 Video Capture (VICAP)
>> +
>> +maintainers:
>> + - Michael Riesch <michael.riesch@...fvision.net>
>> +
>> +description:
>> + The Rockchip RK3568 Video Capture (VICAP) block features a digital video
>> + port (DVP, a parallel video interface) and a MIPI CSI-2 port. It receives
>> + the data from camera sensors, video decoders, or other companion ICs and
>> + transfers it into system main memory by AXI bus.
>> +
>> +properties:
>> + compatible:
>> + const: rockchip,rk3568-vicap
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
>> + clocks:
>> + items:
>> + - description: ACLK
>> + - description: HCLK
>> + - description: DCLK
>> + - description: ICLK
>> +
>> + clock-names:
>> + items:
>> + - const: aclk
>> + - const: hclk
>> + - const: dclk
>> + - const: iclk
>> +
>> + rockchip,cif-clk-delaynum:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 127
>> + description:
>> + Delay the DVP path clock input to align the sampling phase, only valid
>> + in dual edge sampling mode.
>
> I suppose there's further documentation on this somewhere else? A reference
> would be nice.
I like your optimism :-) No, I am afraid this single sentence is all the
the RK3568 TRM has to say about it. I can add a reference to the TRM
page, but everyone who actually follows this reference will be
disappointed...
>> +
>> + iommus:
>> + maxItems: 1
>> +
>> + resets:
>> + items:
>> + - description: ARST
>> + - description: HRST
>> + - description: DRST
>> + - description: PRST
>> + - description: IRST
>> +
>> + reset-names:
>> + items:
>> + - const: arst
>> + - const: hrst
>> + - const: drst
>> + - const: prst
>> + - const: irst
>> +
>> + rockchip,grf:
>> + $ref: /schemas/types.yaml#/definitions/phandle
>> + description:
>> + Phandle to general register file used for video input block control.
>> +
>> + power-domains:
>> + maxItems: 1
>> +
>> + ports:
>> + $ref: /schemas/graph.yaml#/properties/ports
>> +
>> + properties:
>> + port@0:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description:
>> + The digital video port (DVP, a parallel video interface).
>> +
>> + properties:
>> + endpoint:
>> + $ref: video-interfaces.yaml#
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + bus-type:
>> + enum: [5, 6]
>> +
>> + required:
>> + - bus-type
>> +
>> + port@1:
>> + $ref: /schemas/graph.yaml#/$defs/port-base
>> + unevaluatedProperties: false
>> + description: The MIPI CSI-2 port.
>> +
>> + properties:
>> + endpoint:
>> + $ref: video-interfaces.yaml#
>> + unevaluatedProperties: false
>
> Don't you need things like data-lanes here? Or is this a single lane
> receiver?
This may be a bit confusing, and I probably should extend the
description a bit. This port/endpoint faces the MIPI CSI Host, which has
its own driver provided in patch 6. The connection in between is a link
with some internal format. Hence, no properties required.
This is the same issue as the one discussed in the other thread, since
the other end of this connection is discussed there. I'll fix the issue
on both ends using Rob's suggestion.
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clocks
>> + - ports
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/rk3568-cru.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + #include <dt-bindings/power/rk3568-power.h>
>> + #include <dt-bindings/media/video-interfaces.h>
>> +
>> + parent {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + vicap: video-capture@...e0000 {
>> + compatible = "rockchip,rk3568-vicap";
>> + reg = <0x0 0xfdfe0000 0x0 0x200>;
>> + interrupts = <GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>;
>> + assigned-clocks = <&cru DCLK_VICAP>;
>> + assigned-clock-rates = <300000000>;
>> + clocks = <&cru ACLK_VICAP>, <&cru HCLK_VICAP>,
>> + <&cru DCLK_VICAP>, <&cru ICLK_VICAP_G>;
>> + clock-names = "aclk", "hclk", "dclk", "iclk";
>> + iommus = <&vicap_mmu>;
>> + power-domains = <&power RK3568_PD_VI>;
>> + resets = <&cru SRST_A_VICAP>, <&cru SRST_H_VICAP>,
>> + <&cru SRST_D_VICAP>, <&cru SRST_P_VICAP>,
>> + <&cru SRST_I_VICAP>;
>> + reset-names = "arst", "hrst", "drst", "prst", "irst";
>> + rockchip,grf = <&grf>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + vicap_dvp: port@0 {
>> + reg = <0>;
>> +
>> + vicap_dvp_input: endpoint {
>> + bus-type = <MEDIA_BUS_TYPE_BT656>;
>> + bus-width = <16>;
>> + pclk-sample = <MEDIA_PCLK_SAMPLE_DUAL_EDGE>;
>> + remote-endpoint = <&it6801_output>;
>> + };
>> + };
>> +
>> + vicap_mipi: port@1 {
>> + reg = <1>;
>
> Where is the endpoint?
I'll add the endpoint in the example.
Regards,
Michael
>
>> + };
>> + };
>> + };
>> + };
>> +...
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index bbfaf35d50c6..cd8fa1afe5eb 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -20407,6 +20407,7 @@ M: Michael Riesch <michael.riesch@...fvision.net>
>> L: linux-media@...r.kernel.org
>> S: Maintained
>> F: Documentation/devicetree/bindings/media/rockchip,px30-vip.yaml
>> +F: Documentation/devicetree/bindings/media/rockchip,rk3568-vicap.yaml
>>
>> ROCKCHIP CRYPTO DRIVERS
>> M: Corentin Labbe <clabbe@...libre.com>
>>
>
Powered by blists - more mailing lists