Re: [RFC PATCH v2 5/7] ARM: dts: bcm2711: Add unicam CSI nodes

Hi Dave,

On Mon, Jan 24, 2022 at 12:31:34PM +0000, Dave Stevenson wrote:

On Fri, 21 Jan 2022 at 22:45, Laurent Pinchart wrote:

quoted

On Fri, Jan 21, 2022 at 09:18:08AM +0100, Jean-Michel Hautbois wrote:

quoted

Add both MIPI CSI-2 nodes in the core bcm2711 tree. Use the 3-cells
interrupt declaration, corresponding clocks and default as disabled.

Signed-off-by: Jean-Michel Hautbois <redacted>
---
 arch/arm/boot/dts/bcm2711.dtsi | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index dff18fc9a906..077141df7024 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi

@@ -3,6 +3,7 @@

 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/soc/bcm2835-pm.h>
+#include <dt-bindings/power/raspberrypi-power.h>

 / {
      compatible = "brcm,bcm2711";

@@ -293,6 +294,36 @@ hvs: hvs@7e400000 {
                      interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
              };

+             csi0: csi1@7e800000 {

The node name should be csi@7e800000, not csi1@7e800000. Now, this will
probably cause issues with the firmware that looks for csi1 (and csi0 ?)
to hand over control of the Unicam CSI-2 receiver to the kernel. I
wonder if this is something that could be handled by a firmware update,
to also recognize nodes named "csi" ?

It already looks for any node starting "csi". If you check the
downstream DT [1], then the nodes are "csi0: csi@7e800000" and "csi1:
csi@7e801000".

Oops, indeed. I think I was misled by
https://github.com/raspberrypi/linux/blob/rpi-5.10.y/Documentation/devicetree/bindings/media/bcm2835-unicam.txt
that mentions "csi0" and "csi1".

It's all good then. Jean-Michel, can you update the DT bindings in the
next iteration of the series to correct the DT node naming ?

There is no actual action required to hand the peripheral over to the
kernel, it just prevents the firmware from using it and causing
problems (it masks out the interrupt, and that's checked as part of
the firmware initialising the peripheral).

If using imx219 or one of the other sensors supported by the firmware,
"vcgencmd get_camera" should report that the sensor isn't detected,
and "sudo vcdbg log msg" should have a line similar to
"020174.613: camsubs: Ignoring camera 0 as unicam device not available"

  Dave

[1] https://github.com/raspberrypi/linux/blob/rpi-5.10.y/arch/arm/boot/dts/bcm270x.dtsi#L88

quoted

quoted

+                     compatible = "brcm,bcm2835-unicam";
+                     reg = <0x7e800000 0x800>,
+                           <0x7e802000 0x4>;
+                     interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+                     clocks = <&clocks BCM2835_CLOCK_CAM0>,
+                              <&firmware_clocks 4>;
+                     clock-names = "lp", "vpu";
+                     power-domains = <&power RPI_POWER_DOMAIN_UNICAM0>;
+                     #address-cells = <1>;
+                     #size-cells = <0>;
+                     #clock-cells = <1>;

Why do you need #address-cells, #size-cells and #clock-cells ? They're
not mentioned in the binding.

quoted

+                     status="disabled";

Missing spaces around the =.

Same comment for the next node.

quoted

+             };
+
+             csi1: csi1@7e801000 {
+                     compatible = "brcm,bcm2835-unicam";
+                     reg = <0x7e801000 0x800>,
+                           <0x7e802004 0x4>;
+                     interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+                     clocks = <&clocks BCM2835_CLOCK_CAM1>,
+                              <&firmware_clocks 4>;
+                     clock-names = "lp", "vpu";
+                     power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>;
+                     #address-cells = <1>;
+                     #size-cells = <0>;
+                     #clock-cells = <1>;
+                     status="disabled";
+             };
+
              pixelvalve3: pixelvalve@7ec12000 {
                      compatible = "brcm,bcm2711-pixelvalve3";
                      reg = <0x7ec12000 0x100>;

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel