[PATCH 0/5] media/sun6i: Allwinner A64 CSI support

[PATCH 0/5] media/sun6i: Allwinner A64 CSI support

[Jagan Teki]

This series support CSI on Allwinner A64.

The CSI controller seems similar to that of in H3, so fallback
compatible is used to load the driver.

Unlike other SoC's A64 has set of GPIO Pin gropus SDA, SCK intead
of dedicated I2C controller, so this series used i2c-gpio bitbanging.

Right now the camera is able to detect, but capture images shows 
sequence of red, blue line. any suggestion please help.

Any inputs,
Jagan.

Jagan Teki (5):
  dt-bindings: media: sun6i: Add A64 CSI compatible (w/ H3 fallback)
  dt-bindings: media: sun6i: Add vcc-csi supply property
  media: sun6i: Add vcc-csi supply regulator
  arm64: dts: allwinner: a64: Add A64 CSI controller
  arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

 .../devicetree/bindings/media/sun6i-csi.txt   |  4 ++
 .../allwinner/sun50i-a64-amarula-relic.dts    | 54 +++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 26 +++++++++
 .../platform/sunxi/sun6i-csi/sun6i_csi.c      | 15 ++++++
 4 files changed, 99 insertions(+)

-- 
2.18.0.321.gffc6fa0e3

[PATCH 1/5] dt-bindings: media: sun6i: Add A64 CSI compatible (w/ H3 fallback)

[Jagan Teki]

Allwinner A64 CSI has single channel time-multiplexed BT.656
CMOS sensor interface like H3.

Add a compatible string for it with H3 fallback compatible string,
in this case the H3 driver can be used.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 Documentation/devicetree/bindings/media/sun6i-csi.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt
index cc37cf7fd051..e78cf4f9bc8c 100644
--- a/Documentation/devicetree/bindings/media/sun6i-csi.txt
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -7,6 +7,7 @@ Required properties:
   - compatible: value must be one of:
     * "allwinner,sun6i-a31-csi"
     * "allwinner,sun8i-h3-csi"
+    * "allwinner,sun50i-a64-csi", "allwinner,sun8i-h3-csi"
     * "allwinner,sun8i-v3s-csi"
   - reg: base address and size of the memory-mapped region.
   - interrupts: interrupt associated to this IP
-- 
2.18.0.321.gffc6fa0e3

[PATCH 2/5] dt-bindings: media: sun6i: Add vcc-csi supply property

[Jagan Teki]

Most of the Allwinner A64 CSI controllers are supply with
VCC-PE pin. which need to supply for some of the boards to
trigger the power.

So, document the supply property as vcc-csi so-that the required
board can eable it via device tree.

Used vcc-csi instead of vcc-pe to have better naming convention
wrt other controller pin supplies.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 Documentation/devicetree/bindings/media/sun6i-csi.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt
index e78cf4f9bc8c..5fb6fd4e2c7d 100644
--- a/Documentation/devicetree/bindings/media/sun6i-csi.txt
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -18,6 +18,9 @@ Required properties:
   - clock-names: the clock names mentioned above
   - resets: phandles to the reset line driving the CSI
 
+Optional properties:
+  - vcc-csi-supply: the VCC-CSI power supply of the CSI PE group
+
 The CSI node should contain one 'port' child node with one child 'endpoint'
 node, according to the bindings defined in
 Documentation/devicetree/bindings/media/video-interfaces.txt.
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH 2/5] dt-bindings: media: sun6i: Add vcc-csi supply property

[Chen-Yu Tsai]

On Mon, Dec 3, 2018 at 6:08 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Most of the Allwinner A64 CSI controllers are supply with
> VCC-PE pin. which need to supply for some of the boards to
> trigger the power.
>
> So, document the supply property as vcc-csi so-that the required
> board can eable it via device tree.
>
> Used vcc-csi instead of vcc-pe to have better naming convention
> wrt other controller pin supplies.

This is not related to the CSI controller. It belongs in the pin
controller, but that has its own set of problems like possible
circular dependencies.

ChenYu

Re: [PATCH 2/5] dt-bindings: media: sun6i: Add vcc-csi supply property

[Rob Herring]

