[PATCH 0/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

[PATCH 0/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

[Stephan Gerhold]

Follow the example of the recently added apq8016-sbc-usb-host.dtso and 
convert apq8016-sbc-d3-camera-mezzanine.dts to a DT overlay that can be 
applied on top of the apq8016-sbc.dtb. This makes it more clear that 
this is not a special type of DB410c but just an addon board that can 
be added on top.

I also prepended a patch that cleans up the node names a bit.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Stephan Gerhold (2):
      arm64: dts: qcom: apq8016-sbc-d3-camera: Use more generic node names
      arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

 arch/arm64/boot/dts/qcom/Makefile                    |  5 +++--
 ...nine.dts => apq8016-sbc-d3-camera-mezzanine.dtso} | 20 ++++++++++++++------
 2 files changed, 17 insertions(+), 8 deletions(-)
---
base-commit: 7236e86ce5c8198b01c30933c2334d07d877cf48
change-id: 20230922-apq8016-sbc-camera-dtso-f247bea40f99

Best regards,
-- 
Stephan Gerhold <stephan@gerhold.net>

[PATCH 1/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Use more generic node names

[Stephan Gerhold]

Add "regulator" to the node names of the fixed regulators, and drop the
"_rear" part of the camera node name since it is not part of the class
of the device (which is simply "camera").

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

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
index c08b4be5cc7e..f9cbf8c1d689 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
@@ -9,7 +9,7 @@
 #include "apq8016-sbc.dts"
 
 / {
-	camera_vdddo_1v8: camera-vdddo-1v8 {
+	camera_vdddo_1v8: regulator-camera-vdddo {
 		compatible = "regulator-fixed";
 		regulator-name = "camera_vdddo";
 		regulator-min-microvolt = <1800000>;
@@ -17,7 +17,7 @@ camera_vdddo_1v8: camera-vdddo-1v8 {
 		regulator-always-on;
 	};
 
-	camera_vdda_2v8: camera-vdda-2v8 {
+	camera_vdda_2v8: regulator-camera-vdda {
 		compatible = "regulator-fixed";
 		regulator-name = "camera_vdda";
 		regulator-min-microvolt = <2800000>;
@@ -25,7 +25,7 @@ camera_vdda_2v8: camera-vdda-2v8 {
 		regulator-always-on;
 	};
 
-	camera_vddd_1v5: camera-vddd-1v5 {
+	camera_vddd_1v5: regulator-camera-vddd {
 		compatible = "regulator-fixed";
 		regulator-name = "camera_vddd";
 		regulator-min-microvolt = <1500000>;
@@ -53,7 +53,7 @@ &cci {
 };
 
 &cci_i2c0 {
-	camera_rear@3b {
+	camera@3b {
 		compatible = "ovti,ov5640";
 		reg = <0x3b>;
 

-- 
2.42.0

Re: [PATCH 1/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Use more generic node names

[Bryan O'Donoghue]

On 22/09/2023 16:11, Stephan Gerhold wrote:
> Add "regulator" to the node names of the fixed regulators, and drop the
> "_rear" part of the camera node name since it is not part of the class
> of the device (which is simply "camera").
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---

>   &cci_i2c0 {
> -	camera_rear@3b {
> +	camera@3b {
>   		compatible = "ovti,ov5640";
>   		reg = <0x3b>;

You could consider a follow-up patch here along the lines of

orientation = <2>

to indicate the camera is External.

This shows up with a pretty name in libcamera based applications then.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

[PATCH 2/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

[Stephan Gerhold]

Follow the example of the recently added apq8016-sbc-usb-host.dtso and
convert apq8016-sbc-d3-camera-mezzanine.dts to a DT overlay that can be
applied on top of the apq8016-sbc.dtb. This makes it more clear that
this is not a special type of DB410c but just an addon board that can
be added on top.

Functionally there should not be any difference since
apq8016-sbc-d3-camera-mezzanine.dtb is still generated as before
(but now by applying the overlay on top of apq8016-sbc.dtb).

Since dtc does not know that there are default #address/size-cells in
msm8916.dtsi, repeat those in the overlay to avoid dtc warnings because
it expects the wrong amount of address/size-cells.

It would be nice to have a generic overlay for the D3 camera mezzanine
(that can be applied to all 96Boards) but that's much more complicated
than providing a board-specific DT overlay as intermediate step.

Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
NOTE: I haven't tried booting this (since I don't have the camera
mezzanine) but I compared the resulting apq8016-sbc-d3-camera-mezzanine.dtb
which seems to be equivalent to the old one (aside from phandle
numbering and ordering).

It wouldn't hurt if someone could try booting this to be sure. :-)
---
 arch/arm64/boot/dts/qcom/Makefile                            |  5 +++--
 ...ra-mezzanine.dts => apq8016-sbc-d3-camera-mezzanine.dtso} | 12 ++++++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index d6cb840b7050..641d770662a1 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -1,10 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc.dtb
 
-apq8016-sbc-usb-host-dtbs	:= apq8016-sbc.dtb apq8016-sbc-usb-host.dtbo
+apq8016-sbc-d3-camera-mezzanine-dtbs	:= apq8016-sbc.dtb apq8016-sbc-d3-camera-mezzanine.dtbo
+apq8016-sbc-usb-host-dtbs		:= apq8016-sbc.dtb apq8016-sbc-usb-host.dtbo
 
-dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc-usb-host.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc-d3-camera-mezzanine.dtb
+dtb-$(CONFIG_ARCH_QCOM)	+= apq8016-sbc-usb-host.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.dtso
similarity index 89%
rename from arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
rename to arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dtso
index f9cbf8c1d689..d739ece6b44f 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc-d3-camera-mezzanine.dtso
@@ -5,10 +5,12 @@
  */
 
 /dts-v1/;
+/plugin/;
 
-#include "apq8016-sbc.dts"
+#include <dt-bindings/clock/qcom,gcc-msm8916.h>
+#include <dt-bindings/gpio/gpio.h>
 
-/ {
+&{/} {
 	camera_vdddo_1v8: regulator-camera-vdddo {
 		compatible = "regulator-fixed";
 		regulator-name = "camera_vdddo";
@@ -38,6 +40,9 @@ &camss {
 	status = "okay";
 
 	ports {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
 		port@0 {
 			reg = <0>;
 			csiphy0_ep: endpoint {
@@ -53,6 +58,9 @@ &cci {
 };
 
 &cci_i2c0 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
 	camera@3b {
 		compatible = "ovti,ov5640";
 		reg = <0x3b>;

-- 
2.42.0

Re: [PATCH 0/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

[Bryan O'Donoghue]

On 22/09/2023 16:11, Stephan Gerhold wrote:
> Follow the example of the recently added apq8016-sbc-usb-host.dtso and
> convert apq8016-sbc-d3-camera-mezzanine.dts to a DT overlay that can be
> applied on top of the apq8016-sbc.dtb. This makes it more clear that
> this is not a special type of DB410c but just an addon board that can
> be added on top.
> 
> I also prepended a patch that cleans up the node names a bit.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
> Stephan Gerhold (2):
>        arm64: dts: qcom: apq8016-sbc-d3-camera: Use more generic node names
>        arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay
> 
>   arch/arm64/boot/dts/qcom/Makefile                    |  5 +++--
>   ...nine.dts => apq8016-sbc-d3-camera-mezzanine.dtso} | 20 ++++++++++++++------
>   2 files changed, 17 insertions(+), 8 deletions(-)
> ---
> base-commit: 7236e86ce5c8198b01c30933c2334d07d877cf48
> change-id: 20230922-apq8016-sbc-camera-dtso-f247bea40f99
> 
> Best regards,

db410c doesn't ship with a bootloader that is capable of applying a dtbo 
though, so this conversion mandates an updated or chainloaded bootloader 
or out-of-tree kernel patch to support.

__adding__ is fine but, converting implies imposes a new requirement on 
the bootchain.

Perhaps a middle road solution is to

- Add, not convert a standalone dtbo or
- Add a dtbo that includes the mezzanine dts but amends it

Option 2 for preference but, I'm not sure the dts syntax can be 
meaningfully made to do that.

---
bod

Re: [PATCH 0/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

[Stephan Gerhold]

On Fri, Sep 22, 2023 at 09:47:07PM +0100, Bryan O'Donoghue wrote:
> On 22/09/2023 16:11, Stephan Gerhold wrote:
> > Follow the example of the recently added apq8016-sbc-usb-host.dtso and
> > convert apq8016-sbc-d3-camera-mezzanine.dts to a DT overlay that can be
> > applied on top of the apq8016-sbc.dtb. This makes it more clear that
> > this is not a special type of DB410c but just an addon board that can
> > be added on top.
> > 
> > I also prepended a patch that cleans up the node names a bit.
> > 
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> > Stephan Gerhold (2):
> >        arm64: dts: qcom: apq8016-sbc-d3-camera: Use more generic node names
> >        arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay
> > 
> >   arch/arm64/boot/dts/qcom/Makefile                    |  5 +++--
> >   ...nine.dts => apq8016-sbc-d3-camera-mezzanine.dtso} | 20 ++++++++++++++------
> >   2 files changed, 17 insertions(+), 8 deletions(-)
> > ---
> > base-commit: 7236e86ce5c8198b01c30933c2334d07d877cf48
> > change-id: 20230922-apq8016-sbc-camera-dtso-f247bea40f99
> > 
> > Best regards,
> 
> db410c doesn't ship with a bootloader that is capable of applying a dtbo
> though, so this conversion mandates an updated or chainloaded bootloader or
> out-of-tree kernel patch to support.
> 
> __adding__ is fine but, converting implies imposes a new requirement on the
> bootchain.
> 
> Perhaps a middle road solution is to
> 
> - Add, not convert a standalone dtbo or
> - Add a dtbo that includes the mezzanine dts but amends it
> 
> Option 2 for preference but, I'm not sure the dts syntax can be meaningfully
> made to do that.
> 

With these patches the apq8016-sbc-d3-camera-mezzanine.dtb is still
magically built, by running fdtoverlay on apq8016-sbc.dtb and applying
the dtbo. It's applied during the build process so you don't need a
bootloader that supports DTBOs.

There is literally *no change* for you in terms of usage. :-)

Thanks,
Stephan

Re: [PATCH 0/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

[Bryan O'Donoghue]

On 22/09/2023 21:58, Stephan Gerhold wrote:
> On Fri, Sep 22, 2023 at 09:47:07PM +0100, Bryan O'Donoghue wrote:
>> On 22/09/2023 16:11, Stephan Gerhold wrote:
>>> Follow the example of the recently added apq8016-sbc-usb-host.dtso and
>>> convert apq8016-sbc-d3-camera-mezzanine.dts to a DT overlay that can be
>>> applied on top of the apq8016-sbc.dtb. This makes it more clear that
>>> this is not a special type of DB410c but just an addon board that can
>>> be added on top.
>>>
>>> I also prepended a patch that cleans up the node names a bit.
>>>
>>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>>> ---
>>> Stephan Gerhold (2):
>>>         arm64: dts: qcom: apq8016-sbc-d3-camera: Use more generic node names
>>>         arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay
>>>
>>>    arch/arm64/boot/dts/qcom/Makefile                    |  5 +++--
>>>    ...nine.dts => apq8016-sbc-d3-camera-mezzanine.dtso} | 20 ++++++++++++++------
>>>    2 files changed, 17 insertions(+), 8 deletions(-)
>>> ---
>>> base-commit: 7236e86ce5c8198b01c30933c2334d07d877cf48
>>> change-id: 20230922-apq8016-sbc-camera-dtso-f247bea40f99
>>>
>>> Best regards,
>>
>> db410c doesn't ship with a bootloader that is capable of applying a dtbo
>> though, so this conversion mandates an updated or chainloaded bootloader or
>> out-of-tree kernel patch to support.
>>
>> __adding__ is fine but, converting implies imposes a new requirement on the
>> bootchain.
>>
>> Perhaps a middle road solution is to
>>
>> - Add, not convert a standalone dtbo or
>> - Add a dtbo that includes the mezzanine dts but amends it
>>
>> Option 2 for preference but, I'm not sure the dts syntax can be meaningfully
>> made to do that.
>>
> 
> With these patches the apq8016-sbc-d3-camera-mezzanine.dtb is still
> magically built, by running fdtoverlay on apq8016-sbc.dtb and applying
> the dtbo. It's applied during the build process so you don't need a
> bootloader that supports DTBOs.
> 
> There is literally *no change* for you in terms of usage. :-)

Ah I see

+apq8016-sbc-d3-camera-mezzanine-dtbs	:= apq8016-sbc.dtb 
apq8016-sbc-d3-camera-mezzanine.dtbo

I guess I like the idea of having the base board dtb a mezzanine.dtb and 
a dtbo that could be applied as an overlay to the base board dtb, 
optionally.

Our model then for mezzanine enablement wouild be

1. baseboard.dtb
2. baseboard-mezzanine.dtb
3. baseboard-mezzanine.dtbo

With booting #2 or #1+#3 resulting in the same image but, potentially 
allowing for

1. baseboard.dtb
2. baseboard-mezzanine.dtb
3. baseboard-mezzanine.dtbo
4. baseboard-some-other-mezzanine.dtb
5. baseboard-some-other-mezzanine.dtbo

#1 + #3 + #5 which would represent a hypothetical stacking of mezzanine 
boards.

As a model, I think that has merit.

Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Re: [PATCH 0/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay

[Bjorn Andersson]

On Fri, 22 Sep 2023 17:11:55 +0200, Stephan Gerhold wrote:
> Follow the example of the recently added apq8016-sbc-usb-host.dtso and
> convert apq8016-sbc-d3-camera-mezzanine.dts to a DT overlay that can be
> applied on top of the apq8016-sbc.dtb. This makes it more clear that
> this is not a special type of DB410c but just an addon board that can
> be added on top.
> 
> I also prepended a patch that cleans up the node names a bit.
> 
> [...]

Applied, thanks!

[1/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Use more generic node names
      commit: 0f893a2cb1ab7af6c88cfa1034debb6a790fb6c6
[2/2] arm64: dts: qcom: apq8016-sbc-d3-camera: Convert to DT overlay
      commit: ea689ec32bf0d885277d3f58450a85df5149c98b

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>