Messages in this thread | | | Date | Fri, 26 Aug 2022 22:08:04 +0300 | From | Laurent Pinchart <> | Subject | Re: [PATCH NOT FOR MERGE v5 6/6] ARM: dts: sun8i: a83t: bananapi-m3: Enable MIPI CSI-2 with OV8865 |
| |
Hi Paul and Kévin,
Thank you for the patch.
On Fri, Aug 26, 2022 at 08:28:03PM +0200, Paul Kocialkowski wrote: > From: Kévin L'hôpital <kevin.lhopital@bootlin.com> > > The Bananapi M3 supports a camera module which includes an OV8865 sensor > connected via the parallel CSI interface and an OV8865 sensor connected > via MIPI CSI-2. > > The I2C2 bus is shared by the two sensors as well as the (active-low) > reset signal, but each sensor has it own shutdown line.
I see a single sensor in this file, am I missing something ?
Sounds like a perfect candidate for an overlay, it could then be merged upstream.
> Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com> > --- > arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 102 +++++++++++++++++++ > 1 file changed, 102 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts > index 5a7e1bd5f825..80fd99cf24b2 100644 > --- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts > +++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts > @@ -85,6 +85,30 @@ led-1 { > }; > }; > > + reg_ov8865_avdd: ov8865-avdd { > + compatible = "regulator-fixed"; > + regulator-name = "ov8865-avdd"; > + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > + vin-supply = <®_dldo4>; > + }; > + > + reg_ov8865_dovdd: ov8865-dovdd { > + compatible = "regulator-fixed"; > + regulator-name = "ov8865-dovdd"; > + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > + vin-supply = <®_dldo4>; > + }; > + > + reg_ov8865_dvdd: ov8865-dvdd { > + compatible = "regulator-fixed"; > + regulator-name = "ov8865-dvdd"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1200000>; > + vin-supply = <®_eldo1>; > + };
Are those three regulators on the Banana Pi, or on the camera module ?
> + > reg_usb1_vbus: reg-usb1-vbus { > compatible = "regulator-fixed"; > regulator-name = "usb1-vbus"; > @@ -115,6 +139,23 @@ &cpu100 { > cpu-supply = <®_dcdc3>; > }; > > +&csi { > + status = "okay"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@1 { > + reg = <1>;
All of this (except the status = "okay") should go to sun8i-a83t.dtsi.
> + > + csi_in_mipi_csi2: endpoint { > + remote-endpoint = <&mipi_csi2_out_csi>; > + };
This too actually, once the issue mentioned in patch 5/6 gets fixed.
> + }; > + }; > +}; > + > &de { > status = "okay"; > }; > @@ -147,6 +188,36 @@ hdmi_out_con: endpoint { > }; > }; > > +&i2c2 { > + pinctrl-names = "default"; > + pinctrl-0 = <&i2c2_pe_pins>;
This looks like a candidate for upstreaming in sun8i-a83t-bananapi-m3.dts, it shouldn't be in the overlay.
> + status = "okay"; > + > + ov8865: camera@36 { > + compatible = "ovti,ov8865"; > + reg = <0x36>; > + pinctrl-names = "default"; > + pinctrl-0 = <&csi_mclk_pin>; > + clocks = <&ccu CLK_CSI_MCLK>; > + assigned-clocks = <&ccu CLK_CSI_MCLK>; > + assigned-clock-rates = <24000000>; > + avdd-supply = <®_ov8865_avdd>; > + dovdd-supply = <®_ov8865_dovdd>; > + dvdd-supply = <®_ov8865_dvdd>; > + powerdown-gpios = <&pio 4 17 GPIO_ACTIVE_LOW>; /* PE17 */ > + reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */ > + > + port { > + ov8865_out_mipi_csi2: endpoint { > + data-lanes = <1 2 3 4>; > + link-frequencies = /bits/ 64 <360000000>; > + > + remote-endpoint = <&mipi_csi2_in_ov8865>; > + }; > + }; > + }; > +}; > + > &mdio { > rgmii_phy: ethernet-phy@1 { > compatible = "ethernet-phy-ieee802.3-c22"; > @@ -154,6 +225,24 @@ rgmii_phy: ethernet-phy@1 { > }; > }; > > +&mipi_csi2 { > + status = "okay"; > +}; > + > +&mipi_csi2_in { > + mipi_csi2_in_ov8865: endpoint { > + data-lanes = <1 2 3 4>; > + > + remote-endpoint = <&ov8865_out_mipi_csi2>; > + }; > +}; > + > +&mipi_csi2_out { > + mipi_csi2_out_csi: endpoint { > + remote-endpoint = <&csi_in_mipi_csi2>; > + }; > +}; > + > &mmc0 { > pinctrl-names = "default"; > pinctrl-0 = <&mmc0_pins>; > @@ -327,11 +416,24 @@ ®_dldo3 { > regulator-name = "vcc-pd"; > }; > > +®_dldo4 { > + regulator-always-on;
Does it have to be always on ?
> + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > + regulator-name = "avdd-csi";
Doesn't this belong to the base board .dts ? Same below.
> +}; > + > ®_drivevbus { > regulator-name = "usb0-vbus"; > status = "okay"; > }; > > +®_eldo1 { > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <1200000>; > + regulator-name = "dvdd-csi-r"; > +}; > + > ®_fldo1 { > regulator-min-microvolt = <1080000>; > regulator-max-microvolt = <1320000>;
-- Regards,
Laurent Pinchart
| |