2022-02-23 14:56:56

by Jean-Michel Hautbois

[permalink] [raw]
Subject: [PATCH v5.1 2/2] ARM: dts: bcm2711: Add unicam CSI nodes

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

Thanks to Stefan Wahren for his guidance on how to deal with different
RPi variants.

Signed-off-by: Jean-Michel Hautbois <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
arch/arm/boot/dts/bcm2711.dtsi | 8 ++++++++
arch/arm/boot/dts/bcm2835-rpi.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/bcm283x.dtsi | 22 ++++++++++++++++++++++
3 files changed, 44 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2711.dtsi b/arch/arm/boot/dts/bcm2711.dtsi
index dff18fc9a906..53dbd5d00c3a 100644
--- a/arch/arm/boot/dts/bcm2711.dtsi
+++ b/arch/arm/boot/dts/bcm2711.dtsi
@@ -1036,6 +1036,14 @@ &rmem {
#address-cells = <2>;
};

+&csi0 {
+ interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+};
+
+&csi1 {
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+};
+
&cma {
/*
* arm64 reserves the CMA by default somewhere in ZONE_DMA32,
diff --git a/arch/arm/boot/dts/bcm2835-rpi.dtsi b/arch/arm/boot/dts/bcm2835-rpi.dtsi
index 8f988dd253fc..d339f62bdc8f 100644
--- a/arch/arm/boot/dts/bcm2835-rpi.dtsi
+++ b/arch/arm/boot/dts/bcm2835-rpi.dtsi
@@ -40,6 +40,20 @@ vchiq: mailbox@7e00b840 {
};
};

+&csi0 {
+ clocks = <&clocks BCM2835_CLOCK_CAM0>,
+ <&firmware_clocks 4>;
+ clock-names = "lp", "vpu";
+ power-domains = <&power RPI_POWER_DOMAIN_UNICAM0>;
+};
+
+&csi1 {
+ clocks = <&clocks BCM2835_CLOCK_CAM1>,
+ <&firmware_clocks 4>;
+ clock-names = "lp", "vpu";
+ power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>;
+};
+
&gpio {
pinctrl-names = "default";

diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi
index c113661a6668..7b96e1707024 100644
--- a/arch/arm/boot/dts/bcm283x.dtsi
+++ b/arch/arm/boot/dts/bcm283x.dtsi
@@ -456,6 +456,28 @@ dsi1: dsi@7e700000 {
status = "disabled";
};

+ csi0: csi@7e800000 {
+ compatible = "brcm,bcm2835-unicam";
+ reg = <0x7e800000 0x800>,
+ <0x7e802000 0x4>;
+ reg-names = "unicam", "cmi";
+ interrupts = <2 6>;
+ status = "disabled";
+ port {
+ };
+ };
+
+ csi1: csi@7e801000 {
+ compatible = "brcm,bcm2835-unicam";
+ reg = <0x7e801000 0x800>,
+ <0x7e802004 0x4>;
+ reg-names = "unicam", "cmi";
+ interrupts = <2 7>;
+ status = "disabled";
+ port {
+ };
+ };
+
i2c1: i2c@7e804000 {
compatible = "brcm,bcm2835-i2c";
reg = <0x7e804000 0x1000>;
--
2.32.0


2022-02-24 17:21:34

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v5.1 2/2] ARM: dts: bcm2711: Add unicam CSI nodes

Hi Jean-Michel,

the version v5.1 is a little bit confusing, because it looks like to be
applied to stable linux-5.1, which is not intended.

Am 23.02.22 um 15:34 schrieb Jean-Michel Hautbois:
> Add both MIPI CSI-2 nodes in the core bcm2711 tree. Use the 3-cells
> interrupt declaration, corresponding clocks and default as disabled.
>
> Thanks to Stefan Wahren for his guidance on how to deal with different
> RPi variants.

Can you please explain why you split these patches from the original series?

I didn't had the time to test, but applying these patches would
"disable" bcm2835-camera driver?

Best regards
Stefan


2022-02-24 17:44:54

by Jean-Michel Hautbois

[permalink] [raw]
Subject: Re: [PATCH v5.1 2/2] ARM: dts: bcm2711: Add unicam CSI nodes

Hi Stefan,

On 24/02/2022 18:03, Stefan Wahren wrote:
> Hi Jean-Michel,
>
> the version v5.1 is a little bit confusing, because it looks like to be
> applied to stable linux-5.1, which is not intended.

Sorry for the confusion, I don't want to send a full v6 now, as it will
need to be rebased on top of the multiplexed streams series in its v11
which is not yet posted on the ML :-).

>
> Am 23.02.22 um 15:34 schrieb Jean-Michel Hautbois:
>> Add both MIPI CSI-2 nodes in the core bcm2711 tree. Use the 3-cells
>> interrupt declaration, corresponding clocks and default as disabled.
>>
>> Thanks to Stefan Wahren for his guidance on how to deal with different
>> RPi variants.
>
> Can you please explain why you split these patches from the original series?

Because the firmware clocks are independent from the csi nodes and so,
it sounded logical to split it in half ?

>
> I didn't had the time to test, but applying these patches would
> "disable" bcm2835-camera driver?

Wasn't it already the case ? It is intended, we don't want those to be
"okay" by default ? Or do I miss your point maybe (probably :-)) ?

Thanks,
JM

>
> Best regards
> Stefan
>
>

2022-02-25 01:37:51

by Stefan Wahren

[permalink] [raw]
Subject: Re: [PATCH v5.1 2/2] ARM: dts: bcm2711: Add unicam CSI nodes

Hi Jean-Michel,

Am 24.02.22 um 18:07 schrieb Jean-Michel Hautbois:
> Hi Stefan,
>
> On 24/02/2022 18:03, Stefan Wahren wrote:
>> Hi Jean-Michel,
>>
>> the version v5.1 is a little bit confusing, because it looks like to be
>> applied to stable linux-5.1, which is not intended.
>
> Sorry for the confusion, I don't want to send a full v6 now, as it
> will need to be rebased on top of the multiplexed streams series in
> its v11 which is not yet posted on the ML :-).
>
>>
>> Am 23.02.22 um 15:34 schrieb Jean-Michel Hautbois:
>>> Add both MIPI CSI-2 nodes in the core bcm2711 tree. Use the 3-cells
>>> interrupt declaration, corresponding clocks and default as disabled.
>>>
>>> Thanks to Stefan Wahren for his guidance on how to deal with different
>>> RPi variants.
>>
>> Can you please explain why you split these patches from the original
>> series?
>
> Because the firmware clocks are independent from the csi nodes and so,
> it sounded logical to split it in half ?
I don't think this should be split, because these belong together
(dt-binding, driver, device tree changes).
>
>>
>> I didn't had the time to test, but applying these patches would
>> "disable" bcm2835-camera driver?
>
> Wasn't it already the case ? It is intended, we don't want those to be
> "okay" by default ? Or do I miss your point maybe (probably :-)) ?

With these 2 patches applied, both drivers won't work which is a
regression. Yes, it would be the best to have unicam disabled per
default and some kind of imx219 overlay should enable it. But at least
the unexpected (not for you or me but all the other reviewer)
consequence should have been in the commit log.

From my understand the unicam driver is not a fully replacement for the
bcm2835-camera or am i wrong?

Best regards

>
> Thanks,
> JM
>
>>
>> Best regards
>> Stefan
>>
>>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel