[PATCH 0/6] ARM: sun8i: a83t: Support Camera Sensor Interface controller

[PATCH 0/6] ARM: sun8i: a83t: Support Camera Sensor Interface controller

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

Hi everyone,

This series adds support for the camera sensor interface controller
found on the Allwinner A83T SoC. The controller is similar to the one
found on  the H3, with the addition of a MIPI CSI-2 interface. However,
this series only supports parallel and BT.656 interfaces, based on the
existing driver.

Patch 1 adds an undocumented clock parent of the CSI MCLK. This was
found after finding the default value to sometimes work and sometimes
not, and then comparing against BSP code.

Patch 2 adds a compatible string for the A83T variant.

Patch 3 adds support for the A83T variant to the existing sun6i-csi
driver.

Patch 4 adds a device node for the controller, as well as commonly
used pin muxing options.

Patch 5 adds a pin muxing option for I2C1 on the PE pins, used in
conjunction with the CSI pins.

Patch 6 provides an example usage of the CSI controller: the Bananapi M3
with its camera module attached.

Please have a look.


Regards
ChenYu


Chen-Yu Tsai (6):
  clk: sunxi-ng: a83t: Add pll-video0 as parent of csi-mclk
  dt-bindings: media: sun6i-csi: Add compatible string for A83T variant
  media: sun6i: Support A83T variant
  ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor
    Interface)
  ARM: dts: sun8i: a83t: Add I2C2 pinmux setting for PE pins
  [DO NOT MERGE] ARM: dts: sun8i: a83t: bananapi-m3: Enable BPI OV5640
    camera

 .../devicetree/bindings/media/sun6i-csi.txt   |   1 +
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts  | 101 ++++++++++++++++++
 arch/arm/boot/dts/sun8i-a83t.dtsi             |  37 +++++++
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c         |   5 +-
 .../platform/sunxi/sun6i-csi/sun6i_csi.c      |   1 +
 5 files changed, 143 insertions(+), 2 deletions(-)

-- 
2.20.1

[PATCH 1/6] clk: sunxi-ng: a83t: Add pll-video0 as parent of csi-mclk

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

Allwinner's BSP for the A83T lists pll-video0 as the first parent to
csi-mclk with index 0. This parent is not listed in the datasheet, but
actually works, and makes more sense considering the index is the
default value out of reset.

Add pll-video0 as a parent to csi-mclk with index 0.

Fixes: 05359be1176b ("clk: sunxi-ng: Add driver for A83T CCU")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi-ng/ccu-sun8i-a83t.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
index 2d6555d73170..5f714b4d8ee4 100644
--- a/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
+++ b/drivers/clk/sunxi-ng/ccu-sun8i-a83t.c
@@ -513,8 +513,9 @@ static SUNXI_CCU_GATE(csi_misc_clk, "csi-misc", "osc24M", 0x130, BIT(16), 0);
 
 static SUNXI_CCU_GATE(mipi_csi_clk, "mipi-csi", "osc24M", 0x130, BIT(31), 0);
 