On Mon, Dec 03, 2018 at 06:11:35PM +0800, Chen-Yu Tsai wrote:
> On Mon, Dec 3, 2018 at 6:08 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Most of the Allwinner A64 CSI controllers are supply with
> > VCC-PE pin. which need to supply for some of the boards to
> > trigger the power.
> >
> > So, document the supply property as vcc-csi so-that the required
> > board can eable it via device tree.
> >
> > Used vcc-csi instead of vcc-pe to have better naming convention
> > wrt other controller pin supplies.
> 
> This is not related to the CSI controller. It belongs in the pin
> controller, but that has its own set of problems like possible
> circular dependencies.

That might be a better choice, but I think most platforms put the supply 
in the module node. But that wouldn't work well if the module is not 
used and you want to use the pins for GPIO or some other function. Maybe 
we don't hit that property because most I/O supplies are always on.

Rob

[PATCH 3/5] media: sun6i: Add vcc-csi supply regulator

[Jagan Teki]

Most of the Allwinner A64 CSI controllers are supply with
VCC-PE pin, which may not be turned on by default.

Add support for such boards by adding voltage regulator handling
code to sun6i csi driver.

Used vcc-csi instead of vcc-pe to have better naming convention
wrt other controller pin supplies.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../media/platform/sunxi/sun6i-csi/sun6i_csi.c    | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index 6950585edb5a..5836fa5e6b01 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -18,6 +18,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/sched.h>
 #include <linux/sizes.h>
@@ -36,6 +37,7 @@ struct sun6i_csi_dev {
 	struct clk			*clk_mod;
 	struct clk			*clk_ram;
 	struct reset_control		*rstc_bus;
+	struct regulator		*regulator;
 
 	int				planar_offset[3];
 };
@@ -163,9 +165,16 @@ int sun6i_csi_set_power(struct sun6i_csi *csi, bool enable)
 		clk_disable_unprepare(sdev->clk_ram);
 		clk_disable_unprepare(sdev->clk_mod);
 		reset_control_assert(sdev->rstc_bus);
+		regulator_disable(sdev->regulator);
 		return 0;
 	}
 
