V2:
- Adds R/B - Konrad
- Adds newline as flagged - Konrad
- Squashes patch#6 into patch#5 ammends commit log
- Following up on TPG comentary:
The CAMSS would not have populated device nodes prior to this series
save for the case a user had a D3 Engineering board.
Splitting out the sensor from the core board is correct w/r/t the
hardware/dts since few db410c have the mezzanine.
Once split we can enable the basic camss node by default and thus get the
TPG going for both the mezzanine and non-mezzanine cases.
Link: https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-08-07-db410c-rb3-camss-dts-v2
V1:
This is a series which updates the apq8016-sbc to fixup CAMSS support.
The first four patches fixup the current state of the ov5640 bindings for
the apq8016.
Following on from that we move the ov5640 sensor from the main apq8016-sbc
into a standalone mezzanine dts with an accompanying patch to enable the
sensor by default in the mezzaine. This makes sense since the D3
Engineering camera mezzanine is but one of a slew of camera mezzanines we
can attach here.
The final patch switches on CAMSS in the core apq8016-sbc allowing us to use
the test-pattern-generator TPG on apq8016-sbc with or without a camera mezzaine
attached. This to me is a good idea since it means we can test out and
verify the CAMSS on this board absent a camera mezzaine on the main apq8016
board.
Link: https://git.codelinaro.org/bryan.odonoghue/kernel/-/tree/linux-next-23-08-07-db410c-rb3-camss-dts
Bryan O'Donoghue (6):
arm64: dts: qcom: apq8016-sbc: Fix ov5640 regulator supply names
arm64: dts: qcom: apq8016-sbc: Fix ov5640 data-lanes declaration
arm64: dts: qcom: apq8016-sbc: Set ov5640 assigned-clock
arm64: dts: qcom: apq8016-sbc: Rename ov5640 enable-gpios to
powerdown-gpios
arm64: dts: qcom: apq8016-sbc-d3-camera-mezzanine: Move default ov5640
to a standalone dts
arm64: dts: qcom: apq8016-sbc: Enable camss for non-mezzanine cases
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../qcom/apq8016-sbc-d3-camera-mezzanine.dts | 55 +++++++++++++++++++
arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 40 +-------------
3 files changed, 57 insertions(+), 39 deletions(-)
create mode 100644 arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
--
2.39.2
At the moment we define a single ov5640 sensor in the apq8016-sbc and
disable that sensor.
The sensor mezzanine for this is a D3 Engineering Dual ov5640 mezzanine
card. Move the definition from the apq8016-sbc where it shouldn't be to a
standalone dts.
Enables the sensor by default, as we are adding a standalone mezzanine
structure.
Signed-off-by: Bryan O'Donoghue <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/Makefile | 1 +
.../qcom/apq8016-sbc-d3-camera-mezzanine.dts | 55 +++++++++++++++++++
arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 49 -----------------
3 files changed, 56 insertions(+), 49 deletions(-)
create mode 100644 arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index f15548dbfa56e..19016765ba4c6 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb
+dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc-d3-camera-mezzanine.dtb
dtb-$(CONFIG_ARCH_QCOM) += apq8039-t2.dtb
dtb-$(CONFIG_ARCH_QCOM) += apq8094-sony-xperia-kitakami-karin_windy.dtb
dtb-$(CONFIG_ARCH_QCOM) += apq8096-db820c.dtb
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
new file mode 100644
index 0000000000000..ef0e76e424898
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2023, Linaro Ltd.
+ */
+
+/dts-v1/;
+
+#include "apq8016-sbc.dts"
+
+&camss {
+ status = "okay";
+
+ ports {
+ port@0 {
+ reg = <0>;
+ csiphy0_ep: endpoint {
+ data-lanes = <0 2>;
+ remote-endpoint = <&ov5640_ep>;
+ status = "okay";
+ };
+ };
+ };
+};
+
+&cci {
+ status = "okay";
+};
+
+&cci_i2c0 {
+ camera_rear@3b {
+ compatible = "ovti,ov5640";
+ reg = <0x3b>;
+
+ powerdown-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&tlmm 35 GPIO_ACTIVE_LOW>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&camera_rear_default>;
+
+ clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
+ clock-names = "xclk";
+ assigned-clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
+ assigned-clock-rates = <23880000>;
+
+ DOVDD-supply = <&camera_vdddo_1v8>;
+ AVDD-supply = <&camera_vdda_2v8>;
+ DVDD-supply = <&camera_vddd_1v5>;
+
+ port {
+ ov5640_ep: endpoint {
+ data-lanes = <1 2>;
+ remote-endpoint = <&csiphy0_ep>;
+ };
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
index ddb19709a9eee..84641925f3329 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
@@ -250,55 +250,6 @@ &blsp_uart2 {
label = "LS-UART1";
};
-&camss {
- status = "okay";
- ports {
- port@0 {
- reg = <0>;
- csiphy0_ep: endpoint {
- data-lanes = <0 2>;
- remote-endpoint = <&ov5640_ep>;
- status = "okay";
- };
- };
- };
-};
-
-&cci {
- status = "okay";
-};
-
-&cci_i2c0 {
- camera_rear@3b {
- compatible = "ovti,ov5640";
- reg = <0x3b>;
-
- powerdown-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;
- reset-gpios = <&tlmm 35 GPIO_ACTIVE_LOW>;
- pinctrl-names = "default";
- pinctrl-0 = <&camera_rear_default>;
-
- clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
- clock-names = "xclk";
- assigned-clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
- assigned-clock-rates = <23880000>;
-
- DOVDD-supply = <&camera_vdddo_1v8>;
- AVDD-supply = <&camera_vdda_2v8>;
- DVDD-supply = <&camera_vddd_1v5>;
-
- /* No camera mezzanine by default */
- status = "disabled";
-
- port {
- ov5640_ep: endpoint {
- data-lanes = <1 2>;
- remote-endpoint = <&csiphy0_ep>;
- };
- };
- };
-};
-
&lpass {
status = "okay";
};
--
2.39.2
The ov5640 driver expects DOVDD, AVDD and DVDD as regulator supply names.
The ov5640 has depended on these names since the driver was committed
upstream in 2017. Similarly apq8016-sbc.dtsi has had completely different
regulator names since its own initial commit in 2020.
Perhaps the regulators were left on in previous 410c bootloaders. In any
case today on 6.5 we won't switch on the ov5640 without correctly naming
the regulators.
Fixes: 39e0ce6cd1bf ("arm64: dts: qcom: apq8016-sbc: Add CCI/Sensor nodes")
Signed-off-by: Bryan O'Donoghue <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
index f6eeb25988465..75b4e5ff7c95c 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
@@ -282,9 +282,9 @@ camera_rear@3b {
clock-names = "xclk";
clock-frequency = <23880000>;
- vdddo-supply = <&camera_vdddo_1v8>;
- vdda-supply = <&camera_vdda_2v8>;
- vddd-supply = <&camera_vddd_1v5>;
+ DOVDD-supply = <&camera_vdddo_1v8>;
+ AVDD-supply = <&camera_vdda_2v8>;
+ DVDD-supply = <&camera_vddd_1v5>;
/* No camera mezzanine by default */
status = "disabled";
--
2.39.2
The driver for the ov5640 doesn't do a set-rate, instead it expects the
clock to already be set at an appropriate rate.
Similarly the yaml for ov5640 doesn't understand clock-frequency. Convert
from clock-rate to assigned-clock and assigned-clock-rate to remediate.
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
index 0481a4a82090a..ada0777567623 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
@@ -280,7 +280,8 @@ camera_rear@3b {
clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
clock-names = "xclk";
- clock-frequency = <23880000>;
+ assigned-clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
+ assigned-clock-rates = <23880000>;
DOVDD-supply = <&camera_vdddo_1v8>;
AVDD-supply = <&camera_vdda_2v8>;
--
2.39.2
The yaml constraint for data-lanes is [1, 2] not [0, 2]. The driver itself
doesn't do anything with the data-lanes declaration save count the number
of specified data-lanes and calculate the link rate so, this change doesn't
have any functional side-effects.
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
index 75b4e5ff7c95c..0481a4a82090a 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
@@ -291,7 +291,7 @@ camera_rear@3b {
port {
ov5640_ep: endpoint {
- data-lanes = <0 2>;
+ data-lanes = <1 2>;
remote-endpoint = <&csiphy0_ep>;
};
};
--
2.39.2
When we have no camera mezzanine attached it is still possible to run the
test-pattern generator of the CSID block.
As an example:
media-ctl --reset
yavta --no-query -w '0x009f0903 1' /dev/v4l-subdev2
yavta --list /dev/v4l-subdev2
media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:UYVY8_1X16/1920x1080 field:none]'
media-ctl -l '"msm_csid0":1->"msm_ispif0":0[1]'
media-ctl -d /dev/media0 -V '"msm_ispif0":0[fmt:UYVY8_1X16/1920x1080 field:none]'
media-ctl -l '"msm_ispif0":1->"msm_vfe0_rdi0":0[1]'
media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:UYVY8_1X16/1920x1080]'
media-ctl -d /dev/media0 -p
yavta -B capture-mplane --capture=5 -n 5 -I -f UYVY -s 1920x1080 --file=TPG-UYVU-1920x1080-000-#.bin /dev/video0
Signed-off-by: Bryan O'Donoghue <[email protected]>
---
arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
index 84641925f3329..4f9d6f6ec1157 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
@@ -250,6 +250,16 @@ &blsp_uart2 {
label = "LS-UART1";
};
+&camss {
+ status = "okay";
+
+ ports {
+ port@0 {
+ reg = <0>;
+ };
+ };
+};
+
&lpass {
status = "okay";
};
--
2.39.2
There are two control lines controlled by GPIO going into ov5640
- Reset
- Powerdown
The driver and yaml expect "reset-gpios" and "powerdown-gpios" there has
never been an "enable-gpios".
Fixes: 39e0ce6cd1bf ("arm64: dts: qcom: apq8016-sbc: Add CCI/Sensor nodes")
Signed-off-by: Bryan O'Donoghue <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
---
arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
index ada0777567623..ddb19709a9eee 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
@@ -273,7 +273,7 @@ camera_rear@3b {
compatible = "ovti,ov5640";
reg = <0x3b>;
- enable-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;
+ powerdown-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;
reset-gpios = <&tlmm 35 GPIO_ACTIVE_LOW>;
pinctrl-names = "default";
pinctrl-0 = <&camera_rear_default>;
--
2.39.2
On 9.08.2023 22:23, Bryan O'Donoghue wrote:
> When we have no camera mezzanine attached it is still possible to run the
> test-pattern generator of the CSID block.
>
> As an example:
>
> media-ctl --reset
>
> yavta --no-query -w '0x009f0903 1' /dev/v4l-subdev2
> yavta --list /dev/v4l-subdev2
>
> media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:UYVY8_1X16/1920x1080 field:none]'
> media-ctl -l '"msm_csid0":1->"msm_ispif0":0[1]'
> media-ctl -d /dev/media0 -V '"msm_ispif0":0[fmt:UYVY8_1X16/1920x1080 field:none]'
> media-ctl -l '"msm_ispif0":1->"msm_vfe0_rdi0":0[1]'
> media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:UYVY8_1X16/1920x1080]'
> media-ctl -d /dev/media0 -p
>
> yavta -B capture-mplane --capture=5 -n 5 -I -f UYVY -s 1920x1080 --file=TPG-UYVU-1920x1080-000-#.bin /dev/video0
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> ---
Hm.. perhaps the ports could be just defined in soc dtsi?
Konrad
On 9.08.2023 22:23, Bryan O'Donoghue wrote:
> The driver for the ov5640 doesn't do a set-rate, instead it expects the
> clock to already be set at an appropriate rate.
>
> Similarly the yaml for ov5640 doesn't understand clock-frequency. Convert
> from clock-rate to assigned-clock and assigned-clock-rate to remediate.
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On 10/08/2023 14:32, Konrad Dybcio wrote:
> On 9.08.2023 22:23, Bryan O'Donoghue wrote:
>> When we have no camera mezzanine attached it is still possible to run the
>> test-pattern generator of the CSID block.
>>
>> As an example:
>>
>> media-ctl --reset
>>
>> yavta --no-query -w '0x009f0903 1' /dev/v4l-subdev2
>> yavta --list /dev/v4l-subdev2
>>
>> media-ctl -d /dev/media0 -V '"msm_csid0":0[fmt:UYVY8_1X16/1920x1080 field:none]'
>> media-ctl -l '"msm_csid0":1->"msm_ispif0":0[1]'
>> media-ctl -d /dev/media0 -V '"msm_ispif0":0[fmt:UYVY8_1X16/1920x1080 field:none]'
>> media-ctl -l '"msm_ispif0":1->"msm_vfe0_rdi0":0[1]'
>> media-ctl -d /dev/media0 -V '"msm_vfe0_rdi0":0[fmt:UYVY8_1X16/1920x1080]'
>> media-ctl -d /dev/media0 -p
>>
>> yavta -B capture-mplane --capture=5 -n 5 -I -f UYVY -s 1920x1080 --file=TPG-UYVU-1920x1080-000-#.bin /dev/video0
>>
>> Signed-off-by: Bryan O'Donoghue <[email protected]>
>> ---
> Hm.. perhaps the ports could be just defined in soc dtsi?
>
> Konrad
yeah they should be.
On 10/08/2023 16:11, Stephan Gerhold wrote:
> Hi,
>
> just some nitpicks. Some of them were present before already but maybe
> you can prepend or append another cleanup patch while at it. :)
>
> On Wed, Aug 09, 2023 at 09:23:42PM +0100, Bryan O'Donoghue wrote:
>> At the moment we define a single ov5640 sensor in the apq8016-sbc and
>> disable that sensor.
>>
>> The sensor mezzanine for this is a D3 Engineering Dual ov5640 mezzanine
>> card. Move the definition from the apq8016-sbc where it shouldn't be to a
>> standalone dts.
>>
>
> I wonder what would be required to implement this using a DT overlay,
> rather than a standalone separate DT? Seems like there are some .dtso
> files in upstream Linux.
>
> I'm also fine with the separate DTB personally, though.
So, we've discussed that previously and its a good model, which I like
and which works well for RPI as an example.
AFAIK though the runtime dtbo overlay is still missing at least one
upstream commit and the state of dtbo in qcom bootloaders is variable,
probably good in LK, good in u-boot and then I'd say nothing doing.
I'm hoping to transition the mezzanine dtb files to something "generic"
for boards that support 96 boards interfaces.
Its a bit out of scope for this series as, all I really want to do here
is fixup obvious errors as I find them in camss and its dtbs.
So anyway the idea would be to define labels in the core board dts files
for stuff like "powerdown-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;" I'm not
sure that's really feasible until its tried though.
Basically any mezzanine board would ideally only be defined once, with
96boards supporting baseboards providing the necessary additional detail
on pins and regulators for the mezzanine to consume..
Come to think of it though you'd have to #include "myboard.dts" so
maybe, probably, that idea not feasible.
dtbo would be better still but like I say I'm not presupposing a decent
bootloader that can apply the overlay.
I/we will look again at dtbo since its just a neater model really.
>> Enables the sensor by default, as we are adding a standalone mezzanine
>> structure.
>>
>> Signed-off-by: Bryan O'Donoghue <[email protected]>
>> Reviewed-by: Konrad Dybcio <[email protected]>
>> ---
>> arch/arm64/boot/dts/qcom/Makefile | 1 +
>> .../qcom/apq8016-sbc-d3-camera-mezzanine.dts | 55 +++++++++++++++++++
>> arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 49 -----------------
>> 3 files changed, 56 insertions(+), 49 deletions(-)
>> create mode 100644 arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
>>
>> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
>> index f15548dbfa56e..19016765ba4c6 100644
>> --- a/arch/arm64/boot/dts/qcom/Makefile
>> +++ b/arch/arm64/boot/dts/qcom/Makefile
>> @@ -1,5 +1,6 @@
>> # SPDX-License-Identifier: GPL-2.0
>> dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb
>> +dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc-d3-camera-mezzanine.dtb
>> dtb-$(CONFIG_ARCH_QCOM) += apq8039-t2.dtb
>> dtb-$(CONFIG_ARCH_QCOM) += apq8094-sony-xperia-kitakami-karin_windy.dtb
>> dtb-$(CONFIG_ARCH_QCOM) += apq8096-db820c.dtb
>> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
>> new file mode 100644
>> index 0000000000000..ef0e76e424898
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
>> @@ -0,0 +1,55 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) 2023, Linaro Ltd.
>> + */
>
> I assume you have permission from the original contributor to relicense
> this? apq8016-sbc.dts is GPL.
Eh it was Loic @ Linaro but, TBH I just added a boilerplate here ...
No you're right this should be // SPDX-License-Identifier: GPL-2.0-only
>
>> +
>> +/dts-v1/;
>> +
>> +#include "apq8016-sbc.dts"
>> +
>
> Please also move the fixed regulators here, they're part of the
> mezzanine, not the DB410c.
ack, true
>
>> +&camss {
>> + status = "okay";
>> +
>> + ports {
>> + port@0 {
>> + reg = <0>;
>> + csiphy0_ep: endpoint {
>> + data-lanes = <0 2>;
>> + remote-endpoint = <&ov5640_ep>;
>> + status = "okay";
>
> Should be unneeded since it's not set to disabled anywhere?
>> + };
>> + };
>> + };
>> +};
>> +
>> +&cci {
>> + status = "okay";
>> +};
>> +
>> +&cci_i2c0 {
>> + camera_rear@3b {
>
> camera@
sure
---
bod
Hi,
just some nitpicks. Some of them were present before already but maybe
you can prepend or append another cleanup patch while at it. :)
On Wed, Aug 09, 2023 at 09:23:42PM +0100, Bryan O'Donoghue wrote:
> At the moment we define a single ov5640 sensor in the apq8016-sbc and
> disable that sensor.
>
> The sensor mezzanine for this is a D3 Engineering Dual ov5640 mezzanine
> card. Move the definition from the apq8016-sbc where it shouldn't be to a
> standalone dts.
>
I wonder what would be required to implement this using a DT overlay,
rather than a standalone separate DT? Seems like there are some .dtso
files in upstream Linux.
I'm also fine with the separate DTB personally, though.
> Enables the sensor by default, as we are adding a standalone mezzanine
> structure.
>
> Signed-off-by: Bryan O'Donoghue <[email protected]>
> Reviewed-by: Konrad Dybcio <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/Makefile | 1 +
> .../qcom/apq8016-sbc-d3-camera-mezzanine.dts | 55 +++++++++++++++++++
> arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 49 -----------------
> 3 files changed, 56 insertions(+), 49 deletions(-)
> create mode 100644 arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
>
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index f15548dbfa56e..19016765ba4c6 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc.dtb
> +dtb-$(CONFIG_ARCH_QCOM) += apq8016-sbc-d3-camera-mezzanine.dtb
> dtb-$(CONFIG_ARCH_QCOM) += apq8039-t2.dtb
> dtb-$(CONFIG_ARCH_QCOM) += apq8094-sony-xperia-kitakami-karin_windy.dtb
> dtb-$(CONFIG_ARCH_QCOM) += apq8096-db820c.dtb
> diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
> new file mode 100644
> index 0000000000000..ef0e76e424898
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023, Linaro Ltd.
> + */
I assume you have permission from the original contributor to relicense
this? apq8016-sbc.dts is GPL.
> +
> +/dts-v1/;
> +
> +#include "apq8016-sbc.dts"
> +
Please also move the fixed regulators here, they're part of the
mezzanine, not the DB410c.
> +&camss {
> + status = "okay";
> +
> + ports {
> + port@0 {
> + reg = <0>;
> + csiphy0_ep: endpoint {
> + data-lanes = <0 2>;
> + remote-endpoint = <&ov5640_ep>;
> + status = "okay";
Should be unneeded since it's not set to disabled anywhere?
> + };
> + };
> + };
> +};
> +
> +&cci {
> + status = "okay";
> +};
> +
> +&cci_i2c0 {
> + camera_rear@3b {
camera@
Thanks,
Stephan
On 10/08/2023 16:26, Bryan O'Donoghue wrote:
> probably good in LK
lk2nd obvs !
On Thu, Aug 10, 2023 at 04:26:34PM +0100, Bryan O'Donoghue wrote:
> On 10/08/2023 16:11, Stephan Gerhold wrote:
> > Hi,
> >
> > just some nitpicks. Some of them were present before already but maybe
> > you can prepend or append another cleanup patch while at it. :)
> >
> > On Wed, Aug 09, 2023 at 09:23:42PM +0100, Bryan O'Donoghue wrote:
> > > At the moment we define a single ov5640 sensor in the apq8016-sbc and
> > > disable that sensor.
> > >
> > > The sensor mezzanine for this is a D3 Engineering Dual ov5640 mezzanine
> > > card. Move the definition from the apq8016-sbc where it shouldn't be to a
> > > standalone dts.
> > >
> >
> > I wonder what would be required to implement this using a DT overlay,
> > rather than a standalone separate DT? Seems like there are some .dtso
> > files in upstream Linux.
> >
> > I'm also fine with the separate DTB personally, though.
>
> So, we've discussed that previously and its a good model, which I like and
> which works well for RPI as an example.
>
> AFAIK though the runtime dtbo overlay is still missing at least one upstream
> commit and the state of dtbo in qcom bootloaders is variable, probably good
> in LK, good in u-boot and then I'd say nothing doing.
>
AFAIU there is work going on (at Linaro?) to move the qcom RBs to use
U-Boot, so I guess that would be the easiest common base to work with.
There is an U-Boot port for DB410c as well.
> I'm hoping to transition the mezzanine dtb files to something "generic" for
> boards that support 96 boards interfaces.
>
> Its a bit out of scope for this series as, all I really want to do here is
> fixup obvious errors as I find them in camss and its dtbs.
>
Right, yeah I think it's fine to have the separate DTB for now. I was
always confused about the disabled camera parts in apq8016-sbc, having
it in a separate DTB is definitely less confusing. :)
> So anyway the idea would be to define labels in the core board dts files for
> stuff like "powerdown-gpios = <&tlmm 34 GPIO_ACTIVE_HIGH>;" I'm not sure
> that's really feasible until its tried though.
Handling the GPIOs sounds complicated... I think it would be also okay
to have board-specific mezzanine overlays as a first step though. (i.e.
one for DB410c, others for other compatible 96boards).
>
> Basically any mezzanine board would ideally only be defined once, with
> 96boards supporting baseboards providing the necessary additional detail on
> pins and regulators for the mezzanine to consume..
>
> Come to think of it though you'd have to #include "myboard.dts" so maybe,
> probably, that idea not feasible.
>
> dtbo would be better still but like I say I'm not presupposing a decent
> bootloader that can apply the overlay.
>
> I/we will look again at dtbo since its just a neater model really.
>
Thanks, I'm looking forward to seeing what you come up with. :D
Stephan