-static const char * const csi_mclk_parents[] = { "pll-de", "osc24M" };
-static const u8 csi_mclk_table[] = { 3, 5 };
+static const char * const csi_mclk_parents[] = { "pll-video0", "pll-de",
+						 "osc24M" };
+static const u8 csi_mclk_table[] = { 0, 3, 5 };
 static SUNXI_CCU_M_WITH_MUX_TABLE_GATE(csi_mclk_clk, "csi-mclk",
 				       csi_mclk_parents, csi_mclk_table,
 				       0x134,
-- 
2.20.1

[PATCH 2/6] dt-bindings: media: sun6i-csi: Add compatible string for A83T variant

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

The A83T SoC has a camera sensor interface (known as CSI in Allwinner
lingo), which is similar to the one found on the A64 and H3. The only
difference seems to be that support of MIPI CSI through a connected
MIPI CSI-2 bridge.

Add a compatible string for this variant.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 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 0dd540bb03db..a2e3e56f0257 100644
--- a/Documentation/devicetree/bindings/media/sun6i-csi.txt
+++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt
@@ -6,6 +6,7 @@ Allwinner V3s SoC features a CSI module(CSI1) with parallel interface.
 Required properties:
   - compatible: value must be one of:
     * "allwinner,sun6i-a31-csi"
+    * "allwinner,sun8i-a83t-csi"
     * "allwinner,sun8i-h3-csi"
     * "allwinner,sun8i-v3s-csi"
     * "allwinner,sun50i-a64-csi"
-- 
2.20.1

[PATCH 3/6] media: sun6i: Support A83T variant

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

The A83T SoC has a camera sensor interface (known as CSI in Allwinner
lingo), which is similar to the one found on the A64 and H3. The only
difference seems to be that support of MIPI CSI through a connected
MIPI CSI-2 bridge.

Add support for this variant.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index 4c79eb64a7a7..6e0e894154f4 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -924,6 +924,7 @@ static int sun6i_csi_remove(struct platform_device *pdev)
 
 static const struct of_device_id sun6i_csi_of_match[] = {
 	{ .compatible = "allwinner,sun6i-a31-csi", },
+	{ .compatible = "allwinner,sun8i-a83t-csi", },
 	{ .compatible = "allwinner,sun8i-h3-csi", },
 	{ .compatible = "allwinner,sun8i-v3s-csi", },
 	{ .compatible = "allwinner,sun50i-a64-csi", },
-- 
2.20.1

[PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

The A83T SoC has a camera sensor interface (known as CSI in Allwinner
lingo), which is similar to the one found on the A64 and H3. The only
difference seems to be that support of MIPI CSI through a connected
MIPI CSI-2 bridge.

Add a device node for it, and pinctrl nodes for the commonly used MCLK
and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
the pinctrl nodes to keep the device tree blob size down if they are
unused.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index f739b88efb53..0c52f945fd5f 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -682,6 +682,20 @@
 			#interrupt-cells = <3>;
 			#gpio-cells = <3>;
 
+			/omit-if-no-ref/
+			csi_8bit_parallel_pins: csi-8bit-parallel-pins {
+				pins = "PE0", "PE2", "PE3", "PE6", "PE7",
+				       "PE8", "PE9", "PE10", "PE11",
+				       "PE12", "PE13";
+				function = "csi";
+			};
+
+			/omit-if-no-ref/
+			csi_mclk_pin: csi-mclk-pin {
+				pins = "PE1";
+				function = "csi";
+			};
+
 			emac_rgmii_pins: emac-rgmii-pins {
 				pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
 				       "PD11", "PD12", "PD13", "PD14", "PD18",
@@ -994,6 +1008,23 @@
 			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
 		};
 
+		csi: camera@1cb0000 {
+			compatible = "allwinner,sun8i-a83t-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>;
+			status = "disabled";
+
+			csi_in: port {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
+
 		hdmi: hdmi@1ee0000 {
 			compatible = "allwinner,sun8i-a83t-dw-hdmi";
 			reg = <0x01ee0000 0x10000>;
-- 
2.20.1

[PATCH 5/6] ARM: dts: sun8i: a83t: Add I2C2 pinmux setting for PE pins

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

I2C2 is available on the PE pingroup, on the same pins as the camera
sensor interface (CSI) controller's camera control interface pins.
This provides an option to use I2C2 instead of that control interface
to configure camera sensors.

Add a pinctrl node for it. The property /omit-if-no-ref/ is added to
keep the device tree blob size down if it is unused.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun8i-a83t.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
index 0c52f945fd5f..350ea40ea560 100644
--- a/arch/arm/boot/dts/sun8i-a83t.dtsi
+++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
@@ -723,6 +723,12 @@
 				function = "i2c1";
 			};
 
+			/omit-if-no-ref/
+			i2c2_pe_pins: i2c2-pe-pins {
+				pins = "PE14", "PE15";
+				function = "i2c2";
+			};
+
 			i2c2_ph_pins: i2c2-ph-pins {
 				pins = "PH4", "PH5";
 				function = "i2c2";
-- 
2.20.1

[PATCH 6/6] [DO NOT MERGE] ARM: dts: sun8i: a83t: bananapi-m3: Enable BPI OV5640 camera

[Chen-Yu Tsai]

From: Chen-Yu Tsai <wens@csie.org>

Bananapi produces a camera module specifically for the Bananapi M3. This
module has an OV5640 sensor connected via the parallel CSI interface,
and an OV8865 sensor connected via MIPI CSI. I2C2 is the shared control
interface for them. The active-low reset signal is shared by the two
sensors, while each sensor has a dedicated shutdown line. The module's
onboard regulator provide power for the OV5640, while the Bananapi M3
provides power from the PMIC for the OV8865.

There's also a VM149C voice coil motor driver at I2C address 0x0c, and
an unknown device responding on I2C address 0x20. Both are related to
the OV8655 at address 0x36.

Since there is no support for the OV8865, this patch only enables the
OV5640.

Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts | 101 +++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
index 838be7b3715f..11b27755207a 100644
--- a/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
+++ b/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts
@@ -85,6 +85,33 @@
 		};
 	};
 
+	reg_ov5640_avdd: ov5640-avdd {
+		compatible = "regulator-fixed";
+		regulator-name = "ov5640-avdd";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		/* dldo4 enables this regulator; real supply is PS */
+		vin-supply = <&reg_dldo4>;
+	};
+
+	reg_ov5640_dovdd: ov5640-iovdd {
+		compatible = "regulator-fixed";
+		regulator-name = "ov5640-dovdd";
+		regulator-min-microvolt = <2800000>;
+		regulator-max-microvolt = <2800000>;
+		/* dldo4 enables this regulator; real supply is PS */
+		vin-supply = <&reg_dldo4>;
+	};
+
+	reg_ov5640_dvdd: ov5640-dvdd {
+		compatible = "regulator-fixed";
+		regulator-name = "ov5640-dvdd";
+		regulator-min-microvolt = <1500000>;
+		regulator-max-microvolt = <1500000>;
+		/* dldo4 enables this regulator; real supply is PS */
+		vin-supply = <&reg_dldo4>;
+	};
+
 	reg_usb1_vbus: reg-usb1-vbus {
 		compatible = "regulator-fixed";
 		regulator-name = "usb1-vbus";
@@ -115,6 +142,30 @@
 	cpu-supply = <&reg_dcdc3>;
 };
 
+&ccu {
+	/* Use a stable clock source with known fixed rate for MCLK */
+	assigned-clocks = <&ccu CLK_CSI_MCLK>;
+	assigned-clock-parents = <&osc24M>;
+	assigned-clock-rates = <24000000>;
+};
+
+&csi {
+	pinctrl-names = "default";
+	pinctrl-0 = <&csi_8bit_parallel_pins>;
+	status = "okay";
+};
+
+&csi_in {
+	csi_from_ov5640: endpoint {
+		remote-endpoint = <&ov5640_to_csi>;
+		bus-width = <8>;
+		data-shift = <2>;
+		hsync-active = <1>;     /* Active high */
+		vsync-active = <1>;     /* Active high */
+		pclk-sample = <1>;      /* Rising */
+	};
+};
+
 &de {
 	status = "okay";
 };
@@ -147,6 +198,36 @@
 	};
 };
 
+&i2c2 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&i2c2_pe_pins>;
+	status = "okay";
+
+	ov5640: camera@3c {
+		compatible = "ovti,ov5640";
+		reg = <0x3c>;
+		clocks = <&ccu CLK_CSI_MCLK>;
+		clock-names = "xclk";
+		AVDD-supply = <&reg_ov5640_avdd>;
+		DOVDD-supply = <&reg_ov5640_dovdd>;
+		DVDD-supply = <&reg_ov5640_dvdd>;
+		powerdown-gpios = <&pio 3 15 GPIO_ACTIVE_HIGH>; /* PD15 */
+		reset-gpios = <&pio 4 16 GPIO_ACTIVE_LOW>; /* PE16 */
+		rotation = <180>;
+
+		port {
+			ov5640_to_csi: endpoint {
+				remote-endpoint = <&csi_from_ov5640>;
+				bus-width = <8>;
+				data-shift = <2>;
+				hsync-active = <1>;     /* Active high */
+				vsync-active = <1>;     /* Active high */
+				pclk-sample = <1>;      /* Rising */
+			};
+		};
+	};
+};
+
 &mdio {
 	rgmii_phy: ethernet-phy@1 {
 		compatible = "ethernet-phy-ieee802.3-c22";
@@ -191,6 +272,12 @@
 	status = "okay";
 };
 
+&pio {
+	/* pinctrl hog: MCLK is shared by both camera sensors */
+	pinctrl-names = "default";
+	pinctrl-0 = <&csi_mclk_pin>;
+};
+
 &r_cir {
 	clock-frequency = <3000000>;
 	status = "okay";
@@ -327,11 +414,25 @@
 	regulator-name = "vcc-pd";
 };
 
+&reg_dldo4 {
+	/* This also powers the pull-ups on the CSI connector */
+	regulator-always-on;
+	regulator-min-microvolt = <2800000>;
+	regulator-max-microvolt = <2800000>;
+	regulator-name = "avdd-csi";
+};
+
 &reg_drivevbus {
 	regulator-name = "usb0-vbus";
 	status = "okay";
 };
 
+&reg_eldo1 {
+	regulator-min-microvolt = <1200000>;
+	regulator-max-microvolt = <1200000>;
+	regulator-name = "dvdd-csi-r";
+};
+
 &reg_fldo1 {
 	regulator-min-microvolt = <1080000>;
 	regulator-max-microvolt = <1320000>;
-- 
2.20.1

Re: [PATCH 3/6] media: sun6i: Support A83T variant

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On Tue, Apr 09, 2019 at 12:57:41AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
>
> The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> lingo), which is similar to the one found on the A64 and H3. The only
> difference seems to be that support of MIPI CSI through a connected
> MIPI CSI-2 bridge.
>
> Add support for this variant.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/6] clk: sunxi-ng: a83t: Add pll-video0 as parent of csi-mclk

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

On Tue, Apr 09, 2019 at 12:57:39AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
>
> Allwinner's BSP for the A83T lists pll-video0 as the first parent to
> csi-mclk with index 0. This parent is not listed in the datasheet, but
> actually works, and makes more sense considering the index is the
> default value out of reset.
>
> Add pll-video0 as a parent to csi-mclk with index 0.
>
> Fixes: 05359be1176b ("clk: sunxi-ng: Add driver for A83T CCU")
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/6] dt-bindings: media: sun6i-csi: Add compatible string for A83T variant

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