+	ret = regulator_enable(sdev->regulator);
+	if (ret) {
+		dev_err(sdev->dev, "Enable vcc csi supply err %d\n", ret);
+		return ret;
+	}
+
 	ret = clk_prepare_enable(sdev->clk_mod);
 	if (ret) {
 		dev_err(sdev->dev, "Enable csi clk err %d\n", ret);
@@ -809,6 +818,12 @@ static int sun6i_csi_resource_request(struct sun6i_csi_dev *sdev,
 	if (IS_ERR(io_base))
 		return PTR_ERR(io_base);
 
+	sdev->regulator = devm_regulator_get(&pdev->dev, "vcc-csi");
+	if (IS_ERR(sdev->regulator)) {
+		dev_err(&pdev->dev, "Unable to acquire csi vcc supply\n");
+		return PTR_ERR(sdev->regulator);
+	}
+
 	sdev->regmap = devm_regmap_init_mmio_clk(&pdev->dev, "bus", io_base,
 						 &sun6i_csi_regmap_config);
 	if (IS_ERR(sdev->regmap)) {
-- 
2.18.0.321.gffc6fa0e3

[PATCH 4/5] arm64: dts: allwinner: a64: Add A64 CSI controller

[Jagan Teki]

Allwinner A64 CSI controller has similar features as like in
H3, So add support for A64 via H3 fallback.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 21 +++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 384c417cb7a2..d32ff694ac5c 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -532,6 +532,12 @@
 			interrupt-controller;
 			#interrupt-cells = <3>;
 
+			csi_pins: csi-pins {
+				pins = "PE0", "PE2", "PE3", "PE4", "PE5", "PE6",
+				       "PE7", "PE8", "PE9", "PE10", "PE11";
+				function = "csi0";
+			};
+
 			i2c0_pins: i2c0_pins {
 				pins = "PH0", "PH1";
 				function = "i2c0";
@@ -899,6 +905,21 @@
 			status = "disabled";
 		};
 
+		csi: csi@1cb0000 {
+			compatible = "allwinner,sun50i-a64-csi",
+				     "allwinner,sun8i-h3-csi";
+			reg = <0x01cb0000 0x1000>;
+			interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_CSI>,
+				 <&ccu CLK_CSI_SCLK>,
+				 <&ccu CLK_DRAM_CSI>;
+			clock-names = "bus", "mod", "ram";
+			resets = <&ccu RST_BUS_CSI>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&csi_pins>;
+			status = "disabled";
+		};
+
 		hdmi: hdmi@1ee0000 {
 			compatible = "allwinner,sun50i-a64-dw-hdmi",
 				     "allwinner,sun8i-a83t-dw-hdmi";
-- 
2.18.0.321.gffc6fa0e3

[PATCH 5/5] arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

[Jagan Teki]

Amarula A64-Relic board by default bound with OV5640 camera,
so add support for it with below pin information.

- PE13, PE12 via i2c-gpio bitbanging
- CLK_CSI_MCLK as external clock
- PE1 as external clock pin muxing
- DLDO3 as vcc-csi supply
- DLDO3 as AVDD supply
- ALDO1 as DOVDD supply
- ELDO3 as DVDD supply
- PE14 gpio for reset pin
- PE15 gpio for powerdown pin

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 .../allwinner/sun50i-a64-amarula-relic.dts    | 54 +++++++++++++++++++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  5 ++
 2 files changed, 59 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
index 6cb2b7f0c817..9ac6d773188b 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
@@ -22,6 +22,41 @@
 		stdout-path = "serial0:115200n8";
 	};
 
+	i2c-csi {
+		compatible = "i2c-gpio";
+		sda-gpios = <&pio 4 13 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+		scl-gpios = <&pio 4 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
+		i2c-gpio,delay-us = <5>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ov5640: camera@3c {
+			compatible = "ovti,ov5640";
+			reg = <0x3c>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&csi_mclk_pin>;
+			clocks = <&ccu CLK_CSI_MCLK>;
+			clock-names = "xclk";
+
+			AVDD-supply = <&reg_dldo3>;
+			DOVDD-supply = <&reg_aldo1>;
+			DVDD-supply = <&reg_eldo3>;
+			reset-gpios = <&pio 4 14 GPIO_ACTIVE_LOW>; /* CSI-RST-R: PE14 */
+			powerdown-gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* CSI-STBY-R: PE15 */
+
+			port {
+				ov5640_ep: endpoint {
+					remote-endpoint = <&csi_ep>;
+					bus-width = <8>;
+					hsync-active = <1>; /* Active high */
+					vsync-active = <0>; /* Active low */
+					data-active = <1>;  /* Active high */
+					pclk-sample = <1>;  /* Rising */
+				};
+			};
+		};
+	};
+
 	wifi_pwrseq: wifi-pwrseq {
 		compatible = "mmc-pwrseq-simple";
 		clocks = <&rtc 1>;
@@ -30,6 +65,25 @@
 	};
 };
 
+&csi {
+	vcc-csi-supply = <&reg_dldo3>;
+	status = "okay";
+
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		csi_ep: endpoint {
+			remote-endpoint = <&ov5640_ep>;
+			bus-width = <8>;
+			hsync-active = <1>; /* Active high */
+			vsync-active = <0>; /* Active low */
+			data-active = <1>;  /* Active high */
+			pclk-sample = <1>;  /* Rising */
+		};
+	};
+};
+
 &ehci0 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index d32ff694ac5c..844bb44a78af 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -538,6 +538,11 @@
 				function = "csi0";
 			};
 
