| Message ID | 20220121081810.155500-6-jeanmichel.hautbois@ideasonboard.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Hans Verkuil |
| Headers |
Received: from vger.kernel.org ([23.128.96.18])
by www.linuxtv.org with esmtp (Exim 4.92)
(envelope-from <linux-media-owner@vger.kernel.org>)
id 1nAp8O-001F1E-Di; Fri, 21 Jan 2022 08:19:00 +0000
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S1379458AbiAUISy (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other);
Fri, 21 Jan 2022 03:18:54 -0500
Received: from perceval.ideasonboard.com ([213.167.242.64]:48796 "EHLO
perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S1379433AbiAUISZ (ORCPT
<rfc822;linux-media@vger.kernel.org>);
Fri, 21 Jan 2022 03:18:25 -0500
Received: from tatooine.ideasonboard.com (unknown
[IPv6:2a01:e0a:169:7140:d916:876:fa52:7241])
by perceval.ideasonboard.com (Postfix) with ESMTPSA id E81951273;
Fri, 21 Jan 2022 09:18:20 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;
s=mail; t=1642753101;
bh=g62rX4H5ojw3TBC5rBoXLBe/JwwWPY/fHbZ3UYFUP/Y=;
h=From:To:Cc:Subject:Date:In-Reply-To:References:From;
b=LTTO06JZu+GazJe00N9u2tRIXpZjop6ARaNurKJe/z9E+6UHxS/eBXA/rHNMX7hTA
Hc4nJRcdkdAtoxhDNWh+ttmT6ZWagSOaK38DtLMGBDaGFdDNhMko7+Hqhcx+YITRGJ
qYgHvBk6dDmNG3wlWmOsVTyorulHaLUqbN0/33oE=
From: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
To: jeanmichel.hautbois@ideasonboard.com
Cc: dave.stevenson@raspberrypi.com, devicetree@vger.kernel.org,
kernel-list@raspberrypi.com, laurent.pinchart@ideasonboard.com,
linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
linux-media@vger.kernel.org, linux-rpi-kernel@lists.infradead.org,
lukasz@jany.st, mchehab@kernel.org, naush@raspberrypi.com,
robh@kernel.org, tomi.valkeinen@ideasonboard.com
Subject: [RFC PATCH v2 5/7] ARM: dts: bcm2711: Add unicam CSI nodes
Date: Fri, 21 Jan 2022 09:18:08 +0100
Message-Id: <20220121081810.155500-6-jeanmichel.hautbois@ideasonboard.com>
X-Mailer: git-send-email 2.32.0
In-Reply-To: <20220121081810.155500-1-jeanmichel.hautbois@ideasonboard.com>
References: <20220121081810.155500-1-jeanmichel.hautbois@ideasonboard.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk
List-ID: <linux-media.vger.kernel.org>
X-Mailing-List: linux-media@vger.kernel.org
X-LSpam-Score: -7.5 (-------)
X-LSpam-Report: No,
score=-7.5 required=5.0 tests=BAYES_00=-1.9,DKIM_SIGNED=0.1,DKIM_VALID=-0.1,DKIM_VALID_AU=-0.1,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,RCVD_IN_DNSWL_HI=-5
autolearn=ham autolearn_force=no
|
| Series |
Add support for BCM2835 camera interface (unicam)
|
|
Commit Message
Jean-Michel Hautbois
Jan. 21, 2022, 8:18 a.m. UTC
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 <jeanmichel.hautbois@ideasonboard.com>
---
arch/arm/boot/dts/bcm2711.dtsi | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
Comments
Hi Jean-Michel, Thank you for the patch. On Fri, Jan 21, 2022 at 09:18:08AM +0100, Jean-Michel Hautbois wrote: > 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 <jeanmichel.hautbois@ideasonboard.com> > --- > 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" ? > + 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. > + status="disabled"; Missing spaces around the =. Same comment for the next node. > + }; > + > + 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>;
Hi Laurent and Jean-Michel On Fri, 21 Jan 2022 at 22:45, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Jean-Michel, > > Thank you for the patch. > > On Fri, Jan 21, 2022 at 09:18:08AM +0100, Jean-Michel Hautbois wrote: > > 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 <jeanmichel.hautbois@ideasonboard.com> > > --- > > 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". 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 > > + 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. > > > + status="disabled"; > > Missing spaces around the =. > > Same comment for the next node. > > > + }; > > + > > + 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
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: > > On Fri, Jan 21, 2022 at 09:18:08AM +0100, Jean-Michel Hautbois wrote: > > > 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 <jeanmichel.hautbois@ideasonboard.com> > > > --- > > > 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 > > > > + 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. > > > > > + status="disabled"; > > > > Missing spaces around the =. > > > > Same comment for the next node. > > > > > + }; > > > + > > > + 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>;
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 { + 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>; + status="disabled"; + }; + + 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>;