| Message ID | 20190408165744.11672-5-wens@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: sun8i: a83t: Support Camera Sensor Interface controller | expand |
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
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
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
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?
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
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
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
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 >
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
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
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
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
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
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>;