+			csi_mclk_pin: csi-mclk {
+				pins = "PE1";
+				function = "csi0";
+			};
+
 			i2c0_pins: i2c0_pins {
 				pins = "PH0", "PH1";
 				function = "i2c0";
-- 
2.18.0.321.gffc6fa0e3

Re: [PATCH 5/5] arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

[Chen-Yu Tsai]

On Mon, Dec 3, 2018 at 6:08 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Amarula A64-Relic board by default bound with OV5640 camera,
> so add support for it with below pin information.
>
> - PE13, PE12 via i2c-gpio bitbanging
> - CLK_CSI_MCLK as external clock
> - PE1 as external clock pin muxing
> - DLDO3 as vcc-csi supply
> - DLDO3 as AVDD supply
> - ALDO1 as DOVDD supply
> - ELDO3 as DVDD supply
> - PE14 gpio for reset pin
> - PE15 gpio for powerdown pin
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  .../allwinner/sun50i-a64-amarula-relic.dts    | 54 +++++++++++++++++++
>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  5 ++
>  2 files changed, 59 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> index 6cb2b7f0c817..9ac6d773188b 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> @@ -22,6 +22,41 @@
>                 stdout-path = "serial0:115200n8";
>         };
>
> +       i2c-csi {
> +               compatible = "i2c-gpio";
> +               sda-gpios = <&pio 4 13 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> +               scl-gpios = <&pio 4 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;

FYI our hardware doesn't do open drain.

> +               i2c-gpio,delay-us = <5>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               ov5640: camera@3c {
> +                       compatible = "ovti,ov5640";
> +                       reg = <0x3c>;
> +                       pinctrl-names = "default";
> +                       pinctrl-0 = <&csi_mclk_pin>;
> +                       clocks = <&ccu CLK_CSI_MCLK>;
> +                       clock-names = "xclk";
> +
> +                       AVDD-supply = <&reg_dldo3>;
> +                       DOVDD-supply = <&reg_aldo1>;

DOVDD is the supply for I/O. You say it is ALDO1 here.

> +                       DVDD-supply = <&reg_eldo3>;
> +                       reset-gpios = <&pio 4 14 GPIO_ACTIVE_LOW>; /* CSI-RST-R: PE14 */
> +                       powerdown-gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* CSI-STBY-R: PE15 */
> +
> +                       port {
> +                               ov5640_ep: endpoint {
> +                                       remote-endpoint = <&csi_ep>;
> +                                       bus-width = <8>;
> +                                       hsync-active = <1>; /* Active high */
> +                                       vsync-active = <0>; /* Active low */
> +                                       data-active = <1>;  /* Active high */
> +                                       pclk-sample = <1>;  /* Rising */
> +                               };
> +                       };
> +               };
> +       };
> +
>         wifi_pwrseq: wifi-pwrseq {
>                 compatible = "mmc-pwrseq-simple";
>                 clocks = <&rtc 1>;
> @@ -30,6 +65,25 @@
>         };
>  };
>
> +&csi {
> +       vcc-csi-supply = <&reg_dldo3>;

But here you say the SoC-side pins are driven from DLDO3.

This is a somewhat odd mismatch.

Regardless, the ov5640 driver enables all three regulators at probe time.
Shouldn't that be enough to get the I2C bus working? The pin voltage
supply does not belong here.

> +       status = "okay";
> +
> +       port {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               csi_ep: endpoint {
> +                       remote-endpoint = <&ov5640_ep>;
> +                       bus-width = <8>;
> +                       hsync-active = <1>; /* Active high */
> +                       vsync-active = <0>; /* Active low */
> +                       data-active = <1>;  /* Active high */
> +                       pclk-sample = <1>;  /* Rising */
> +               };
> +       };
> +};
> +
>  &ehci0 {
>         status = "okay";
>  };
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> index d32ff694ac5c..844bb44a78af 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> @@ -538,6 +538,11 @@
>                                 function = "csi0";
>                         };
>
> +                       csi_mclk_pin: csi-mclk {
> +                               pins = "PE1";
> +                               function = "csi0";
> +                       };
> +

This should be a separate patch.

ChenYu

>                         i2c0_pins: i2c0_pins {
>                                 pins = "PH0", "PH1";
>                                 function = "i2c0";
> --
> 2.18.0.321.gffc6fa0e3
>

Re: [PATCH 5/5] arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

[Jagan Teki]

On Mon, Dec 3, 2018 at 3:55 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Dec 3, 2018 at 6:08 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Amarula A64-Relic board by default bound with OV5640 camera,
> > so add support for it with below pin information.
> >
> > - PE13, PE12 via i2c-gpio bitbanging
> > - CLK_CSI_MCLK as external clock
> > - PE1 as external clock pin muxing
> > - DLDO3 as vcc-csi supply
> > - DLDO3 as AVDD supply
> > - ALDO1 as DOVDD supply
> > - ELDO3 as DVDD supply
> > - PE14 gpio for reset pin
> > - PE15 gpio for powerdown pin
> >
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >  .../allwinner/sun50i-a64-amarula-relic.dts    | 54 +++++++++++++++++++
> >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  5 ++
> >  2 files changed, 59 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > index 6cb2b7f0c817..9ac6d773188b 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > @@ -22,6 +22,41 @@
> >                 stdout-path = "serial0:115200n8";
> >         };
> >
> > +       i2c-csi {
> > +               compatible = "i2c-gpio";
> > +               sda-gpios = <&pio 4 13 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> > +               scl-gpios = <&pio 4 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
>
> FYI our hardware doesn't do open drain.

True, but the kernel is enforcing it seems, from the change from [1].
does that mean Linux use open drain even though hardware doens't have?
or did I miss anything?

[    3.659235] gpio-141 (sda): enforced open drain please flag it
properly in DT/ACPI DSDT/board file
[    3.679954] gpio-140 (scl): enforced open drain please flag it
properly in DT/ACPI DSDT/board file
[    3.814878] i2c-gpio i2c-csi: using lines 141 (SDA) and 140 (SCL)

>
> > +               i2c-gpio,delay-us = <5>;
> > +               #address-cells = <1>;
> > +               #size-cells = <0>;
> > +
> > +               ov5640: camera@3c {
> > +                       compatible = "ovti,ov5640";
> > +                       reg = <0x3c>;
> > +                       pinctrl-names = "default";
> > +                       pinctrl-0 = <&csi_mclk_pin>;
> > +                       clocks = <&ccu CLK_CSI_MCLK>;
> > +                       clock-names = "xclk";
> > +
> > +                       AVDD-supply = <&reg_dldo3>;
> > +                       DOVDD-supply = <&reg_aldo1>;
>
> DOVDD is the supply for I/O. You say it is ALDO1 here.
>
> > +                       DVDD-supply = <&reg_eldo3>;
> > +                       reset-gpios = <&pio 4 14 GPIO_ACTIVE_LOW>; /* CSI-RST-R: PE14 */
> > +                       powerdown-gpios = <&pio 4 15 GPIO_ACTIVE_HIGH>; /* CSI-STBY-R: PE15 */
> > +
> > +                       port {
> > +                               ov5640_ep: endpoint {
> > +                                       remote-endpoint = <&csi_ep>;
> > +                                       bus-width = <8>;
> > +                                       hsync-active = <1>; /* Active high */
> > +                                       vsync-active = <0>; /* Active low */
> > +                                       data-active = <1>;  /* Active high */
> > +                                       pclk-sample = <1>;  /* Rising */
> > +                               };
> > +                       };
> > +               };
> > +       };
> > +
> >         wifi_pwrseq: wifi-pwrseq {
> >                 compatible = "mmc-pwrseq-simple";
> >                 clocks = <&rtc 1>;
> > @@ -30,6 +65,25 @@
> >         };
> >  };
> >
> > +&csi {
> > +       vcc-csi-supply = <&reg_dldo3>;
>
> But here you say the SoC-side pins are driven from DLDO3.
>
> This is a somewhat odd mismatch.
>
> Regardless, the ov5640 driver enables all three regulators at probe time.
> Shouldn't that be enough to get the I2C bus working? The pin voltage
> supply does not belong here.

It is working w/o supplying PE group, but I think that may be reason
of supplying similar regulator via DOVDD on sensor side. But we need
to make sure the pin-group must be powered right like DSI, HDMI? if
yes may be we do something via power-domain driver like other SoC's
does or do we have any other option.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/gpio/gpiolib.c?id=f926dfc112bc6cf41d7068ee5e3f261e13a5bec8

Re: [PATCH 5/5] arm64: dts: allwinner: a64-amarula-relic: Add OV5640 camera node

[Rob Herring]

On Thu, Dec 06, 2018 at 04:43:33PM +0530, Jagan Teki wrote:
> On Mon, Dec 3, 2018 at 3:55 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > On Mon, Dec 3, 2018 at 6:08 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Amarula A64-Relic board by default bound with OV5640 camera,
> > > so add support for it with below pin information.
> > >
> > > - PE13, PE12 via i2c-gpio bitbanging
> > > - CLK_CSI_MCLK as external clock
> > > - PE1 as external clock pin muxing
> > > - DLDO3 as vcc-csi supply
> > > - DLDO3 as AVDD supply
> > > - ALDO1 as DOVDD supply
> > > - ELDO3 as DVDD supply
> > > - PE14 gpio for reset pin
> > > - PE15 gpio for powerdown pin
> > >
> > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > ---
> > >  .../allwinner/sun50i-a64-amarula-relic.dts    | 54 +++++++++++++++++++
> > >  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi |  5 ++
> > >  2 files changed, 59 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > > index 6cb2b7f0c817..9ac6d773188b 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-amarula-relic.dts
> > > @@ -22,6 +22,41 @@
> > >                 stdout-path = "serial0:115200n8";
> > >         };
> > >
> > > +       i2c-csi {
> > > +               compatible = "i2c-gpio";
> > > +               sda-gpios = <&pio 4 13 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> > > +               scl-gpios = <&pio 4 12 (GPIO_ACTIVE_HIGH|GPIO_OPEN_DRAIN)>;
> >
> > FYI our hardware doesn't do open drain.
> 
> True, but the kernel is enforcing it seems, from the change from [1].
> does that mean Linux use open drain even though hardware doens't have?
> or did I miss anything?

It's forced because you can't do I2C without open drain. Things like the 
slave doing clock stretching won't work.

Rob

Re: [PATCH 0/5] media/sun6i: Allwinner A64 CSI support

[Chen-Yu Tsai]

On Mon, Dec 3, 2018 at 6:07 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> This series support CSI on Allwinner A64.
>
> The CSI controller seems similar to that of in H3, so fallback
> compatible is used to load the driver.
>
> Unlike other SoC's A64 has set of GPIO Pin gropus SDA, SCK intead
> of dedicated I2C controller, so this series used i2c-gpio bitbanging.
>
> Right now the camera is able to detect, but capture images shows
> sequence of red, blue line. any suggestion please help.

The CSI controller doesn't seem to work properly at the default
clock rate of 600 MHz. Dropping it down to 300 MHz, the default
rate used by the BSP, fixes things.

The BSP also tries to use different clock rates (multiples of 108 MHz)
according to the captured image size. I've not tried this since the
driver no longer exports sub-device controls, and I currently don't
know how to handle that to change the resolution.

Regards
ChenYu

Re: [PATCH 0/5] media/sun6i: Allwinner A64 CSI support

[Jagan Teki]

On Mon, Dec 3, 2018 at 3:44 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> On Mon, Dec 3, 2018 at 6:07 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > This series support CSI on Allwinner A64.
> >
> > The CSI controller seems similar to that of in H3, so fallback
> > compatible is used to load the driver.
> >
> > Unlike other SoC's A64 has set of GPIO Pin gropus SDA, SCK intead
> > of dedicated I2C controller, so this series used i2c-gpio bitbanging.
> >
> > Right now the camera is able to detect, but capture images shows
> > sequence of red, blue line. any suggestion please help.
>
> The CSI controller doesn't seem to work properly at the default
> clock rate of 600 MHz. Dropping it down to 300 MHz, the default
> rate used by the BSP, fixes things.
>
> The BSP also tries to use different clock rates (multiples of 108 MHz)
> according to the captured image size. I've not tried this since the
> driver no longer exports sub-device controls, and I currently don't
> know how to handle that to change the resolution.

I saw 1080p@30 capture is not working, but rest 320x240@30,
640x480@30, 640x480@60, 1280x720@30 with UYVY8_2X8 and YUYV8_2X8 seems
working on 300MHz.

I even tried 1080p on 600MHz but kernel seems crashing
[video4linux2,v4l2 @ 0x2eaa9380] ioctl(VIDIOC_G_PARM): Inappropriate
ioctl for device
[video4linux2,v4l2 @ 0x2eaa9380] Time per frame unknown
[video4linux2,v4l2 @ 0x2eaa9380] Stream #0: not enough frames to
estimate rate; consider increasing probesize
Input #0, video4linux2,v4l2, from '/dev/video0':
  Duration: N/A, start: 44.934807, bitrate: N/A
    Stream #0:0: Video: rawvideo (I420 / 0x30323449), yuv420p,
1920x1080, 1000k tbr, 1000k tbn, 1000k tbc
Stream mapping:
  Stream #0:0 -> #0:0 (rawvideo (native) -> mpeg4 (native))
Press [q] to stop, [?] for help
Output #0, matroska, to 'output_YUYV8_2X8_1920x1080@1_30-new.mkv':
  Metadata:
    encoder         : Lavf57.83.100
    Stream #0:0: Video: mpeg4 (FMP4 / 0x34504D46), yuv420p, 1920x1080,
q=2-31, 200 kb/s, 65535 fps, 1k tbn, 65535 tbc
    Metadata:
      encoder         : Lavc57.107.100 mpeg4
    Side data:
      cpb: bitrate max/min/avg: 0/0/200000 buffer size: 0 vbv_delay: -1
[   45.255793] random: ffmpeg: uninitialized urandom read (4 bytes read)
frame=    4 fps=0.0 q=10.0 size=     150kB time=00:00:00.10
bitrate=12162.7kbits/s speed=0.17x    [   46.115153] ------------[ cut
here ]------------
[   46.119804] kernel BUG at arch/arm64/kernel/traps.c:426!
[   46.125120] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[   46.130607] Modules linked in:
[   46.133670] CPU: 2 PID: 1630 Comm: ffmpeg Not tainted
4.20.0-rc4-next-20181130-00027-g58d9b3c5c1db-dirty #10
[   46.143497] Hardware name: Amarula A64-Relic (DT)
[   46.148203] pstate: 60000085 (nZCv daIf -PAN -UAO)
[   46.153005] pc : do_undefinstr+0x248/0x260
[   46.157103] lr : do_undefinstr+0x13c/0x260
[   46.161198] sp : ffff000008013bb0
[   46.164513] x29: ffff000008013bb0 x28: ffff800035b30d40
[   46.169827] x27: ffff00000815a758 x26: 0000000000000001
[   46.175141] x25: 0000000000000000 x24: 0000000000000000
[   46.180454] x23: 0000000000000000 x22: ffff000009344140
[   46.185768] x21: 00000000837f8080 x20: ffff000008013c00
[   46.191082] x19: ffff0000091dd000 x18: 0000000000000000
[   46.196395] x17: 0000000000000000 x16: 0000000000000000
[   46.201709] x15: 0000000000000043 x14: 0000000000000341
[   46.207022] x13: 0000000000000400 x12: 0000000000000043
[   46.212336] x11: 0000000000000400 x10: 000000000001234d
[   46.217650] x9 : 0000000000000001 x8 : ffff800037db7900
[   46.222964] x7 : ffff800037db8380 x6 : ffff000008013bf8
[   46.228278] x5 : ffff0000091e8310 x4 : 00000000d5300000
[   46.233592] x3 : 0000000083700000 x2 : 0000000000000000
[   46.238906] x1 : ffff800035b30d40 x0 : 0000000000000005
[   46.244222] Process ffmpeg (pid: 1630, stack limit = 0x(____ptrval____))
[   46.250921] Call trace:
[   46.253371]  do_undefinstr+0x248/0x260
[   46.257125]  el1_undef+0x10/0x70
[   46.260358]  task_tick_fair+0x140/0x548
[   46.264199]  scheduler_tick+0x6c/0x110
[   46.267956]  update_process_times+0x40/0x58
[   46.272144]  tick_sched_handle.isra.5+0x30/0x50
[   46.276677]  tick_sched_timer+0x48/0x98
[   46.280516]  __hrtimer_run_queues+0x11c/0x190
[   46.284875]  hrtimer_interrupt+0xe4/0x240
[   46.288890]  arch_timer_handler_phys+0x30/0x40
[   46.293340]  handle_percpu_devid_irq+0x78/0x130
[   46.297874]  generic_handle_irq+0x24/0x38
[   46.301887]  __handle_domain_irq+0x5c/0xb8
[   46.305986]  gic_handle_irq+0x58/0xb0
[   46.309651]  el0_irq_naked+0x4c/0x54
[   46.313233] Code: b5fff8c0 b94047b5 17ffff9e d503201f (d4210000)
[   46.319338] ---[ end trace 0463ef25f03a52f8 ]---
[   46.323957] Kernel panic - not syncing: Fatal exception in interrupt
[   46.330311] SMP: stopping secondary CPUs
[   46.334240] Kernel Offset: disabled
[   46.337732] CPU features: 0x2,24802004
[   46.341481] Memory Limit: none