2023-11-22 14:16:36

by Paul Kocialkowski

[permalink] [raw]
Subject: [PATCH v7 6/7] ARM: dts: sun8i-a83t: Add BananaPi M3 OV5640 camera overlay

Add an overlay supporting the OV5640 from the BananaPi Camera v3
peripheral board. The board has two sensors (OV5640 and OV8865)
which cannot be supported in parallel as they share the same reset
pin and the kernel currently has no support for this case.

Signed-off-by: Paul Kocialkowski <[email protected]>
---
arch/arm/boot/dts/allwinner/Makefile | 1 +
.../sun8i-a83t-bananapi-m3-camera-ov5640.dtso | 117 ++++++++++++++++++
2 files changed, 118 insertions(+)
create mode 100644 arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso

diff --git a/arch/arm/boot/dts/allwinner/Makefile b/arch/arm/boot/dts/allwinner/Makefile
index eebb5a0c873a..a0a9aa6595e4 100644
--- a/arch/arm/boot/dts/allwinner/Makefile
+++ b/arch/arm/boot/dts/allwinner/Makefile
@@ -277,6 +277,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
sun8i-a33-sinlinx-sina33.dtb \
sun8i-a83t-allwinner-h8homlet-v2.dtb \
sun8i-a83t-bananapi-m3.dtb \
+ sun8i-a83t-bananapi-m3-camera-ov5640.dtbo \
sun8i-a83t-cubietruck-plus.dtb \
sun8i-a83t-tbs-a711.dtb \
sun8i-h2-plus-bananapi-m2-zero.dtb \
diff --git a/arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso b/arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso
new file mode 100644
index 000000000000..5868ef11bdee
--- /dev/null
+++ b/arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0 OR X11
+/*
+ * Copyright 2022 Bootlin
+ * Author: Paul Kocialkowski <[email protected]>
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/sun8i-a83t-ccu.h>
+#include <dt-bindings/gpio/gpio.h>
+
+&{/} {
+ /*
+ * These regulators actually have DLDO4 tied to their EN pin, which is
+ * described as input supply here for lack of a better representation.
+ * Their actual supply is PS, which is always-on.
+ */
+
+ ov5640_avdd: ov5640-avdd {
+ compatible = "regulator-fixed";
+ regulator-name = "ov5640-avdd";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ vin-supply = <&reg_dldo4>;
+ };
+
+ ov5640_dovdd: ov5640-dovdd {
+ compatible = "regulator-fixed";
+ regulator-name = "ov5640-dovdd";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ vin-supply = <&reg_dldo4>;
+ };
+
+ ov5640_dvdd: ov5640-dvdd {
+ compatible = "regulator-fixed";
+ regulator-name = "ov5640-dvdd";
+ regulator-min-microvolt = <1500000>;
+ regulator-max-microvolt = <1500000>;
+ vin-supply = <&reg_dldo4>;
+ };
+};
+
+&csi {
+ pinctrl-names = "default";
+ pinctrl-0 = <&csi_8bit_parallel_pins>;
+ status = "okay";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ csi_in_ov5640: endpoint {
+ remote-endpoint = <&ov5640_out_csi>;
+ bus-width = <8>;
+ data-shift = <2>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ pclk-sample = <1>;
+ };
+ };
+ };
+};
+
+&i2c2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_pe_pins>;
+ status = "okay";
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ov5640: camera@3c {
+ compatible = "ovti,ov5640";
+ reg = <0x3c>;
+
+ clocks = <&ccu CLK_CSI_MCLK>;
+ clock-names = "xclk";
+ assigned-clocks = <&ccu CLK_CSI_MCLK>;
+ assigned-clock-parents = <&osc24M>;
+ assigned-clock-rates = <24000000>;
+
+ AVDD-supply = <&ov5640_avdd>;
+ DOVDD-supply = <&ov5640_dovdd>;
+ DVDD-supply = <&ov5640_dvdd>;
+
+ powerdown-gpios = <&pio 3 15 GPIO_ACTIVE_HIGH>; /* PD15 */
+ reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
+
+ rotation = <180>;
+
+ port {
+ ov5640_out_csi: endpoint {
+ remote-endpoint = <&csi_in_ov5640>;
+ bus-width = <8>;
+ data-shift = <2>;
+ hsync-active = <1>;
+ vsync-active = <1>;
+ pclk-sample = <1>;
+ };
+ };
+ };
+};
+
+&pio {
+ pinctrl-names = "default";
+ pinctrl-0 = <&csi_mclk_pin>;
+};
+
+&reg_dldo4 {
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+};
--
2.42.1


2023-12-13 20:25:50

by Jernej Škrabec

[permalink] [raw]
Subject: Re: [PATCH v7 6/7] ARM: dts: sun8i-a83t: Add BananaPi M3 OV5640 camera overlay

Hi Paul!

On Wednesday, November 22, 2023 3:14:24 PM CET Paul Kocialkowski wrote:
> Add an overlay supporting the OV5640 from the BananaPi Camera v3
> peripheral board. The board has two sensors (OV5640 and OV8865)
> which cannot be supported in parallel as they share the same reset
> pin and the kernel currently has no support for this case.
>
> Signed-off-by: Paul Kocialkowski <[email protected]>
> ---
> arch/arm/boot/dts/allwinner/Makefile | 1 +
> .../sun8i-a83t-bananapi-m3-camera-ov5640.dtso | 117 ++++++++++++++++++
> 2 files changed, 118 insertions(+)
> create mode 100644 arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso
>
> diff --git a/arch/arm/boot/dts/allwinner/Makefile b/arch/arm/boot/dts/allwinner/Makefile
> index eebb5a0c873a..a0a9aa6595e4 100644
> --- a/arch/arm/boot/dts/allwinner/Makefile
> +++ b/arch/arm/boot/dts/allwinner/Makefile
> @@ -277,6 +277,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
> sun8i-a33-sinlinx-sina33.dtb \
> sun8i-a83t-allwinner-h8homlet-v2.dtb \
> sun8i-a83t-bananapi-m3.dtb \
> + sun8i-a83t-bananapi-m3-camera-ov5640.dtbo \
> sun8i-a83t-cubietruck-plus.dtb \
> sun8i-a83t-tbs-a711.dtb \
> sun8i-h2-plus-bananapi-m2-zero.dtb \
> diff --git a/arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso b/arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso
> new file mode 100644
> index 000000000000..5868ef11bdee
> --- /dev/null
> +++ b/arch/arm/boot/dts/allwinner/sun8i-a83t-bananapi-m3-camera-ov5640.dtso
> @@ -0,0 +1,117 @@
> +// SPDX-License-Identifier: GPL-2.0 OR X11
> +/*
> + * Copyright 2022 Bootlin
> + * Author: Paul Kocialkowski <[email protected]>
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/clock/sun8i-a83t-ccu.h>
> +#include <dt-bindings/gpio/gpio.h>

I think you should tie this overlay to BananaPi M3 board by specifying its
compatible here, like:

/ {
compatible = "sinovoip,bpi-m3";
};

> +
> +&{/} {
> + /*
> + * These regulators actually have DLDO4 tied to their EN pin, which is
> + * described as input supply here for lack of a better representation.
> + * Their actual supply is PS, which is always-on.
> + */
> +
> + ov5640_avdd: ov5640-avdd {
> + compatible = "regulator-fixed";
> + regulator-name = "ov5640-avdd";
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + vin-supply = <&reg_dldo4>;
> + };
> +
> + ov5640_dovdd: ov5640-dovdd {
> + compatible = "regulator-fixed";
> + regulator-name = "ov5640-dovdd";
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> + vin-supply = <&reg_dldo4>;
> + };
> +
> + ov5640_dvdd: ov5640-dvdd {
> + compatible = "regulator-fixed";
> + regulator-name = "ov5640-dvdd";
> + regulator-min-microvolt = <1500000>;
> + regulator-max-microvolt = <1500000>;
> + vin-supply = <&reg_dldo4>;
> + };
> +};
> +
> +&csi {
> + pinctrl-names = "default";
> + pinctrl-0 = <&csi_8bit_parallel_pins>;
> + status = "okay";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + csi_in_ov5640: endpoint {
> + remote-endpoint = <&ov5640_out_csi>;
> + bus-width = <8>;
> + data-shift = <2>;
> + hsync-active = <1>;
> + vsync-active = <1>;
> + pclk-sample = <1>;
> + };
> + };
> + };
> +};
> +
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pe_pins>;
> + status = "okay";
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ov5640: camera@3c {
> + compatible = "ovti,ov5640";
> + reg = <0x3c>;
> +
> + clocks = <&ccu CLK_CSI_MCLK>;
> + clock-names = "xclk";
> + assigned-clocks = <&ccu CLK_CSI_MCLK>;
> + assigned-clock-parents = <&osc24M>;
> + assigned-clock-rates = <24000000>;

Are those really necessary? If so, can it be moved to the driver? Assigned
clock rates have no guarantee that they will stay at that rate. Additionally,
same sensor in pinetab doesn't need that. What's the difference?

> +
> + AVDD-supply = <&ov5640_avdd>;
> + DOVDD-supply = <&ov5640_dovdd>;
> + DVDD-supply = <&ov5640_dvdd>;
> +
> + powerdown-gpios = <&pio 3 15 GPIO_ACTIVE_HIGH>; /* PD15 */
> + reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
> +
> + rotation = <180>;
> +
> + port {
> + ov5640_out_csi: endpoint {
> + remote-endpoint = <&csi_in_ov5640>;
> + bus-width = <8>;
> + data-shift = <2>;
> + hsync-active = <1>;
> + vsync-active = <1>;
> + pclk-sample = <1>;
> + };
> + };
> + };
> +};
> +
> +&pio {
> + pinctrl-names = "default";
> + pinctrl-0 = <&csi_mclk_pin>;

This should be moved to ov5640 node.

Best regards,
Jernej

> +};
> +
> +&reg_dldo4 {
> + regulator-min-microvolt = <2800000>;
> + regulator-max-microvolt = <2800000>;
> +};
>