Hi both
On 15/02/2024 11:02, Laurent Pinchart wrote:
On Wed, Feb 14, 2024 at 05:37:17PM +0000, Conor Dooley wrote:
On Wed, Feb 14, 2024 at 04:28:25PM +0200, Laurent Pinchart wrote:
On Wed, Feb 14, 2024 at 02:19:03PM +0000, Daniel Scally wrote:
Add the yaml binding for ARM's Mali-C55 Image Signal Processor.
Acked-by: Nayden Kanchev <nayden.kanchev@xxxxxxx>
Signed-off-by: Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
---
Changes in v2:
- Added clocks information
- Fixed the warnings raised by Rob
.../bindings/media/arm,mali-c55.yaml | 77 +++++++++++++++++++
1 file changed, 77 insertions(+)
create mode 100644 Documentation/devicetree/bindings/media/arm,mali-c55.yaml
diff --git a/Documentation/devicetree/bindings/media/arm,mali-c55.yaml b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
new file mode 100644
index 000000000000..30038cfec3a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/arm,mali-c55.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/arm,mali-c55.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Mali-C55 Image Signal Processor
+
+maintainers:
+ - Daniel Scally <dan.scally@xxxxxxxxxxxxxxxx>
+ - Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>
+
+properties:
+ compatible:
+ const: arm,mali-c55
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ items:
+ - description: ISP video clock
I wonder if we need this clock. Granted, it's an input clock to the ISP,
but it's part of the input video bus. I don't expect anyone would ever
need to control it manually, it should be provided by the video source
automatically.
I'd say that if there's a clock controller providing this clock, even if
it is implicit in the video feed it's good to have here. Being able to
increment the refcount on that clock would be good, even if you don't
actually control it manually?
I don't expect there would be a clock controller to directly reference
in most cases. It depends a bit on where the clock domain crossing
between the source (often a CSI-2 receiver) and the ISP is. If it's
implemented in glue logic bundled with the ISP, which wouldn't be
described in DT as a separate node, then there's a higher chance we'll
have a clock controller for vclk. If it's implemented in the source,
vclk will just come from the source, which won't be listed as a clock
controller.
One option would be to make this clock optional, by moving it to the end
of the clocks list, and setting
minItems: 2
maxItems: 3
+ - description: ISP AXI clock
+ - description: ISP AHB-lite clock
These two other clocks look good to me.
+
+ clock-names:
+ items:
+ - const: vclk
+ - const: aclk
+ - const: hclk
Why not "video" "axi" "ahb-lite"? There's 3 useful letters between the
tree clock names you've provided - they're all clocks, so having "clk"
in them is just noise :)
As far as I understand, the names proposed by Dan come directly from the
IP core documentation.
This is the case, but I do take Conor's point that more descriptive names might be nicer - if I'm
honest I just didn't think about it particularly given "Xclk" is such a common name for them
already, but having been poked into thinking about it I do agree.