On Tue, Apr 09, 2019 at 12:57:40AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
>
> The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> lingo), which is similar to the one found on the A64 and H3. The only
> difference seems to be that support of MIPI CSI through a connected
> MIPI CSI-2 bridge.
>
> Add a compatible string for this variant.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2273 bytes --]

Hi,

On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
>
> The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> lingo), which is similar to the one found on the A64 and H3. The only
> difference seems to be that support of MIPI CSI through a connected
> MIPI CSI-2 bridge.
>
> Add a device node for it, and pinctrl nodes for the commonly used MCLK
> and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> the pinctrl nodes to keep the device tree blob size down if they are
> unused.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> index f739b88efb53..0c52f945fd5f 100644
> --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> @@ -682,6 +682,20 @@
>  			#interrupt-cells = <3>;
>  			#gpio-cells = <3>;
>
> +			/omit-if-no-ref/
> +			csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> +				pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> +				       "PE8", "PE9", "PE10", "PE11",
> +				       "PE12", "PE13";
> +				function = "csi";
> +			};
> +
> +			/omit-if-no-ref/
> +			csi_mclk_pin: csi-mclk-pin {
> +				pins = "PE1";
> +				function = "csi";
> +			};
> +
>  			emac_rgmii_pins: emac-rgmii-pins {
>  				pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
>  				       "PD11", "PD12", "PD13", "PD14", "PD18",
> @@ -994,6 +1008,23 @@
>  			interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
>  		};
>
> +		csi: camera@1cb0000 {
> +			compatible = "allwinner,sun8i-a83t-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>;
> +			status = "disabled";
> +
> +			csi_in: port {
> +				#address-cells = <1>;
> +				#size-cells = <0>;

If we expect a single enpoint, then we don't need the address-cells
and size-cells properties.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 5/6] ARM: dts: sun8i: a83t: Add I2C2 pinmux setting for PE pins

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]

On Tue, Apr 09, 2019 at 12:57:43AM +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
>
> I2C2 is available on the PE pingroup, on the same pins as the camera
> sensor interface (CSI) controller's camera control interface pins.
> This provides an option to use I2C2 instead of that control interface
> to configure camera sensors.
>
> Add a pinctrl node for it. The property /omit-if-no-ref/ is added to
> keep the device tree blob size down if it is unused.
>
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>

Applied, thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Chen-Yu Tsai]

On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> Hi,
>
> On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <wens@csie.org>
> >
> > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > lingo), which is similar to the one found on the A64 and H3. The only
> > difference seems to be that support of MIPI CSI through a connected
> > MIPI CSI-2 bridge.
> >
> > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > the pinctrl nodes to keep the device tree blob size down if they are
> > unused.
> >
> > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > ---
> >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > index f739b88efb53..0c52f945fd5f 100644
> > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > @@ -682,6 +682,20 @@
> >                       #interrupt-cells = <3>;
> >                       #gpio-cells = <3>;
> >
> > +                     /omit-if-no-ref/
> > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > +                                    "PE8", "PE9", "PE10", "PE11",
> > +                                    "PE12", "PE13";
> > +                             function = "csi";
> > +                     };
> > +
> > +                     /omit-if-no-ref/
> > +                     csi_mclk_pin: csi-mclk-pin {
> > +                             pins = "PE1";
> > +                             function = "csi";
> > +                     };
> > +
> >                       emac_rgmii_pins: emac-rgmii-pins {
> >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > @@ -994,6 +1008,23 @@
> >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> >               };
> >
> > +             csi: camera@1cb0000 {
> > +                     compatible = "allwinner,sun8i-a83t-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>;
> > +                     status = "disabled";
> > +
> > +                     csi_in: port {
> > +                             #address-cells = <1>;
> > +                             #size-cells = <0>;
>
> If we expect a single enpoint, then we don't need the address-cells
> and size-cells properties.

I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
is kind of genius if not very hacky. They have two "identical" sensors
on the same I2C bus and CSI bus, with shared reset line but separate
shutdown lines. Since they are identical, they also have the same I2C
address. I haven't figured out how to model this in the device tree.

The point is, it's perfectly possible to have two or more sensors use
the same controller, provided only one be active at a time.

ChenYu

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4028 bytes --]

On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > From: Chen-Yu Tsai <wens@csie.org>
> > >
> > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > lingo), which is similar to the one found on the A64 and H3. The only
> > > difference seems to be that support of MIPI CSI through a connected
> > > MIPI CSI-2 bridge.
> > >
> > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > the pinctrl nodes to keep the device tree blob size down if they are
> > > unused.
> > >
> > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > ---
> > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> > >
> > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > index f739b88efb53..0c52f945fd5f 100644
> > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > @@ -682,6 +682,20 @@
> > >                       #interrupt-cells = <3>;
> > >                       #gpio-cells = <3>;
> > >
> > > +                     /omit-if-no-ref/
> > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > +                                    "PE12", "PE13";
> > > +                             function = "csi";
> > > +                     };
> > > +
> > > +                     /omit-if-no-ref/
> > > +                     csi_mclk_pin: csi-mclk-pin {
> > > +                             pins = "PE1";
> > > +                             function = "csi";
> > > +                     };
> > > +
> > >                       emac_rgmii_pins: emac-rgmii-pins {
> > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > @@ -994,6 +1008,23 @@
> > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > >               };
> > >
> > > +             csi: camera@1cb0000 {
> > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > +                     status = "disabled";
> > > +
> > > +                     csi_in: port {
> > > +                             #address-cells = <1>;
> > > +                             #size-cells = <0>;
> >
> > If we expect a single enpoint, then we don't need the address-cells
> > and size-cells properties.
>
> I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> is kind of genius if not very hacky. They have two "identical" sensors
> on the same I2C bus and CSI bus, with shared reset line but separate
> shutdown lines. Since they are identical, they also have the same I2C
> address. I haven't figured out how to model this in the device tree.
>
> The point is, it's perfectly possible to have two or more sensors use
> the same controller, provided only one be active at a time.

Right, but I guess the common case would be to have a single sensor,
where that wouldn't be needed.

In odd cases, we can always specify it in the DTS, and if it becomes
common enough, we can move it to the DTSI.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Chen-Yu Tsai]

On Tue, Apr 9, 2019 at 4:28 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > From: Chen-Yu Tsai <wens@csie.org>
> > > >
> > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > difference seems to be that support of MIPI CSI through a connected
> > > > MIPI CSI-2 bridge.
> > > >
> > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > unused.
> > > >
> > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > ---
> > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > >  1 file changed, 31 insertions(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > index f739b88efb53..0c52f945fd5f 100644
> > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > @@ -682,6 +682,20 @@
> > > >                       #interrupt-cells = <3>;
> > > >                       #gpio-cells = <3>;
> > > >
> > > > +                     /omit-if-no-ref/
> > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > +                                    "PE12", "PE13";
> > > > +                             function = "csi";
> > > > +                     };
> > > > +
> > > > +                     /omit-if-no-ref/
> > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > +                             pins = "PE1";
> > > > +                             function = "csi";
> > > > +                     };
> > > > +
> > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > @@ -994,6 +1008,23 @@
> > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > >               };
> > > >
> > > > +             csi: camera@1cb0000 {
> > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > +                     status = "disabled";
> > > > +
> > > > +                     csi_in: port {
> > > > +                             #address-cells = <1>;
> > > > +                             #size-cells = <0>;
> > >
> > > If we expect a single enpoint, then we don't need the address-cells
> > > and size-cells properties.
> >
> > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > is kind of genius if not very hacky. They have two "identical" sensors
> > on the same I2C bus and CSI bus, with shared reset line but separate
> > shutdown lines. Since they are identical, they also have the same I2C
> > address. I haven't figured out how to model this in the device tree.
> >
> > The point is, it's perfectly possible to have two or more sensors use
> > the same controller, provided only one be active at a time.
>
> Right, but I guess the common case would be to have a single sensor,
> where that wouldn't be needed.
>
> In odd cases, we can always specify it in the DTS, and if it becomes
> common enough, we can move it to the DTSI.

Makes sense. Do you want me to re-spin?

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4600 bytes --]

On Tue, Apr 09, 2019 at 04:40:40PM +0800, Chen-Yu Tsai wrote:
> On Tue, Apr 9, 2019 at 4:28 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > > From: Chen-Yu Tsai <wens@csie.org>
> > > > >
> > > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > > difference seems to be that support of MIPI CSI through a connected
> > > > > MIPI CSI-2 bridge.
> > > > >
> > > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > > unused.
> > > > >
> > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > > ---
> > > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > > >  1 file changed, 31 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > index f739b88efb53..0c52f945fd5f 100644
> > > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > @@ -682,6 +682,20 @@
> > > > >                       #interrupt-cells = <3>;
> > > > >                       #gpio-cells = <3>;
> > > > >
> > > > > +                     /omit-if-no-ref/
> > > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > > +                                    "PE12", "PE13";
> > > > > +                             function = "csi";
> > > > > +                     };
> > > > > +
> > > > > +                     /omit-if-no-ref/
> > > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > > +                             pins = "PE1";
> > > > > +                             function = "csi";
> > > > > +                     };
> > > > > +
> > > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > > @@ -994,6 +1008,23 @@
> > > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > > >               };
> > > > >
> > > > > +             csi: camera@1cb0000 {
> > > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > > +                     status = "disabled";
> > > > > +
> > > > > +                     csi_in: port {
> > > > > +                             #address-cells = <1>;
> > > > > +                             #size-cells = <0>;
> > > >
> > > > If we expect a single enpoint, then we don't need the address-cells
> > > > and size-cells properties.
> > >
> > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > is kind of genius if not very hacky. They have two "identical" sensors
> > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > shutdown lines. Since they are identical, they also have the same I2C
> > > address. I haven't figured out how to model this in the device tree.
> > >
> > > The point is, it's perfectly possible to have two or more sensors use
> > > the same controller, provided only one be active at a time.
> >
> > Right, but I guess the common case would be to have a single sensor,
> > where that wouldn't be needed.
> >
> > In odd cases, we can always specify it in the DTS, and if it becomes
> > common enough, we can move it to the DTSI.
>
> Makes sense. Do you want me to re-spin?

If there's no other comment, we'll fix it when applying.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Ondřej Jirman]

On Tue, Apr 09, 2019 at 10:28:18AM +0200, Maxime Ripard wrote:
> On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > From: Chen-Yu Tsai <wens@csie.org>
> > > >
> > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > difference seems to be that support of MIPI CSI through a connected
> > > > MIPI CSI-2 bridge.
> > > >
> > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > unused.
> > > >
> > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > ---
> > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > >  1 file changed, 31 insertions(+)
> > > >
> > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > index f739b88efb53..0c52f945fd5f 100644
> > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > @@ -682,6 +682,20 @@
> > > >                       #interrupt-cells = <3>;
> > > >                       #gpio-cells = <3>;
> > > >
> > > > +                     /omit-if-no-ref/
> > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > +                                    "PE12", "PE13";
> > > > +                             function = "csi";
> > > > +                     };
> > > > +
> > > > +                     /omit-if-no-ref/
> > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > +                             pins = "PE1";
> > > > +                             function = "csi";
> > > > +                     };
> > > > +
> > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > @@ -994,6 +1008,23 @@
> > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > >               };
> > > >
> > > > +             csi: camera@1cb0000 {
> > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > +                     status = "disabled";
> > > > +
> > > > +                     csi_in: port {
> > > > +                             #address-cells = <1>;
> > > > +                             #size-cells = <0>;
> > >
> > > If we expect a single enpoint, then we don't need the address-cells
> > > and size-cells properties.
> >
> > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > is kind of genius if not very hacky. They have two "identical" sensors
> > on the same I2C bus and CSI bus, with shared reset line but separate
> > shutdown lines. Since they are identical, they also have the same I2C
> > address. I haven't figured out how to model this in the device tree.
> >
> > The point is, it's perfectly possible to have two or more sensors use
> > the same controller, provided only one be active at a time.
> 
> Right, but I guess the common case would be to have a single sensor,
> where that wouldn't be needed.
> 
> In odd cases, we can always specify it in the DTS, and if it becomes
> common enough, we can move it to the DTSI.

I'm planning on having two sensors there, in a less arcane setup,
though - no shared resets, and different I2C addresses.

Anyway, I can confirm that CSI driver works fine on A83T with just
a DTSI patch, even without the clock patch in this series. I've been
running it for quite a while that way without any issues (different
camera chip than the ones being used by wens).

regards,
	o.

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Paul Kocialkowski]

Hi,

Le mercredi 10 avril 2019 à 00:00 +0200, Ondřej Jirman a écrit :
> On Tue, Apr 09, 2019 at 10:28:18AM +0200, Maxime Ripard wrote:
> > On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > > From: Chen-Yu Tsai <wens@csie.org>
> > > > > 
> > > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > > difference seems to be that support of MIPI CSI through a connected
> > > > > MIPI CSI-2 bridge.
> > > > > 
> > > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > > unused.
> > > > > 
> > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > > ---
> > > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > > >  1 file changed, 31 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > index f739b88efb53..0c52f945fd5f 100644
> > > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > @@ -682,6 +682,20 @@
> > > > >                       #interrupt-cells = <3>;
> > > > >                       #gpio-cells = <3>;
> > > > > 
> > > > > +                     /omit-if-no-ref/
> > > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > > +                                    "PE12", "PE13";
> > > > > +                             function = "csi";
> > > > > +                     };
> > > > > +
> > > > > +                     /omit-if-no-ref/
> > > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > > +                             pins = "PE1";
> > > > > +                             function = "csi";
> > > > > +                     };
> > > > > +
> > > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > > @@ -994,6 +1008,23 @@
> > > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > > >               };
> > > > > 
> > > > > +             csi: camera@1cb0000 {
> > > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > > +                     status = "disabled";
> > > > > +
> > > > > +                     csi_in: port {
> > > > > +                             #address-cells = <1>;
> > > > > +                             #size-cells = <0>;
> > > > 
> > > > If we expect a single enpoint, then we don't need the address-cells
> > > > and size-cells properties.
> > > 
> > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > is kind of genius if not very hacky. They have two "identical" sensors
> > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > shutdown lines. Since they are identical, they also have the same I2C
> > > address. I haven't figured out how to model this in the device tree.
> > > 
> > > The point is, it's perfectly possible to have two or more sensors use
> > > the same controller, provided only one be active at a time.
> > 
> > Right, but I guess the common case would be to have a single sensor,
> > where that wouldn't be needed.
> > 
> > In odd cases, we can always specify it in the DTS, and if it becomes
> > common enough, we can move it to the DTSI.
> 
> I'm planning on having two sensors there, in a less arcane setup,
> though - no shared resets, and different I2C addresses.
> 
> Anyway, I can confirm that CSI driver works fine on A83T with just
> a DTSI patch, even without the clock patch in this series. I've been
> running it for quite a while that way without any issues (different
> camera chip than the ones being used by wens).

That's quite nice to hear! I would be interested in getting some
insight on which sensors are known to work and which are broken or have
limitations.

Would you happen to have a list of the sensors that you tested and
whether you encountered such issues with them?

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Ondřej Jirman]

On Thu, Apr 11, 2019 at 02:47:52PM +0200, Paul Kocialkowski wrote:
> > > > > If we expect a single enpoint, then we don't need the address-cells
> > > > > and size-cells properties.
> > > > 
> > > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > > is kind of genius if not very hacky. They have two "identical" sensors
> > > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > > shutdown lines. Since they are identical, they also have the same I2C
> > > > address. I haven't figured out how to model this in the device tree.
> > > > 
> > > > The point is, it's perfectly possible to have two or more sensors use
> > > > the same controller, provided only one be active at a time.
> > > 
> > > Right, but I guess the common case would be to have a single sensor,
> > > where that wouldn't be needed.
> > > 
> > > In odd cases, we can always specify it in the DTS, and if it becomes
> > > common enough, we can move it to the DTSI.
> > 
> > I'm planning on having two sensors there, in a less arcane setup,
> > though - no shared resets, and different I2C addresses.
> > 
> > Anyway, I can confirm that CSI driver works fine on A83T with just
> > a DTSI patch, even without the clock patch in this series. I've been
> > running it for quite a while that way without any issues (different
> > camera chip than the ones being used by wens).
> 
> That's quite nice to hear! I would be interested in getting some
> insight on which sensors are known to work and which are broken or have
> limitations.
> 
> Would you happen to have a list of the sensors that you tested and
> whether you encountered such issues with them?

I wrote the driver for the sensor I'm using, so the issues were mostly
during the development. It's Himax HM5065 sensor (not yet upstream).

If you try using other mainline sensors, the issues you'll face will mostly
be configuring the buses (CSI, I2C) correctly in DTS, or lack of support
for some VSYNC/HSYNC combinations on the sensor driver side. Luckily, CSI
controller is quite flexible, and will accomodate lack of configurability
on the sensor side.

regards,
  o.

> Cheers,
> 
> Paul
> 
> -- 
> Paul Kocialkowski, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com
> 

Re: [PATCH 2/6] dt-bindings: media: sun6i-csi: Add compatible string for A83T variant

[Rob Herring]

On Tue,  9 Apr 2019 00:57:40 +0800, Chen-Yu Tsai wrote:
> From: Chen-Yu Tsai <wens@csie.org>
> 
> The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> lingo), which is similar to the one found on the A64 and H3. The only
> difference seems to be that support of MIPI CSI through a connected
> MIPI CSI-2 bridge.
> 
> Add a compatible string for this variant.
> 
> Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> ---
>  Documentation/devicetree/bindings/media/sun6i-csi.txt | 1 +
>  1 file changed, 1 insertion(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Ondřej Jirman]

Hi Maxime,

On Tue, Apr 09, 2019 at 04:52:25PM +0200, Maxime Ripard wrote:
> On Tue, Apr 09, 2019 at 04:40:40PM +0800, Chen-Yu Tsai wrote:
> > On Tue, Apr 9, 2019 at 4:28 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > > > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > > > From: Chen-Yu Tsai <wens@csie.org>
> > > > > >
> > > > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > > > difference seems to be that support of MIPI CSI through a connected
> > > > > > MIPI CSI-2 bridge.
> > > > > >
> > > > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > > > unused.
> > > > > >
> > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > > > ---
> > > > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 31 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > index f739b88efb53..0c52f945fd5f 100644
> > > > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > @@ -682,6 +682,20 @@
> > > > > >                       #interrupt-cells = <3>;
> > > > > >                       #gpio-cells = <3>;
> > > > > >
> > > > > > +                     /omit-if-no-ref/
> > > > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > > > +                                    "PE12", "PE13";
> > > > > > +                             function = "csi";
> > > > > > +                     };
> > > > > > +
> > > > > > +                     /omit-if-no-ref/
> > > > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > > > +                             pins = "PE1";
> > > > > > +                             function = "csi";
> > > > > > +                     };
> > > > > > +
> > > > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > > > @@ -994,6 +1008,23 @@
> > > > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > > > >               };
> > > > > >
> > > > > > +             csi: camera@1cb0000 {
> > > > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > > > +                     status = "disabled";
> > > > > > +
> > > > > > +                     csi_in: port {
> > > > > > +                             #address-cells = <1>;
> > > > > > +                             #size-cells = <0>;
> > > > >
> > > > > If we expect a single enpoint, then we don't need the address-cells
> > > > > and size-cells properties.
> > > >
> > > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > > is kind of genius if not very hacky. They have two "identical" sensors
> > > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > > shutdown lines. Since they are identical, they also have the same I2C
> > > > address. I haven't figured out how to model this in the device tree.
> > > >
> > > > The point is, it's perfectly possible to have two or more sensors use
> > > > the same controller, provided only one be active at a time.
> > >
> > > Right, but I guess the common case would be to have a single sensor,
> > > where that wouldn't be needed.
> > >
> > > In odd cases, we can always specify it in the DTS, and if it becomes
> > > common enough, we can move it to the DTSI.
> >
> > Makes sense. Do you want me to re-spin?
> 
> If there's no other comment, we'll fix it when applying.

This patch series seems to have been forgotten. It doesn't seem there are any
blockers. Can you please apply it now? I have some further series (camera module
support for TBS-A711) that depend on this.

thank you and regards,
	Ondrej

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 5644 bytes --]

Hi Ondřej,

On Sun, May 19, 2019 at 03:54:22PM +0200, Ondřej Jirman wrote:
> On Tue, Apr 09, 2019 at 04:52:25PM +0200, Maxime Ripard wrote:
> > On Tue, Apr 09, 2019 at 04:40:40PM +0800, Chen-Yu Tsai wrote:
> > > On Tue, Apr 9, 2019 at 4:28 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > > > > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > > > > From: Chen-Yu Tsai <wens@csie.org>
> > > > > > >
> > > > > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > > > > difference seems to be that support of MIPI CSI through a connected
> > > > > > > MIPI CSI-2 bridge.
> > > > > > >
> > > > > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > > > > unused.
> > > > > > >
> > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > > > > ---
> > > > > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 31 insertions(+)
> > > > > > >
> > > > > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > index f739b88efb53..0c52f945fd5f 100644
> > > > > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > @@ -682,6 +682,20 @@
> > > > > > >                       #interrupt-cells = <3>;
> > > > > > >                       #gpio-cells = <3>;
> > > > > > >
> > > > > > > +                     /omit-if-no-ref/
> > > > > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > > > > +                                    "PE12", "PE13";
> > > > > > > +                             function = "csi";
> > > > > > > +                     };
> > > > > > > +
> > > > > > > +                     /omit-if-no-ref/
> > > > > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > > > > +                             pins = "PE1";
> > > > > > > +                             function = "csi";
> > > > > > > +                     };
> > > > > > > +
> > > > > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > > > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > > > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > > > > @@ -994,6 +1008,23 @@
> > > > > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > > > > >               };
> > > > > > >
> > > > > > > +             csi: camera@1cb0000 {
> > > > > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > > > > +                     status = "disabled";
> > > > > > > +
> > > > > > > +                     csi_in: port {
> > > > > > > +                             #address-cells = <1>;
> > > > > > > +                             #size-cells = <0>;
> > > > > >
> > > > > > If we expect a single enpoint, then we don't need the address-cells
> > > > > > and size-cells properties.
> > > > >
> > > > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > > > is kind of genius if not very hacky. They have two "identical" sensors
> > > > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > > > shutdown lines. Since they are identical, they also have the same I2C
> > > > > address. I haven't figured out how to model this in the device tree.
> > > > >
> > > > > The point is, it's perfectly possible to have two or more sensors use
> > > > > the same controller, provided only one be active at a time.
> > > >
> > > > Right, but I guess the common case would be to have a single sensor,
> > > > where that wouldn't be needed.
> > > >
> > > > In odd cases, we can always specify it in the DTS, and if it becomes
> > > > common enough, we can move it to the DTSI.
> > >
> > > Makes sense. Do you want me to re-spin?
> >
> > If there's no other comment, we'll fix it when applying.
>
> This patch series seems to have been forgotten. It doesn't seem there are any
> blockers.

Sorry about that :/

> Can you please apply it now? I have some further series (camera module
> support for TBS-A711) that depend on this.

Some parts of it will have to be merged through v4l2, and I can't
apply those patches.

Can you resend that series, and ping on a regular basis (like once a
week) if you don't get any feedback?

Thanks!
Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Ondřej Jirman]

Hi Maxime,

On Mon, May 20, 2019 at 01:10:49PM +0200, Maxime Ripard wrote:
> Hi Ondřej,
> 
> On Sun, May 19, 2019 at 03:54:22PM +0200, Ondřej Jirman wrote:
> > On Tue, Apr 09, 2019 at 04:52:25PM +0200, Maxime Ripard wrote:
> > > On Tue, Apr 09, 2019 at 04:40:40PM +0800, Chen-Yu Tsai wrote:
> > > > On Tue, Apr 9, 2019 at 4:28 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > > > > > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > > > > > From: Chen-Yu Tsai <wens@csie.org>
> > > > > > > >
> > > > > > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > > > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > > > > > difference seems to be that support of MIPI CSI through a connected
> > > > > > > > MIPI CSI-2 bridge.
> > > > > > > >
> > > > > > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > > > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > > > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > > > > > unused.
> > > > > > > >
> > > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > > > > > ---
> > > > > > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > > > > > >  1 file changed, 31 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > index f739b88efb53..0c52f945fd5f 100644
> > > > > > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > @@ -682,6 +682,20 @@
> > > > > > > >                       #interrupt-cells = <3>;
> > > > > > > >                       #gpio-cells = <3>;
> > > > > > > >
> > > > > > > > +                     /omit-if-no-ref/
> > > > > > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > > > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > > > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > > > > > +                                    "PE12", "PE13";
> > > > > > > > +                             function = "csi";
> > > > > > > > +                     };
> > > > > > > > +
> > > > > > > > +                     /omit-if-no-ref/
> > > > > > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > > > > > +                             pins = "PE1";
> > > > > > > > +                             function = "csi";
> > > > > > > > +                     };
> > > > > > > > +
> > > > > > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > > > > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > > > > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > > > > > @@ -994,6 +1008,23 @@
> > > > > > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > > > > > >               };
> > > > > > > >
> > > > > > > > +             csi: camera@1cb0000 {
> > > > > > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > > > > > +                     status = "disabled";
> > > > > > > > +
> > > > > > > > +                     csi_in: port {
> > > > > > > > +                             #address-cells = <1>;
> > > > > > > > +                             #size-cells = <0>;
> > > > > > >
> > > > > > > If we expect a single enpoint, then we don't need the address-cells
> > > > > > > and size-cells properties.
> > > > > >
> > > > > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > > > > is kind of genius if not very hacky. They have two "identical" sensors
> > > > > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > > > > shutdown lines. Since they are identical, they also have the same I2C
> > > > > > address. I haven't figured out how to model this in the device tree.
> > > > > >
> > > > > > The point is, it's perfectly possible to have two or more sensors use
> > > > > > the same controller, provided only one be active at a time.
> > > > >
> > > > > Right, but I guess the common case would be to have a single sensor,
> > > > > where that wouldn't be needed.
> > > > >
> > > > > In odd cases, we can always specify it in the DTS, and if it becomes
> > > > > common enough, we can move it to the DTSI.
> > > >
> > > > Makes sense. Do you want me to re-spin?
> > >
> > > If there's no other comment, we'll fix it when applying.
> >
> > This patch series seems to have been forgotten. It doesn't seem there are any
> > blockers.
> 
> Sorry about that :/
> 
> > Can you please apply it now? I have some further series (camera module
> > support for TBS-A711) that depend on this.
> 
> Some parts of it will have to be merged through v4l2, and I can't
> apply those patches.
> 
> Can you resend that series, and ping on a regular basis (like once a
> week) if you don't get any feedback?

You mean this series for A83t CSI?

regards,
	o.

> Thanks!
> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 6250 bytes --]

On Mon, May 20, 2019 at 01:38:54PM +0200, Ondřej Jirman wrote:
> Hi Maxime,
>
> On Mon, May 20, 2019 at 01:10:49PM +0200, Maxime Ripard wrote:
> > Hi Ondřej,
> >
> > On Sun, May 19, 2019 at 03:54:22PM +0200, Ondřej Jirman wrote:
> > > On Tue, Apr 09, 2019 at 04:52:25PM +0200, Maxime Ripard wrote:
> > > > On Tue, Apr 09, 2019 at 04:40:40PM +0800, Chen-Yu Tsai wrote:
> > > > > On Tue, Apr 9, 2019 at 4:28 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > >
> > > > > > On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > > > > > > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > > > > > > From: Chen-Yu Tsai <wens@csie.org>
> > > > > > > > >
> > > > > > > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > > > > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > > > > > > difference seems to be that support of MIPI CSI through a connected
> > > > > > > > > MIPI CSI-2 bridge.
> > > > > > > > >
> > > > > > > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > > > > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > > > > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > > > > > > unused.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > > > > > > ---
> > > > > > > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > > > > > > >  1 file changed, 31 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > > index f739b88efb53..0c52f945fd5f 100644
> > > > > > > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > > @@ -682,6 +682,20 @@
> > > > > > > > >                       #interrupt-cells = <3>;
> > > > > > > > >                       #gpio-cells = <3>;
> > > > > > > > >
> > > > > > > > > +                     /omit-if-no-ref/
> > > > > > > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > > > > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > > > > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > > > > > > +                                    "PE12", "PE13";
> > > > > > > > > +                             function = "csi";
> > > > > > > > > +                     };
> > > > > > > > > +
> > > > > > > > > +                     /omit-if-no-ref/
> > > > > > > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > > > > > > +                             pins = "PE1";
> > > > > > > > > +                             function = "csi";
> > > > > > > > > +                     };
> > > > > > > > > +
> > > > > > > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > > > > > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > > > > > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > > > > > > @@ -994,6 +1008,23 @@
> > > > > > > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > > > > > > >               };
> > > > > > > > >
> > > > > > > > > +             csi: camera@1cb0000 {
> > > > > > > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > > > > > > +                     status = "disabled";
> > > > > > > > > +
> > > > > > > > > +                     csi_in: port {
> > > > > > > > > +                             #address-cells = <1>;
> > > > > > > > > +                             #size-cells = <0>;
> > > > > > > >
> > > > > > > > If we expect a single enpoint, then we don't need the address-cells
> > > > > > > > and size-cells properties.
> > > > > > >
> > > > > > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > > > > > is kind of genius if not very hacky. They have two "identical" sensors
> > > > > > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > > > > > shutdown lines. Since they are identical, they also have the same I2C
> > > > > > > address. I haven't figured out how to model this in the device tree.
> > > > > > >
> > > > > > > The point is, it's perfectly possible to have two or more sensors use
> > > > > > > the same controller, provided only one be active at a time.
> > > > > >
> > > > > > Right, but I guess the common case would be to have a single sensor,
> > > > > > where that wouldn't be needed.
> > > > > >
> > > > > > In odd cases, we can always specify it in the DTS, and if it becomes
> > > > > > common enough, we can move it to the DTSI.
> > > > >
> > > > > Makes sense. Do you want me to re-spin?
> > > >
> > > > If there's no other comment, we'll fix it when applying.
> > >
> > > This patch series seems to have been forgotten. It doesn't seem there are any
> > > blockers.
> >
> > Sorry about that :/
> >
> > > Can you please apply it now? I have some further series (camera module
> > > support for TBS-A711) that depend on this.
> >
> > Some parts of it will have to be merged through v4l2, and I can't
> > apply those patches.
> >
> > Can you resend that series, and ping on a regular basis (like once a
> > week) if you don't get any feedback?
>
> You mean this series for A83t CSI?

Yes

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/6] ARM: dts: sun8i: a83t: Add device node for CSI (Camera Sensor Interface)

[Ondřej Jirman]

On Mon, May 20, 2019 at 04:05:50PM +0200, Maxime Ripard wrote:
> On Mon, May 20, 2019 at 01:38:54PM +0200, Ondřej Jirman wrote:
> > Hi Maxime,
> >
> > On Mon, May 20, 2019 at 01:10:49PM +0200, Maxime Ripard wrote:
> > > Hi Ondřej,
> > >
> > > On Sun, May 19, 2019 at 03:54:22PM +0200, Ondřej Jirman wrote:
> > > > On Tue, Apr 09, 2019 at 04:52:25PM +0200, Maxime Ripard wrote:
> > > > > On Tue, Apr 09, 2019 at 04:40:40PM +0800, Chen-Yu Tsai wrote:
> > > > > > On Tue, Apr 9, 2019 at 4:28 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > >
> > > > > > > On Tue, Apr 09, 2019 at 04:07:34PM +0800, Chen-Yu Tsai wrote:
> > > > > > > > On Tue, Apr 9, 2019 at 3:58 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > > > > > > On Tue, Apr 09, 2019 at 12:57:42AM +0800, Chen-Yu Tsai wrote:
> > > > > > > > > > From: Chen-Yu Tsai <wens@csie.org>
> > > > > > > > > >
> > > > > > > > > > The A83T SoC has a camera sensor interface (known as CSI in Allwinner
> > > > > > > > > > lingo), which is similar to the one found on the A64 and H3. The only
> > > > > > > > > > difference seems to be that support of MIPI CSI through a connected
> > > > > > > > > > MIPI CSI-2 bridge.
> > > > > > > > > >
> > > > > > > > > > Add a device node for it, and pinctrl nodes for the commonly used MCLK
> > > > > > > > > > and 8-bit parallel interface. The property /omit-if-no-ref/ is added to
> > > > > > > > > > the pinctrl nodes to keep the device tree blob size down if they are
> > > > > > > > > > unused.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Chen-Yu Tsai <wens@csie.org>
> > > > > > > > > > ---
> > > > > > > > > >  arch/arm/boot/dts/sun8i-a83t.dtsi | 31 +++++++++++++++++++++++++++++++
> > > > > > > > > >  1 file changed, 31 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/arch/arm/boot/dts/sun8i-a83t.dtsi b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > > > index f739b88efb53..0c52f945fd5f 100644
> > > > > > > > > > --- a/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > > > +++ b/arch/arm/boot/dts/sun8i-a83t.dtsi
> > > > > > > > > > @@ -682,6 +682,20 @@
> > > > > > > > > >                       #interrupt-cells = <3>;
> > > > > > > > > >                       #gpio-cells = <3>;
> > > > > > > > > >
> > > > > > > > > > +                     /omit-if-no-ref/
> > > > > > > > > > +                     csi_8bit_parallel_pins: csi-8bit-parallel-pins {
> > > > > > > > > > +                             pins = "PE0", "PE2", "PE3", "PE6", "PE7",
> > > > > > > > > > +                                    "PE8", "PE9", "PE10", "PE11",
> > > > > > > > > > +                                    "PE12", "PE13";
> > > > > > > > > > +                             function = "csi";
> > > > > > > > > > +                     };
> > > > > > > > > > +
> > > > > > > > > > +                     /omit-if-no-ref/
> > > > > > > > > > +                     csi_mclk_pin: csi-mclk-pin {
> > > > > > > > > > +                             pins = "PE1";
> > > > > > > > > > +                             function = "csi";
> > > > > > > > > > +                     };
> > > > > > > > > > +
> > > > > > > > > >                       emac_rgmii_pins: emac-rgmii-pins {
> > > > > > > > > >                               pins = "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > > > > > > > > >                                      "PD11", "PD12", "PD13", "PD14", "PD18",
> > > > > > > > > > @@ -994,6 +1008,23 @@
> > > > > > > > > >                       interrupts = <GIC_PPI 9 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> > > > > > > > > >               };
> > > > > > > > > >
> > > > > > > > > > +             csi: camera@1cb0000 {
> > > > > > > > > > +                     compatible = "allwinner,sun8i-a83t-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>;
> > > > > > > > > > +                     status = "disabled";
> > > > > > > > > > +
> > > > > > > > > > +                     csi_in: port {
> > > > > > > > > > +                             #address-cells = <1>;
> > > > > > > > > > +                             #size-cells = <0>;
> > > > > > > > >
> > > > > > > > > If we expect a single enpoint, then we don't need the address-cells
> > > > > > > > > and size-cells properties.
> > > > > > > >
> > > > > > > > I wouldn't bet on anything. The way the Q8 tablets did front/back cameras
> > > > > > > > is kind of genius if not very hacky. They have two "identical" sensors
> > > > > > > > on the same I2C bus and CSI bus, with shared reset line but separate
> > > > > > > > shutdown lines. Since they are identical, they also have the same I2C
> > > > > > > > address. I haven't figured out how to model this in the device tree.
> > > > > > > >
> > > > > > > > The point is, it's perfectly possible to have two or more sensors use
> > > > > > > > the same controller, provided only one be active at a time.
> > > > > > >
> > > > > > > Right, but I guess the common case would be to have a single sensor,
> > > > > > > where that wouldn't be needed.
> > > > > > >
> > > > > > > In odd cases, we can always specify it in the DTS, and if it becomes
> > > > > > > common enough, we can move it to the DTSI.
> > > > > >
> > > > > > Makes sense. Do you want me to re-spin?
> > > > >
> > > > > If there's no other comment, we'll fix it when applying.
> > > >
> > > > This patch series seems to have been forgotten. It doesn't seem there are any
> > > > blockers.
> > >
> > > Sorry about that :/
> > >
> > > > Can you please apply it now? I have some further series (camera module
> > > > support for TBS-A711) that depend on this.
> > >
> > > Some parts of it will have to be merged through v4l2, and I can't
> > > apply those patches.
> > >
> > > Can you resend that series, and ping on a regular basis (like once a
> > > week) if you don't get any feedback?
> >
> > You mean this series for A83t CSI?
> 
> Yes

Ok, done. :)

regards,
	Ondrej

> Maxime
> 
> --
> Maxime Ripard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com



> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel