* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 8:40 ` Yong Deng 0 siblings, 0 replies; 48+ messages in thread From: Yong Deng @ 2018-09-26 8:40 UTC (permalink / raw) To: Maxime Ripard Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Laurent Pinchart, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi, Yong Deng Add binding documentation for Allwinner V3s CSI. Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Yong Deng <yong.deng@magewell.com> --- .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode 100644 index 000000000000..2ff47a9507a6 --- /dev/null +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt @@ -0,0 +1,59 @@ +Allwinner V3s Camera Sensor Interface +------------------------------------- + +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 +interface and CSI1 is used for parallel interface. + +Required properties: + - compatible: value must be "allwinner,sun8i-v3s-csi" + - reg: base address and size of the memory-mapped region. + - interrupts: interrupt associated to this IP + - clocks: phandles to the clocks feeding the CSI + * bus: the CSI interface clock + * mod: the CSI module clock + * ram: the CSI DRAM clock + - clock-names: the clock names mentioned above + - resets: phandles to the reset line driving the CSI + +Each CSI node should contain one 'port' child node with one child 'endpoint' +node, according to the bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or +Bt656 for CSI1. + +Endpoint node properties for CSI1 +--------------------------------- + +- remote-endpoint : (required) a phandle to the bus receiver's endpoint + node +- bus-width: : (required) must be 8, 10, 12 or 16 +- pclk-sample : (optional) (default: sample on falling edge) +- hsync-active : (only required for parallel) +- vsync-active : (only required for parallel) + +Example: + +csi1: csi@1cb4000 { + compatible = "allwinner,sun8i-v3s-csi"; + reg = <0x01cb4000 0x1000>; + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&ccu CLK_BUS_CSI>, + <&ccu CLK_CSI1_SCLK>, + <&ccu CLK_DRAM_CSI>; + clock-names = "bus", "mod", "ram"; + resets = <&ccu RST_BUS_CSI>; + + port { + /* Parallel bus endpoint */ + csi1_ep: endpoint { + remote-endpoint = <&adv7611_ep>; + bus-width = <16>; + + /* If hsync-active/vsync-active are missing, + embedded BT.656 sync is used */ + hsync-active = <0>; /* Active low */ + vsync-active = <0>; /* Active low */ + pclk-sample = <1>; /* Rising */ + }; + }; +}; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 8:40 ` Yong Deng 0 siblings, 0 replies; 48+ messages in thread From: Yong Deng @ 2018-09-26 8:40 UTC (permalink / raw) To: linux-arm-kernel Add binding documentation for Allwinner V3s CSI. Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Yong Deng <yong.deng@magewell.com> --- .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode 100644 index 000000000000..2ff47a9507a6 --- /dev/null +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt @@ -0,0 +1,59 @@ +Allwinner V3s Camera Sensor Interface +------------------------------------- + +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 +interface and CSI1 is used for parallel interface. + +Required properties: + - compatible: value must be "allwinner,sun8i-v3s-csi" + - reg: base address and size of the memory-mapped region. + - interrupts: interrupt associated to this IP + - clocks: phandles to the clocks feeding the CSI + * bus: the CSI interface clock + * mod: the CSI module clock + * ram: the CSI DRAM clock + - clock-names: the clock names mentioned above + - resets: phandles to the reset line driving the CSI + +Each CSI node should contain one 'port' child node with one child 'endpoint' +node, according to the bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or +Bt656 for CSI1. + +Endpoint node properties for CSI1 +--------------------------------- + +- remote-endpoint : (required) a phandle to the bus receiver's endpoint + node +- bus-width: : (required) must be 8, 10, 12 or 16 +- pclk-sample : (optional) (default: sample on falling edge) +- hsync-active : (only required for parallel) +- vsync-active : (only required for parallel) + +Example: + +csi1: csi at 1cb4000 { + compatible = "allwinner,sun8i-v3s-csi"; + reg = <0x01cb4000 0x1000>; + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&ccu CLK_BUS_CSI>, + <&ccu CLK_CSI1_SCLK>, + <&ccu CLK_DRAM_CSI>; + clock-names = "bus", "mod", "ram"; + resets = <&ccu RST_BUS_CSI>; + + port { + /* Parallel bus endpoint */ + csi1_ep: endpoint { + remote-endpoint = <&adv7611_ep>; + bus-width = <16>; + + /* If hsync-active/vsync-active are missing, + embedded BT.656 sync is used */ + hsync-active = <0>; /* Active low */ + vsync-active = <0>; /* Active low */ + pclk-sample = <1>; /* Rising */ + }; + }; +}; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 8:40 ` Yong Deng 0 siblings, 0 replies; 48+ messages in thread From: Yong Deng @ 2018-09-26 8:40 UTC (permalink / raw) To: Maxime Ripard Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Laurent Pinchart, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Yong Deng Add binding documentation for Allwinner V3s CSI. Acked-by: Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Signed-off-by: Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org> --- .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode 100644 index 000000000000..2ff47a9507a6 --- /dev/null +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt @@ -0,0 +1,59 @@ +Allwinner V3s Camera Sensor Interface +------------------------------------- + +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 +interface and CSI1 is used for parallel interface. + +Required properties: + - compatible: value must be "allwinner,sun8i-v3s-csi" + - reg: base address and size of the memory-mapped region. + - interrupts: interrupt associated to this IP + - clocks: phandles to the clocks feeding the CSI + * bus: the CSI interface clock + * mod: the CSI module clock + * ram: the CSI DRAM clock + - clock-names: the clock names mentioned above + - resets: phandles to the reset line driving the CSI + +Each CSI node should contain one 'port' child node with one child 'endpoint' +node, according to the bindings defined in +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or +Bt656 for CSI1. + +Endpoint node properties for CSI1 +--------------------------------- + +- remote-endpoint : (required) a phandle to the bus receiver's endpoint + node +- bus-width: : (required) must be 8, 10, 12 or 16 +- pclk-sample : (optional) (default: sample on falling edge) +- hsync-active : (only required for parallel) +- vsync-active : (only required for parallel) + +Example: + +csi1: csi@1cb4000 { + compatible = "allwinner,sun8i-v3s-csi"; + reg = <0x01cb4000 0x1000>; + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&ccu CLK_BUS_CSI>, + <&ccu CLK_CSI1_SCLK>, + <&ccu CLK_DRAM_CSI>; + clock-names = "bus", "mod", "ram"; + resets = <&ccu RST_BUS_CSI>; + + port { + /* Parallel bus endpoint */ + csi1_ep: endpoint { + remote-endpoint = <&adv7611_ep>; + bus-width = <16>; + + /* If hsync-active/vsync-active are missing, + embedded BT.656 sync is used */ + hsync-active = <0>; /* Active low */ + vsync-active = <0>; /* Active low */ + pclk-sample = <1>; /* Rising */ + }; + }; +}; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-26 8:40 ` Yong Deng (?) @ 2018-09-26 10:19 ` Laurent Pinchart -1 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-26 10:19 UTC (permalink / raw) To: Yong Deng Cc: Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Yong, Thank you for the patch. On Wednesday, 26 September 2018 11:40:04 EEST Yong Deng wrote: > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child > 'endpoint' +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > or > +Bt656 for CSI1. Nitpicking, BT.656 is parallel as well. We should use BT.601 instead of parallel, to differentiate between embedded sync and external sync. This is a problem in Documentation/devicetree/bindings/media/video-interfaces.txt in the first place, so I won't ask you to modify these bindings. I'll try to submit a patch for video-interfaces.txt. > +Endpoint node properties for CSI1 > +--------------------------------- Should you list the CSI0 properties as well ? As the driver in patch 2/2 doesn't support the CSI-2 interface I assume you have left out CSI0 for now, but it should still be listed in the bindings. I'm fine with fixing this as a follow-up patch to avoid missing the v4.20 merge window, but if you end up resubmitting the series, could you please address the problem ? With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > + node > +- bus-width: : (required) must be 8, 10, 12 or 16 > +- pclk-sample : (optional) (default: sample on falling edge) > +- hsync-active : (only required for parallel) > +- vsync-active : (only required for parallel) > > +Example: > + > +csi1: csi@1cb4000 { > + compatible = "allwinner,sun8i-v3s-csi"; > + reg = <0x01cb4000 0x1000>; > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&ccu CLK_BUS_CSI>, > + <&ccu CLK_CSI1_SCLK>, > + <&ccu CLK_DRAM_CSI>; > + clock-names = "bus", "mod", "ram"; > + resets = <&ccu RST_BUS_CSI>; > + > + port { > + /* Parallel bus endpoint */ > + csi1_ep: endpoint { > + remote-endpoint = <&adv7611_ep>; > + bus-width = <16>; > + > + /* If hsync-active/vsync-active are missing, > + embedded BT.656 sync is used */ > + hsync-active = <0>; /* Active low */ > + vsync-active = <0>; /* Active low */ > + pclk-sample = <1>; /* Rising */ > + }; > + }; > +}; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:19 ` Laurent Pinchart 0 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-26 10:19 UTC (permalink / raw) To: linux-arm-kernel Hi Yong, Thank you for the patch. On Wednesday, 26 September 2018 11:40:04 EEST Yong Deng wrote: > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child > 'endpoint' +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > or > +Bt656 for CSI1. Nitpicking, BT.656 is parallel as well. We should use BT.601 instead of parallel, to differentiate between embedded sync and external sync. This is a problem in Documentation/devicetree/bindings/media/video-interfaces.txt in the first place, so I won't ask you to modify these bindings. I'll try to submit a patch for video-interfaces.txt. > +Endpoint node properties for CSI1 > +--------------------------------- Should you list the CSI0 properties as well ? As the driver in patch 2/2 doesn't support the CSI-2 interface I assume you have left out CSI0 for now, but it should still be listed in the bindings. I'm fine with fixing this as a follow-up patch to avoid missing the v4.20 merge window, but if you end up resubmitting the series, could you please address the problem ? With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > + node > +- bus-width: : (required) must be 8, 10, 12 or 16 > +- pclk-sample : (optional) (default: sample on falling edge) > +- hsync-active : (only required for parallel) > +- vsync-active : (only required for parallel) > > +Example: > + > +csi1: csi at 1cb4000 { > + compatible = "allwinner,sun8i-v3s-csi"; > + reg = <0x01cb4000 0x1000>; > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&ccu CLK_BUS_CSI>, > + <&ccu CLK_CSI1_SCLK>, > + <&ccu CLK_DRAM_CSI>; > + clock-names = "bus", "mod", "ram"; > + resets = <&ccu RST_BUS_CSI>; > + > + port { > + /* Parallel bus endpoint */ > + csi1_ep: endpoint { > + remote-endpoint = <&adv7611_ep>; > + bus-width = <16>; > + > + /* If hsync-active/vsync-active are missing, > + embedded BT.656 sync is used */ > + hsync-active = <0>; /* Active low */ > + vsync-active = <0>; /* Active low */ > + pclk-sample = <1>; /* Rising */ > + }; > + }; > +}; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:19 ` Laurent Pinchart 0 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-26 10:19 UTC (permalink / raw) To: Yong Deng Cc: Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel Hi Yong, Thank you for the patch. On Wednesday, 26 September 2018 11:40:04 EEST Yong Deng wrote: > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child > 'endpoint' +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > or > +Bt656 for CSI1. Nitpicking, BT.656 is parallel as well. We should use BT.601 instead of parallel, to differentiate between embedded sync and external sync. This is a problem in Documentation/devicetree/bindings/media/video-interfaces.txt in the first place, so I won't ask you to modify these bindings. I'll try to submit a patch for video-interfaces.txt. > +Endpoint node properties for CSI1 > +--------------------------------- Should you list the CSI0 properties as well ? As the driver in patch 2/2 doesn't support the CSI-2 interface I assume you have left out CSI0 for now, but it should still be listed in the bindings. I'm fine with fixing this as a follow-up patch to avoid missing the v4.20 merge window, but if you end up resubmitting the series, could you please address the problem ? With this fixed, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > + node > +- bus-width: : (required) must be 8, 10, 12 or 16 > +- pclk-sample : (optional) (default: sample on falling edge) > +- hsync-active : (only required for parallel) > +- vsync-active : (only required for parallel) > > +Example: > + > +csi1: csi@1cb4000 { > + compatible = "allwinner,sun8i-v3s-csi"; > + reg = <0x01cb4000 0x1000>; > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&ccu CLK_BUS_CSI>, > + <&ccu CLK_CSI1_SCLK>, > + <&ccu CLK_DRAM_CSI>; > + clock-names = "bus", "mod", "ram"; > + resets = <&ccu RST_BUS_CSI>; > + > + port { > + /* Parallel bus endpoint */ > + csi1_ep: endpoint { > + remote-endpoint = <&adv7611_ep>; > + bus-width = <16>; > + > + /* If hsync-active/vsync-active are missing, > + embedded BT.656 sync is used */ > + hsync-active = <0>; /* Active low */ > + vsync-active = <0>; /* Active low */ > + pclk-sample = <1>; /* Rising */ > + }; > + }; > +}; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-26 10:19 ` Laurent Pinchart (?) @ 2018-09-26 10:35 ` maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ -1 siblings, 0 replies; 48+ messages in thread From: maxime.ripard @ 2018-09-26 10:35 UTC (permalink / raw) To: Laurent Pinchart Cc: Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > +Endpoint node properties for CSI1 > > +--------------------------------- > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > doesn't support the CSI-2 interface I assume you have left out CSI0 for now, > but it should still be listed in the bindings. I'm fine with fixing this as a > follow-up patch to avoid missing the v4.20 merge window, but if you end up > resubmitting the series, could you please address the problem ? That driver is not available, and the documentation isn't either, so there's no easy way to tell which properties are going to be needed before doing the actual work of reverse engineering it and writing a driver for it. Unfortunately... Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:35 ` maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ 0 siblings, 0 replies; 48+ messages in thread From: maxime.ripard at bootlin.com @ 2018-09-26 10:35 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > +Endpoint node properties for CSI1 > > +--------------------------------- > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > doesn't support the CSI-2 interface I assume you have left out CSI0 for now, > but it should still be listed in the bindings. I'm fine with fixing this as a > follow-up patch to avoid missing the v4.20 merge window, but if you end up > resubmitting the series, could you please address the problem ? That driver is not available, and the documentation isn't either, so there's no easy way to tell which properties are going to be needed before doing the actual work of reverse engineering it and writing a driver for it. Unfortunately... Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:35 ` maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ 0 siblings, 0 replies; 48+ messages in thread From: maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ @ 2018-09-26 10:35 UTC (permalink / raw) To: Laurent Pinchart Cc: Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > +Endpoint node properties for CSI1 > > +--------------------------------- > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > doesn't support the CSI-2 interface I assume you have left out CSI0 for now, > but it should still be listed in the bindings. I'm fine with fixing this as a > follow-up patch to avoid missing the v4.20 merge window, but if you end up > resubmitting the series, could you please address the problem ? That driver is not available, and the documentation isn't either, so there's no easy way to tell which properties are going to be needed before doing the actual work of reverse engineering it and writing a driver for it. Unfortunately... Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-26 10:35 ` maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ (?) @ 2018-09-26 10:38 ` Laurent Pinchart -1 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-26 10:38 UTC (permalink / raw) To: maxime.ripard Cc: Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Maxime, On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard@bootlin.com wrote: > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > +Endpoint node properties for CSI1 > > > +--------------------------------- > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > now, but it should still be listed in the bindings. I'm fine with fixing > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > you end up resubmitting the series, could you please address the problem > > ? > > That driver is not available, and the documentation isn't either, so > there's no easy way to tell which properties are going to be needed > before doing the actual work of reverse engineering it and writing a > driver for it. Unfortunately... While DT bindings should be independent from driver implementations, I agree it's difficult to develop good bindings without hardware documentation and without at least one working driver implementation. How about just explicitly stating that these bindings don't support CSI0 yet ? -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:38 ` Laurent Pinchart 0 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-26 10:38 UTC (permalink / raw) To: linux-arm-kernel Hi Maxime, On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard at bootlin.com wrote: > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > +Endpoint node properties for CSI1 > > > +--------------------------------- > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > now, but it should still be listed in the bindings. I'm fine with fixing > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > you end up resubmitting the series, could you please address the problem > > ? > > That driver is not available, and the documentation isn't either, so > there's no easy way to tell which properties are going to be needed > before doing the actual work of reverse engineering it and writing a > driver for it. Unfortunately... While DT bindings should be independent from driver implementations, I agree it's difficult to develop good bindings without hardware documentation and without at least one working driver implementation. How about just explicitly stating that these bindings don't support CSI0 yet ? -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:38 ` Laurent Pinchart 0 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-26 10:38 UTC (permalink / raw) To: maxime.ripard Cc: Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel Hi Maxime, On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard@bootlin.com wrote: > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > +Endpoint node properties for CSI1 > > > +--------------------------------- > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > now, but it should still be listed in the bindings. I'm fine with fixing > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > you end up resubmitting the series, could you please address the problem > > ? > > That driver is not available, and the documentation isn't either, so > there's no easy way to tell which properties are going to be needed > before doing the actual work of reverse engineering it and writing a > driver for it. Unfortunately... While DT bindings should be independent from driver implementations, I agree it's difficult to develop good bindings without hardware documentation and without at least one working driver implementation. How about just explicitly stating that these bindings don't support CSI0 yet ? -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-26 10:38 ` Laurent Pinchart (?) @ 2018-09-26 10:40 ` Maxime Ripard -1 siblings, 0 replies; 48+ messages in thread From: Maxime Ripard @ 2018-09-26 10:40 UTC (permalink / raw) To: Laurent Pinchart Cc: Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Wed, Sep 26, 2018 at 01:38:08PM +0300, Laurent Pinchart wrote: > Hi Maxime, > > On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard@bootlin.com wrote: > > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > > +Endpoint node properties for CSI1 > > > > +--------------------------------- > > > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > > now, but it should still be listed in the bindings. I'm fine with fixing > > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > > you end up resubmitting the series, could you please address the problem > > > ? > > > > That driver is not available, and the documentation isn't either, so > > there's no easy way to tell which properties are going to be needed > > before doing the actual work of reverse engineering it and writing a > > driver for it. Unfortunately... > > While DT bindings should be independent from driver implementations, I agree > it's difficult to develop good bindings without hardware documentation and > without at least one working driver implementation. > > How about just explicitly stating that these bindings don't support CSI0 yet ? Yep, that would work -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:40 ` Maxime Ripard 0 siblings, 0 replies; 48+ messages in thread From: Maxime Ripard @ 2018-09-26 10:40 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 26, 2018 at 01:38:08PM +0300, Laurent Pinchart wrote: > Hi Maxime, > > On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard at bootlin.com wrote: > > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > > +Endpoint node properties for CSI1 > > > > +--------------------------------- > > > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > > now, but it should still be listed in the bindings. I'm fine with fixing > > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > > you end up resubmitting the series, could you please address the problem > > > ? > > > > That driver is not available, and the documentation isn't either, so > > there's no easy way to tell which properties are going to be needed > > before doing the actual work of reverse engineering it and writing a > > driver for it. Unfortunately... > > While DT bindings should be independent from driver implementations, I agree > it's difficult to develop good bindings without hardware documentation and > without at least one working driver implementation. > > How about just explicitly stating that these bindings don't support CSI0 yet ? Yep, that would work -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-26 10:40 ` Maxime Ripard 0 siblings, 0 replies; 48+ messages in thread From: Maxime Ripard @ 2018-09-26 10:40 UTC (permalink / raw) To: Laurent Pinchart Cc: Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw On Wed, Sep 26, 2018 at 01:38:08PM +0300, Laurent Pinchart wrote: > Hi Maxime, > > On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org wrote: > > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > > +Endpoint node properties for CSI1 > > > > +--------------------------------- > > > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > > now, but it should still be listed in the bindings. I'm fine with fixing > > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > > you end up resubmitting the series, could you please address the problem > > > ? > > > > That driver is not available, and the documentation isn't either, so > > there's no easy way to tell which properties are going to be needed > > before doing the actual work of reverse engineering it and writing a > > driver for it. Unfortunately... > > While DT bindings should be independent from driver implementations, I agree > it's difficult to develop good bindings without hardware documentation and > without at least one working driver implementation. > > How about just explicitly stating that these bindings don't support CSI0 yet ? Yep, that would work -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-26 10:38 ` Laurent Pinchart (?) @ 2018-09-27 0:59 ` Yong -1 siblings, 0 replies; 48+ messages in thread From: Yong @ 2018-09-27 0:59 UTC (permalink / raw) To: Laurent Pinchart Cc: maxime.ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi, On Wed, 26 Sep 2018 13:38:08 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Maxime, > > On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard@bootlin.com wrote: > > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > > +Endpoint node properties for CSI1 > > > > +--------------------------------- > > > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > > now, but it should still be listed in the bindings. I'm fine with fixing > > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > > you end up resubmitting the series, could you please address the problem > > > ? > > > > That driver is not available, and the documentation isn't either, so > > there's no easy way to tell which properties are going to be needed > > before doing the actual work of reverse engineering it and writing a > > driver for it. Unfortunately... > > While DT bindings should be independent from driver implementations, I agree > it's difficult to develop good bindings without hardware documentation and > without at least one working driver implementation. > > How about just explicitly stating that these bindings don't support CSI0 yet ? OK. Thanks, Yong ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-27 0:59 ` Yong 0 siblings, 0 replies; 48+ messages in thread From: Yong @ 2018-09-27 0:59 UTC (permalink / raw) To: linux-arm-kernel Hi, On Wed, 26 Sep 2018 13:38:08 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Maxime, > > On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard at bootlin.com wrote: > > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > > +Endpoint node properties for CSI1 > > > > +--------------------------------- > > > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > > now, but it should still be listed in the bindings. I'm fine with fixing > > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > > you end up resubmitting the series, could you please address the problem > > > ? > > > > That driver is not available, and the documentation isn't either, so > > there's no easy way to tell which properties are going to be needed > > before doing the actual work of reverse engineering it and writing a > > driver for it. Unfortunately... > > While DT bindings should be independent from driver implementations, I agree > it's difficult to develop good bindings without hardware documentation and > without at least one working driver implementation. > > How about just explicitly stating that these bindings don't support CSI0 yet ? OK. Thanks, Yong ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-27 0:59 ` Yong 0 siblings, 0 replies; 48+ messages in thread From: Yong @ 2018-09-27 0:59 UTC (permalink / raw) To: Laurent Pinchart Cc: maxime.ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel Hi, On Wed, 26 Sep 2018 13:38:08 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Maxime, > > On Wednesday, 26 September 2018 13:35:47 EEST maxime.ripard@bootlin.com wrote: > > On Wed, Sep 26, 2018 at 01:19:34PM +0300, Laurent Pinchart wrote: > > > > +Endpoint node properties for CSI1 > > > > +--------------------------------- > > > > > > Should you list the CSI0 properties as well ? As the driver in patch 2/2 > > > doesn't support the CSI-2 interface I assume you have left out CSI0 for > > > now, but it should still be listed in the bindings. I'm fine with fixing > > > this as a follow-up patch to avoid missing the v4.20 merge window, but if > > > you end up resubmitting the series, could you please address the problem > > > ? > > > > That driver is not available, and the documentation isn't either, so > > there's no easy way to tell which properties are going to be needed > > before doing the actual work of reverse engineering it and writing a > > driver for it. Unfortunately... > > While DT bindings should be independent from driver implementations, I agree > it's difficult to develop good bindings without hardware documentation and > without at least one working driver implementation. > > How about just explicitly stating that these bindings don't support CSI0 yet ? OK. Thanks, Yong ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-26 8:40 ` Yong Deng (?) @ 2018-09-28 9:38 ` Sakari Ailus -1 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 9:38 UTC (permalink / raw) To: Yong Deng Cc: Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Laurent Pinchart, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Yong, On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> I know... but I have a few more comments. > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt > new file mode 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child 'endpoint' > +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or > +Bt656 for CSI1. Which port represents CSI0 and which one is CSI1? That needs to be documented. > + > +Endpoint node properties for CSI1 How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if the hardware supports lane mapping. > +--------------------------------- > + > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > + node > +- bus-width: : (required) must be 8, 10, 12 or 16 > +- pclk-sample : (optional) (default: sample on falling edge) Could you add that video-interfaces.txt contains documentation of these properties as well? There's a reference above but only discusses port and endpoint nodes. > +- hsync-active : (only required for parallel) > +- vsync-active : (only required for parallel) As you support both Bt656 and parallel (with sync signals), you can detect the interface type from the presence of these properties. I think you should also say that these properties are not allowed on Bt656. So I'd change this to e.g. (required; parallel-only) > + > +Example: > + > +csi1: csi@1cb4000 { > + compatible = "allwinner,sun8i-v3s-csi"; > + reg = <0x01cb4000 0x1000>; > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&ccu CLK_BUS_CSI>, > + <&ccu CLK_CSI1_SCLK>, > + <&ccu CLK_DRAM_CSI>; > + clock-names = "bus", "mod", "ram"; > + resets = <&ccu RST_BUS_CSI>; > + > + port { > + /* Parallel bus endpoint */ > + csi1_ep: endpoint { > + remote-endpoint = <&adv7611_ep>; > + bus-width = <16>; > + > + /* If hsync-active/vsync-active are missing, > + embedded BT.656 sync is used */ > + hsync-active = <0>; /* Active low */ > + vsync-active = <0>; /* Active low */ > + pclk-sample = <1>; /* Rising */ > + }; > + }; > +}; -- Kind regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 9:38 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 9:38 UTC (permalink / raw) To: linux-arm-kernel Hi Yong, On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> I know... but I have a few more comments. > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt > new file mode 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child 'endpoint' > +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or > +Bt656 for CSI1. Which port represents CSI0 and which one is CSI1? That needs to be documented. > + > +Endpoint node properties for CSI1 How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if the hardware supports lane mapping. > +--------------------------------- > + > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > + node > +- bus-width: : (required) must be 8, 10, 12 or 16 > +- pclk-sample : (optional) (default: sample on falling edge) Could you add that video-interfaces.txt contains documentation of these properties as well? There's a reference above but only discusses port and endpoint nodes. > +- hsync-active : (only required for parallel) > +- vsync-active : (only required for parallel) As you support both Bt656 and parallel (with sync signals), you can detect the interface type from the presence of these properties. I think you should also say that these properties are not allowed on Bt656. So I'd change this to e.g. (required; parallel-only) > + > +Example: > + > +csi1: csi at 1cb4000 { > + compatible = "allwinner,sun8i-v3s-csi"; > + reg = <0x01cb4000 0x1000>; > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&ccu CLK_BUS_CSI>, > + <&ccu CLK_CSI1_SCLK>, > + <&ccu CLK_DRAM_CSI>; > + clock-names = "bus", "mod", "ram"; > + resets = <&ccu RST_BUS_CSI>; > + > + port { > + /* Parallel bus endpoint */ > + csi1_ep: endpoint { > + remote-endpoint = <&adv7611_ep>; > + bus-width = <16>; > + > + /* If hsync-active/vsync-active are missing, > + embedded BT.656 sync is used */ > + hsync-active = <0>; /* Active low */ > + vsync-active = <0>; /* Active low */ > + pclk-sample = <1>; /* Rising */ > + }; > + }; > +}; -- Kind regards, Sakari Ailus sakari.ailus at linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 9:38 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 9:38 UTC (permalink / raw) To: Yong Deng Cc: Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Laurent Pinchart, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, linux-media, devicetree, linux-arm-ke Hi Yong, On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> I know... but I have a few more comments. > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt > new file mode 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child 'endpoint' > +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or > +Bt656 for CSI1. Which port represents CSI0 and which one is CSI1? That needs to be documented. > + > +Endpoint node properties for CSI1 How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if the hardware supports lane mapping. > +--------------------------------- > + > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > + node > +- bus-width: : (required) must be 8, 10, 12 or 16 > +- pclk-sample : (optional) (default: sample on falling edge) Could you add that video-interfaces.txt contains documentation of these properties as well? There's a reference above but only discusses port and endpoint nodes. > +- hsync-active : (only required for parallel) > +- vsync-active : (only required for parallel) As you support both Bt656 and parallel (with sync signals), you can detect the interface type from the presence of these properties. I think you should also say that these properties are not allowed on Bt656. So I'd change this to e.g. (required; parallel-only) > + > +Example: > + > +csi1: csi@1cb4000 { > + compatible = "allwinner,sun8i-v3s-csi"; > + reg = <0x01cb4000 0x1000>; > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&ccu CLK_BUS_CSI>, > + <&ccu CLK_CSI1_SCLK>, > + <&ccu CLK_DRAM_CSI>; > + clock-names = "bus", "mod", "ram"; > + resets = <&ccu RST_BUS_CSI>; > + > + port { > + /* Parallel bus endpoint */ > + csi1_ep: endpoint { > + remote-endpoint = <&adv7611_ep>; > + bus-width = <16>; > + > + /* If hsync-active/vsync-active are missing, > + embedded BT.656 sync is used */ > + hsync-active = <0>; /* Active low */ > + vsync-active = <0>; /* Active low */ > + pclk-sample = <1>; /* Rising */ > + }; > + }; > +}; -- Kind regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-28 9:38 ` Sakari Ailus (?) @ 2018-09-28 9:45 ` Laurent Pinchart -1 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-28 9:45 UTC (permalink / raw) To: Sakari Ailus Cc: Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Sakari, On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > Add binding documentation for Allwinner V3s CSI. > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > I know... but I have a few more comments. > > > Reviewed-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > --- > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > 1 file changed, 59 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > 100644 > > index 000000000000..2ff47a9507a6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > @@ -0,0 +1,59 @@ > > +Allwinner V3s Camera Sensor Interface > > +------------------------------------- > > + > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > +interface and CSI1 is used for parallel interface. > > + > > +Required properties: > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > + - reg: base address and size of the memory-mapped region. > > + - interrupts: interrupt associated to this IP > > + - clocks: phandles to the clocks feeding the CSI > > + * bus: the CSI interface clock > > + * mod: the CSI module clock > > + * ram: the CSI DRAM clock > > + - clock-names: the clock names mentioned above > > + - resets: phandles to the reset line driving the CSI > > + > > +Each CSI node should contain one 'port' child node with one child > > 'endpoint' +node, according to the bindings defined in > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > mentioned > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > or +Bt656 for CSI1. > > Which port represents CSI0 and which one is CSI1? That needs to be > documented. There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) only. > > + > > +Endpoint node properties for CSI1 > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > the hardware supports lane mapping. I enquired about that too. As far as I understand, CSI0 isn't supported yet in the driver due to lack of documentation and lack of open-source vendor- provided source code. While DT bindings are not tied to driver implementations, it's not the best idea to design DT bindings without at least one working implementation to test them. I thus proposed just listing CSI0 as being unsupported for now. > > +--------------------------------- > > + > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > + node > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > +- pclk-sample : (optional) (default: sample on falling edge) > > Could you add that video-interfaces.txt contains documentation of these > properties as well? There's a reference above but only discusses port and > endpoint nodes. > > > +- hsync-active : (only required for parallel) > > +- vsync-active : (only required for parallel) > > As you support both Bt656 and parallel (with sync signals), you can detect > the interface type from the presence of these properties. I think you > should also say that these properties are not allowed on Bt656. So I'd > change this to e.g. > > (required; parallel-only) > > > + > > +Example: > > + > > +csi1: csi@1cb4000 { > > + compatible = "allwinner,sun8i-v3s-csi"; > > + reg = <0x01cb4000 0x1000>; > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&ccu CLK_BUS_CSI>, > > + <&ccu CLK_CSI1_SCLK>, > > + <&ccu CLK_DRAM_CSI>; > > + clock-names = "bus", "mod", "ram"; > > + resets = <&ccu RST_BUS_CSI>; > > + > > + port { > > + /* Parallel bus endpoint */ > > + csi1_ep: endpoint { > > + remote-endpoint = <&adv7611_ep>; > > + bus-width = <16>; > > + > > + /* If hsync-active/vsync-active are missing, > > + embedded BT.656 sync is used */ > > + hsync-active = <0>; /* Active low */ > > + vsync-active = <0>; /* Active low */ > > + pclk-sample = <1>; /* Rising */ > > + }; > > + }; > > +}; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 9:45 ` Laurent Pinchart 0 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-28 9:45 UTC (permalink / raw) To: linux-arm-kernel Hi Sakari, On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > Add binding documentation for Allwinner V3s CSI. > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > I know... but I have a few more comments. > > > Reviewed-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > --- > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > 1 file changed, 59 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > 100644 > > index 000000000000..2ff47a9507a6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > @@ -0,0 +1,59 @@ > > +Allwinner V3s Camera Sensor Interface > > +------------------------------------- > > + > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > +interface and CSI1 is used for parallel interface. > > + > > +Required properties: > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > + - reg: base address and size of the memory-mapped region. > > + - interrupts: interrupt associated to this IP > > + - clocks: phandles to the clocks feeding the CSI > > + * bus: the CSI interface clock > > + * mod: the CSI module clock > > + * ram: the CSI DRAM clock > > + - clock-names: the clock names mentioned above > > + - resets: phandles to the reset line driving the CSI > > + > > +Each CSI node should contain one 'port' child node with one child > > 'endpoint' +node, according to the bindings defined in > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > mentioned > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > or +Bt656 for CSI1. > > Which port represents CSI0 and which one is CSI1? That needs to be > documented. There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) only. > > + > > +Endpoint node properties for CSI1 > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > the hardware supports lane mapping. I enquired about that too. As far as I understand, CSI0 isn't supported yet in the driver due to lack of documentation and lack of open-source vendor- provided source code. While DT bindings are not tied to driver implementations, it's not the best idea to design DT bindings without at least one working implementation to test them. I thus proposed just listing CSI0 as being unsupported for now. > > +--------------------------------- > > + > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > + node > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > +- pclk-sample : (optional) (default: sample on falling edge) > > Could you add that video-interfaces.txt contains documentation of these > properties as well? There's a reference above but only discusses port and > endpoint nodes. > > > +- hsync-active : (only required for parallel) > > +- vsync-active : (only required for parallel) > > As you support both Bt656 and parallel (with sync signals), you can detect > the interface type from the presence of these properties. I think you > should also say that these properties are not allowed on Bt656. So I'd > change this to e.g. > > (required; parallel-only) > > > + > > +Example: > > + > > +csi1: csi at 1cb4000 { > > + compatible = "allwinner,sun8i-v3s-csi"; > > + reg = <0x01cb4000 0x1000>; > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&ccu CLK_BUS_CSI>, > > + <&ccu CLK_CSI1_SCLK>, > > + <&ccu CLK_DRAM_CSI>; > > + clock-names = "bus", "mod", "ram"; > > + resets = <&ccu RST_BUS_CSI>; > > + > > + port { > > + /* Parallel bus endpoint */ > > + csi1_ep: endpoint { > > + remote-endpoint = <&adv7611_ep>; > > + bus-width = <16>; > > + > > + /* If hsync-active/vsync-active are missing, > > + embedded BT.656 sync is used */ > > + hsync-active = <0>; /* Active low */ > > + vsync-active = <0>; /* Active low */ > > + pclk-sample = <1>; /* Rising */ > > + }; > > + }; > > +}; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 9:45 ` Laurent Pinchart 0 siblings, 0 replies; 48+ messages in thread From: Laurent Pinchart @ 2018-09-28 9:45 UTC (permalink / raw) To: Sakari Ailus Cc: Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, linux-media, devicetree, linux-arm-kernel Hi Sakari, On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > Add binding documentation for Allwinner V3s CSI. > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > I know... but I have a few more comments. > > > Reviewed-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > --- > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > 1 file changed, 59 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > 100644 > > index 000000000000..2ff47a9507a6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > @@ -0,0 +1,59 @@ > > +Allwinner V3s Camera Sensor Interface > > +------------------------------------- > > + > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > +interface and CSI1 is used for parallel interface. > > + > > +Required properties: > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > + - reg: base address and size of the memory-mapped region. > > + - interrupts: interrupt associated to this IP > > + - clocks: phandles to the clocks feeding the CSI > > + * bus: the CSI interface clock > > + * mod: the CSI module clock > > + * ram: the CSI DRAM clock > > + - clock-names: the clock names mentioned above > > + - resets: phandles to the reset line driving the CSI > > + > > +Each CSI node should contain one 'port' child node with one child > > 'endpoint' +node, according to the bindings defined in > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > mentioned > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > or +Bt656 for CSI1. > > Which port represents CSI0 and which one is CSI1? That needs to be > documented. There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) only. > > + > > +Endpoint node properties for CSI1 > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > the hardware supports lane mapping. I enquired about that too. As far as I understand, CSI0 isn't supported yet in the driver due to lack of documentation and lack of open-source vendor- provided source code. While DT bindings are not tied to driver implementations, it's not the best idea to design DT bindings without at least one working implementation to test them. I thus proposed just listing CSI0 as being unsupported for now. > > +--------------------------------- > > + > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > + node > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > +- pclk-sample : (optional) (default: sample on falling edge) > > Could you add that video-interfaces.txt contains documentation of these > properties as well? There's a reference above but only discusses port and > endpoint nodes. > > > +- hsync-active : (only required for parallel) > > +- vsync-active : (only required for parallel) > > As you support both Bt656 and parallel (with sync signals), you can detect > the interface type from the presence of these properties. I think you > should also say that these properties are not allowed on Bt656. So I'd > change this to e.g. > > (required; parallel-only) > > > + > > +Example: > > + > > +csi1: csi@1cb4000 { > > + compatible = "allwinner,sun8i-v3s-csi"; > > + reg = <0x01cb4000 0x1000>; > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&ccu CLK_BUS_CSI>, > > + <&ccu CLK_CSI1_SCLK>, > > + <&ccu CLK_DRAM_CSI>; > > + clock-names = "bus", "mod", "ram"; > > + resets = <&ccu RST_BUS_CSI>; > > + > > + port { > > + /* Parallel bus endpoint */ > > + csi1_ep: endpoint { > > + remote-endpoint = <&adv7611_ep>; > > + bus-width = <16>; > > + > > + /* If hsync-active/vsync-active are missing, > > + embedded BT.656 sync is used */ > > + hsync-active = <0>; /* Active low */ > > + vsync-active = <0>; /* Active low */ > > + pclk-sample = <1>; /* Rising */ > > + }; > > + }; > > +}; -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-28 9:45 ` Laurent Pinchart (?) @ 2018-09-28 10:23 ` Sakari Ailus -1 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 10:23 UTC (permalink / raw) To: Laurent Pinchart Cc: Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Laurent, On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > Add binding documentation for Allwinner V3s CSI. > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > I know... but I have a few more comments. > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > --- > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > 1 file changed, 59 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > 100644 > > > index 000000000000..2ff47a9507a6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > @@ -0,0 +1,59 @@ > > > +Allwinner V3s Camera Sensor Interface > > > +------------------------------------- > > > + > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > +interface and CSI1 is used for parallel interface. > > > + > > > +Required properties: > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > + - reg: base address and size of the memory-mapped region. > > > + - interrupts: interrupt associated to this IP > > > + - clocks: phandles to the clocks feeding the CSI > > > + * bus: the CSI interface clock > > > + * mod: the CSI module clock > > > + * ram: the CSI DRAM clock > > > + - clock-names: the clock names mentioned above > > > + - resets: phandles to the reset line driving the CSI > > > + > > > +Each CSI node should contain one 'port' child node with one child > > > 'endpoint' +node, according to the bindings defined in > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > mentioned > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > or +Bt656 for CSI1. > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > documented. > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > only. > > > > + > > > +Endpoint node properties for CSI1 > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > the hardware supports lane mapping. > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > the driver due to lack of documentation and lack of open-source vendor- > provided source code. While DT bindings are not tied to driver > implementations, it's not the best idea to design DT bindings without at least > one working implementation to test them. I thus proposed just listing CSI0 as > being unsupported for now. Ack. We should still define which receiver corresponds to a given port. Probably 1 for CSI1 would make sense, in order to avoid changing the order the hardware already uses. 0 doesn't need to be documented no IMO. What do you think? > > > > +--------------------------------- > > > + > > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > > + node > > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > > +- pclk-sample : (optional) (default: sample on falling edge) > > > > Could you add that video-interfaces.txt contains documentation of these > > properties as well? There's a reference above but only discusses port and > > endpoint nodes. > > > > > +- hsync-active : (only required for parallel) > > > +- vsync-active : (only required for parallel) > > > > As you support both Bt656 and parallel (with sync signals), you can detect > > the interface type from the presence of these properties. I think you > > should also say that these properties are not allowed on Bt656. So I'd > > change this to e.g. > > > > (required; parallel-only) > > > > > + > > > +Example: > > > + > > > +csi1: csi@1cb4000 { > > > + compatible = "allwinner,sun8i-v3s-csi"; > > > + reg = <0x01cb4000 0x1000>; > > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&ccu CLK_BUS_CSI>, > > > + <&ccu CLK_CSI1_SCLK>, > > > + <&ccu CLK_DRAM_CSI>; > > > + clock-names = "bus", "mod", "ram"; > > > + resets = <&ccu RST_BUS_CSI>; > > > + > > > + port { > > > + /* Parallel bus endpoint */ > > > + csi1_ep: endpoint { > > > + remote-endpoint = <&adv7611_ep>; > > > + bus-width = <16>; > > > + > > > + /* If hsync-active/vsync-active are missing, > > > + embedded BT.656 sync is used */ > > > + hsync-active = <0>; /* Active low */ > > > + vsync-active = <0>; /* Active low */ > > > + pclk-sample = <1>; /* Rising */ > > > + }; > > > + }; > > > +}; -- Terveisin, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 10:23 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 10:23 UTC (permalink / raw) To: linux-arm-kernel Hi Laurent, On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > Add binding documentation for Allwinner V3s CSI. > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > I know... but I have a few more comments. > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > --- > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > 1 file changed, 59 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > 100644 > > > index 000000000000..2ff47a9507a6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > @@ -0,0 +1,59 @@ > > > +Allwinner V3s Camera Sensor Interface > > > +------------------------------------- > > > + > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > +interface and CSI1 is used for parallel interface. > > > + > > > +Required properties: > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > + - reg: base address and size of the memory-mapped region. > > > + - interrupts: interrupt associated to this IP > > > + - clocks: phandles to the clocks feeding the CSI > > > + * bus: the CSI interface clock > > > + * mod: the CSI module clock > > > + * ram: the CSI DRAM clock > > > + - clock-names: the clock names mentioned above > > > + - resets: phandles to the reset line driving the CSI > > > + > > > +Each CSI node should contain one 'port' child node with one child > > > 'endpoint' +node, according to the bindings defined in > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > mentioned > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > or +Bt656 for CSI1. > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > documented. > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > only. > > > > + > > > +Endpoint node properties for CSI1 > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > the hardware supports lane mapping. > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > the driver due to lack of documentation and lack of open-source vendor- > provided source code. While DT bindings are not tied to driver > implementations, it's not the best idea to design DT bindings without at least > one working implementation to test them. I thus proposed just listing CSI0 as > being unsupported for now. Ack. We should still define which receiver corresponds to a given port. Probably 1 for CSI1 would make sense, in order to avoid changing the order the hardware already uses. 0 doesn't need to be documented no IMO. What do you think? > > > > +--------------------------------- > > > + > > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > > + node > > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > > +- pclk-sample : (optional) (default: sample on falling edge) > > > > Could you add that video-interfaces.txt contains documentation of these > > properties as well? There's a reference above but only discusses port and > > endpoint nodes. > > > > > +- hsync-active : (only required for parallel) > > > +- vsync-active : (only required for parallel) > > > > As you support both Bt656 and parallel (with sync signals), you can detect > > the interface type from the presence of these properties. I think you > > should also say that these properties are not allowed on Bt656. So I'd > > change this to e.g. > > > > (required; parallel-only) > > > > > + > > > +Example: > > > + > > > +csi1: csi at 1cb4000 { > > > + compatible = "allwinner,sun8i-v3s-csi"; > > > + reg = <0x01cb4000 0x1000>; > > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&ccu CLK_BUS_CSI>, > > > + <&ccu CLK_CSI1_SCLK>, > > > + <&ccu CLK_DRAM_CSI>; > > > + clock-names = "bus", "mod", "ram"; > > > + resets = <&ccu RST_BUS_CSI>; > > > + > > > + port { > > > + /* Parallel bus endpoint */ > > > + csi1_ep: endpoint { > > > + remote-endpoint = <&adv7611_ep>; > > > + bus-width = <16>; > > > + > > > + /* If hsync-active/vsync-active are missing, > > > + embedded BT.656 sync is used */ > > > + hsync-active = <0>; /* Active low */ > > > + vsync-active = <0>; /* Active low */ > > > + pclk-sample = <1>; /* Rising */ > > > + }; > > > + }; > > > +}; -- Terveisin, Sakari Ailus sakari.ailus at linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 10:23 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 10:23 UTC (permalink / raw) To: Laurent Pinchart Cc: Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, "David S. Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, linux-media, devicetree, linux-arm-kernel Hi Laurent, On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > Add binding documentation for Allwinner V3s CSI. > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > I know... but I have a few more comments. > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > --- > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > 1 file changed, 59 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > 100644 > > > index 000000000000..2ff47a9507a6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > @@ -0,0 +1,59 @@ > > > +Allwinner V3s Camera Sensor Interface > > > +------------------------------------- > > > + > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > +interface and CSI1 is used for parallel interface. > > > + > > > +Required properties: > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > + - reg: base address and size of the memory-mapped region. > > > + - interrupts: interrupt associated to this IP > > > + - clocks: phandles to the clocks feeding the CSI > > > + * bus: the CSI interface clock > > > + * mod: the CSI module clock > > > + * ram: the CSI DRAM clock > > > + - clock-names: the clock names mentioned above > > > + - resets: phandles to the reset line driving the CSI > > > + > > > +Each CSI node should contain one 'port' child node with one child > > > 'endpoint' +node, according to the bindings defined in > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > mentioned > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > or +Bt656 for CSI1. > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > documented. > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > only. > > > > + > > > +Endpoint node properties for CSI1 > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > the hardware supports lane mapping. > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > the driver due to lack of documentation and lack of open-source vendor- > provided source code. While DT bindings are not tied to driver > implementations, it's not the best idea to design DT bindings without at least > one working implementation to test them. I thus proposed just listing CSI0 as > being unsupported for now. Ack. We should still define which receiver corresponds to a given port. Probably 1 for CSI1 would make sense, in order to avoid changing the order the hardware already uses. 0 doesn't need to be documented no IMO. What do you think? > > > > +--------------------------------- > > > + > > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > > + node > > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > > +- pclk-sample : (optional) (default: sample on falling edge) > > > > Could you add that video-interfaces.txt contains documentation of these > > properties as well? There's a reference above but only discusses port and > > endpoint nodes. > > > > > +- hsync-active : (only required for parallel) > > > +- vsync-active : (only required for parallel) > > > > As you support both Bt656 and parallel (with sync signals), you can detect > > the interface type from the presence of these properties. I think you > > should also say that these properties are not allowed on Bt656. So I'd > > change this to e.g. > > > > (required; parallel-only) > > > > > + > > > +Example: > > > + > > > +csi1: csi@1cb4000 { > > > + compatible = "allwinner,sun8i-v3s-csi"; > > > + reg = <0x01cb4000 0x1000>; > > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&ccu CLK_BUS_CSI>, > > > + <&ccu CLK_CSI1_SCLK>, > > > + <&ccu CLK_DRAM_CSI>; > > > + clock-names = "bus", "mod", "ram"; > > > + resets = <&ccu RST_BUS_CSI>; > > > + > > > + port { > > > + /* Parallel bus endpoint */ > > > + csi1_ep: endpoint { > > > + remote-endpoint = <&adv7611_ep>; > > > + bus-width = <16>; > > > + > > > + /* If hsync-active/vsync-active are missing, > > > + embedded BT.656 sync is used */ > > > + hsync-active = <0>; /* Active low */ > > > + vsync-active = <0>; /* Active low */ > > > + pclk-sample = <1>; /* Rising */ > > > + }; > > > + }; > > > +}; -- Terveisin, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 11:10 ` Chen-Yu Tsai 0 siblings, 0 replies; 48+ messages in thread From: Chen-Yu Tsai @ 2018-09-28 11:10 UTC (permalink / raw) To: Sakari Ailus Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Laurent, > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > Hi Sakari, > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > I know... but I have a few more comments. > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > --- > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > 1 file changed, 59 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > 100644 > > > > index 000000000000..2ff47a9507a6 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > @@ -0,0 +1,59 @@ > > > > +Allwinner V3s Camera Sensor Interface > > > > +------------------------------------- > > > > + > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > +interface and CSI1 is used for parallel interface. > > > > + > > > > +Required properties: > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > + - reg: base address and size of the memory-mapped region. > > > > + - interrupts: interrupt associated to this IP > > > > + - clocks: phandles to the clocks feeding the CSI > > > > + * bus: the CSI interface clock > > > > + * mod: the CSI module clock > > > > + * ram: the CSI DRAM clock > > > > + - clock-names: the clock names mentioned above > > > > + - resets: phandles to the reset line driving the CSI > > > > + > > > > +Each CSI node should contain one 'port' child node with one child > > > > 'endpoint' +node, according to the bindings defined in > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > mentioned > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > or +Bt656 for CSI1. > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > documented. > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > only. > > > > > > + > > > > +Endpoint node properties for CSI1 > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > the hardware supports lane mapping. > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > the driver due to lack of documentation and lack of open-source vendor- > > provided source code. While DT bindings are not tied to driver > > implementations, it's not the best idea to design DT bindings without at least > > one working implementation to test them. I thus proposed just listing CSI0 as > > being unsupported for now. > > Ack. > > We should still define which receiver corresponds to a given port. Probably > 1 for CSI1 would make sense, in order to avoid changing the order the > hardware already uses. 0 doesn't need to be documented no IMO. > > What do you think? AFAICT it would be a completely seperate node, since they have different address spaces, clocks and reset controls. So there's no possibility of confusion. According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, which is the undocumented part. CSI1 has its parallel data pins exposed to the outside. ChenYu > > > > > > +--------------------------------- > > > > + > > > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > > > + node > > > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > > > +- pclk-sample : (optional) (default: sample on falling edge) > > > > > > Could you add that video-interfaces.txt contains documentation of these > > > properties as well? There's a reference above but only discusses port and > > > endpoint nodes. > > > > > > > +- hsync-active : (only required for parallel) > > > > +- vsync-active : (only required for parallel) > > > > > > As you support both Bt656 and parallel (with sync signals), you can detect > > > the interface type from the presence of these properties. I think you > > > should also say that these properties are not allowed on Bt656. So I'd > > > change this to e.g. > > > > > > (required; parallel-only) > > > > > > > + > > > > +Example: > > > > + > > > > +csi1: csi@1cb4000 { > > > > + compatible = "allwinner,sun8i-v3s-csi"; > > > > + reg = <0x01cb4000 0x1000>; > > > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&ccu CLK_BUS_CSI>, > > > > + <&ccu CLK_CSI1_SCLK>, > > > > + <&ccu CLK_DRAM_CSI>; > > > > + clock-names = "bus", "mod", "ram"; > > > > + resets = <&ccu RST_BUS_CSI>; > > > > + > > > > + port { > > > > + /* Parallel bus endpoint */ > > > > + csi1_ep: endpoint { > > > > + remote-endpoint = <&adv7611_ep>; > > > > + bus-width = <16>; > > > > + > > > > + /* If hsync-active/vsync-active are missing, > > > > + embedded BT.656 sync is used */ > > > > + hsync-active = <0>; /* Active low */ > > > > + vsync-active = <0>; /* Active low */ > > > > + pclk-sample = <1>; /* Rising */ > > > > + }; > > > > + }; > > > > +}; > > -- > Terveisin, > > Sakari Ailus > sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 11:10 ` Chen-Yu Tsai 0 siblings, 0 replies; 48+ messages in thread From: Chen-Yu Tsai @ 2018-09-28 11:10 UTC (permalink / raw) To: linux-arm-kernel On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Laurent, > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > Hi Sakari, > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > I know... but I have a few more comments. > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > --- > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > 1 file changed, 59 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > 100644 > > > > index 000000000000..2ff47a9507a6 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > @@ -0,0 +1,59 @@ > > > > +Allwinner V3s Camera Sensor Interface > > > > +------------------------------------- > > > > + > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > +interface and CSI1 is used for parallel interface. > > > > + > > > > +Required properties: > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > + - reg: base address and size of the memory-mapped region. > > > > + - interrupts: interrupt associated to this IP > > > > + - clocks: phandles to the clocks feeding the CSI > > > > + * bus: the CSI interface clock > > > > + * mod: the CSI module clock > > > > + * ram: the CSI DRAM clock > > > > + - clock-names: the clock names mentioned above > > > > + - resets: phandles to the reset line driving the CSI > > > > + > > > > +Each CSI node should contain one 'port' child node with one child > > > > 'endpoint' +node, according to the bindings defined in > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > mentioned > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > or +Bt656 for CSI1. > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > documented. > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > only. > > > > > > + > > > > +Endpoint node properties for CSI1 > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > the hardware supports lane mapping. > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > the driver due to lack of documentation and lack of open-source vendor- > > provided source code. While DT bindings are not tied to driver > > implementations, it's not the best idea to design DT bindings without at least > > one working implementation to test them. I thus proposed just listing CSI0 as > > being unsupported for now. > > Ack. > > We should still define which receiver corresponds to a given port. Probably > 1 for CSI1 would make sense, in order to avoid changing the order the > hardware already uses. 0 doesn't need to be documented no IMO. > > What do you think? AFAICT it would be a completely seperate node, since they have different address spaces, clocks and reset controls. So there's no possibility of confusion. According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, which is the undocumented part. CSI1 has its parallel data pins exposed to the outside. ChenYu > > > > > > +--------------------------------- > > > > + > > > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > > > + node > > > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > > > +- pclk-sample : (optional) (default: sample on falling edge) > > > > > > Could you add that video-interfaces.txt contains documentation of these > > > properties as well? There's a reference above but only discusses port and > > > endpoint nodes. > > > > > > > +- hsync-active : (only required for parallel) > > > > +- vsync-active : (only required for parallel) > > > > > > As you support both Bt656 and parallel (with sync signals), you can detect > > > the interface type from the presence of these properties. I think you > > > should also say that these properties are not allowed on Bt656. So I'd > > > change this to e.g. > > > > > > (required; parallel-only) > > > > > > > + > > > > +Example: > > > > + > > > > +csi1: csi at 1cb4000 { > > > > + compatible = "allwinner,sun8i-v3s-csi"; > > > > + reg = <0x01cb4000 0x1000>; > > > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&ccu CLK_BUS_CSI>, > > > > + <&ccu CLK_CSI1_SCLK>, > > > > + <&ccu CLK_DRAM_CSI>; > > > > + clock-names = "bus", "mod", "ram"; > > > > + resets = <&ccu RST_BUS_CSI>; > > > > + > > > > + port { > > > > + /* Parallel bus endpoint */ > > > > + csi1_ep: endpoint { > > > > + remote-endpoint = <&adv7611_ep>; > > > > + bus-width = <16>; > > > > + > > > > + /* If hsync-active/vsync-active are missing, > > > > + embedded BT.656 sync is used */ > > > > + hsync-active = <0>; /* Active low */ > > > > + vsync-active = <0>; /* Active low */ > > > > + pclk-sample = <1>; /* Rising */ > > > > + }; > > > > + }; > > > > +}; > > -- > Terveisin, > > Sakari Ailus > sakari.ailus at linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 11:10 ` Chen-Yu Tsai 0 siblings, 0 replies; 48+ messages in thread From: Chen-Yu Tsai @ 2018-09-28 11:10 UTC (permalink / raw) To: Sakari Ailus Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > > Hi Laurent, > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > Hi Sakari, > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> > > > > Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > I know... but I have a few more comments. > > > > > > > Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > > > > Signed-off-by: Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org> > > > > --- > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > 1 file changed, 59 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > 100644 > > > > index 000000000000..2ff47a9507a6 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > @@ -0,0 +1,59 @@ > > > > +Allwinner V3s Camera Sensor Interface > > > > +------------------------------------- > > > > + > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > +interface and CSI1 is used for parallel interface. > > > > + > > > > +Required properties: > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > + - reg: base address and size of the memory-mapped region. > > > > + - interrupts: interrupt associated to this IP > > > > + - clocks: phandles to the clocks feeding the CSI > > > > + * bus: the CSI interface clock > > > > + * mod: the CSI module clock > > > > + * ram: the CSI DRAM clock > > > > + - clock-names: the clock names mentioned above > > > > + - resets: phandles to the reset line driving the CSI > > > > + > > > > +Each CSI node should contain one 'port' child node with one child > > > > 'endpoint' +node, according to the bindings defined in > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > mentioned > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > or +Bt656 for CSI1. > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > documented. > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > only. > > > > > > + > > > > +Endpoint node properties for CSI1 > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > the hardware supports lane mapping. > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > the driver due to lack of documentation and lack of open-source vendor- > > provided source code. While DT bindings are not tied to driver > > implementations, it's not the best idea to design DT bindings without at least > > one working implementation to test them. I thus proposed just listing CSI0 as > > being unsupported for now. > > Ack. > > We should still define which receiver corresponds to a given port. Probably > 1 for CSI1 would make sense, in order to avoid changing the order the > hardware already uses. 0 doesn't need to be documented no IMO. > > What do you think? AFAICT it would be a completely seperate node, since they have different address spaces, clocks and reset controls. So there's no possibility of confusion. According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, which is the undocumented part. CSI1 has its parallel data pins exposed to the outside. ChenYu > > > > > > +--------------------------------- > > > > + > > > > +- remote-endpoint : (required) a phandle to the bus receiver's endpoint > > > > + node > > > > +- bus-width: : (required) must be 8, 10, 12 or 16 > > > > +- pclk-sample : (optional) (default: sample on falling edge) > > > > > > Could you add that video-interfaces.txt contains documentation of these > > > properties as well? There's a reference above but only discusses port and > > > endpoint nodes. > > > > > > > +- hsync-active : (only required for parallel) > > > > +- vsync-active : (only required for parallel) > > > > > > As you support both Bt656 and parallel (with sync signals), you can detect > > > the interface type from the presence of these properties. I think you > > > should also say that these properties are not allowed on Bt656. So I'd > > > change this to e.g. > > > > > > (required; parallel-only) > > > > > > > + > > > > +Example: > > > > + > > > > +csi1: csi@1cb4000 { > > > > + compatible = "allwinner,sun8i-v3s-csi"; > > > > + reg = <0x01cb4000 0x1000>; > > > > + interrupts = <GIC_SPI 84 IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&ccu CLK_BUS_CSI>, > > > > + <&ccu CLK_CSI1_SCLK>, > > > > + <&ccu CLK_DRAM_CSI>; > > > > + clock-names = "bus", "mod", "ram"; > > > > + resets = <&ccu RST_BUS_CSI>; > > > > + > > > > + port { > > > > + /* Parallel bus endpoint */ > > > > + csi1_ep: endpoint { > > > > + remote-endpoint = <&adv7611_ep>; > > > > + bus-width = <16>; > > > > + > > > > + /* If hsync-active/vsync-active are missing, > > > > + embedded BT.656 sync is used */ > > > > + hsync-active = <0>; /* Active low */ > > > > + vsync-active = <0>; /* Active low */ > > > > + pclk-sample = <1>; /* Rising */ > > > > + }; > > > > + }; > > > > +}; > > -- > Terveisin, > > Sakari Ailus > sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-28 11:10 ` Chen-Yu Tsai (?) @ 2018-09-28 12:56 ` Sakari Ailus -1 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 12:56 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Chen-Yu, On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: > > > > Hi Laurent, > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > Hi Sakari, > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > I know... but I have a few more comments. > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > --- > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > 1 file changed, 59 insertions(+) > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > 100644 > > > > > index 000000000000..2ff47a9507a6 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > @@ -0,0 +1,59 @@ > > > > > +Allwinner V3s Camera Sensor Interface > > > > > +------------------------------------- > > > > > + > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > +interface and CSI1 is used for parallel interface. > > > > > + > > > > > +Required properties: > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > + - reg: base address and size of the memory-mapped region. > > > > > + - interrupts: interrupt associated to this IP > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > + * bus: the CSI interface clock > > > > > + * mod: the CSI module clock > > > > > + * ram: the CSI DRAM clock > > > > > + - clock-names: the clock names mentioned above > > > > > + - resets: phandles to the reset line driving the CSI > > > > > + > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > 'endpoint' +node, according to the bindings defined in > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > mentioned > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > or +Bt656 for CSI1. > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > documented. > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > only. > > > > > > > > + > > > > > +Endpoint node properties for CSI1 > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > the hardware supports lane mapping. > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > the driver due to lack of documentation and lack of open-source vendor- > > > provided source code. While DT bindings are not tied to driver > > > implementations, it's not the best idea to design DT bindings without at least > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > being unsupported for now. > > > > Ack. > > > > We should still define which receiver corresponds to a given port. Probably > > 1 for CSI1 would make sense, in order to avoid changing the order the > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > What do you think? > > AFAICT it would be a completely seperate node, since they have different address > spaces, clocks and reset controls. So there's no possibility of confusion. > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > which is the undocumented part. CSI1 has its parallel data pins exposed to the > outside. Thanks for clearing up the confusion. If these are truly different kinds of devices, then don't they also deserve different compatible strings? And possibly also different DT binding documentation in a separate file. -- Regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 12:56 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 12:56 UTC (permalink / raw) To: linux-arm-kernel Hi Chen-Yu, On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: > > > > Hi Laurent, > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > Hi Sakari, > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > I know... but I have a few more comments. > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > --- > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > 1 file changed, 59 insertions(+) > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > 100644 > > > > > index 000000000000..2ff47a9507a6 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > @@ -0,0 +1,59 @@ > > > > > +Allwinner V3s Camera Sensor Interface > > > > > +------------------------------------- > > > > > + > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > +interface and CSI1 is used for parallel interface. > > > > > + > > > > > +Required properties: > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > + - reg: base address and size of the memory-mapped region. > > > > > + - interrupts: interrupt associated to this IP > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > + * bus: the CSI interface clock > > > > > + * mod: the CSI module clock > > > > > + * ram: the CSI DRAM clock > > > > > + - clock-names: the clock names mentioned above > > > > > + - resets: phandles to the reset line driving the CSI > > > > > + > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > 'endpoint' +node, according to the bindings defined in > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > mentioned > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > or +Bt656 for CSI1. > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > documented. > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > only. > > > > > > > > + > > > > > +Endpoint node properties for CSI1 > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > the hardware supports lane mapping. > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > the driver due to lack of documentation and lack of open-source vendor- > > > provided source code. While DT bindings are not tied to driver > > > implementations, it's not the best idea to design DT bindings without at least > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > being unsupported for now. > > > > Ack. > > > > We should still define which receiver corresponds to a given port. Probably > > 1 for CSI1 would make sense, in order to avoid changing the order the > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > What do you think? > > AFAICT it would be a completely seperate node, since they have different address > spaces, clocks and reset controls. So there's no possibility of confusion. > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > which is the undocumented part. CSI1 has its parallel data pins exposed to the > outside. Thanks for clearing up the confusion. If these are truly different kinds of devices, then don't they also deserve different compatible strings? And possibly also different DT binding documentation in a separate file. -- Regards, Sakari Ailus sakari.ailus at linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 12:56 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 12:56 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List Hi Chen-Yu, On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: > > > > Hi Laurent, > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > Hi Sakari, > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > I know... but I have a few more comments. > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > --- > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > 1 file changed, 59 insertions(+) > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > 100644 > > > > > index 000000000000..2ff47a9507a6 > > > > > --- /dev/null > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > @@ -0,0 +1,59 @@ > > > > > +Allwinner V3s Camera Sensor Interface > > > > > +------------------------------------- > > > > > + > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > +interface and CSI1 is used for parallel interface. > > > > > + > > > > > +Required properties: > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > + - reg: base address and size of the memory-mapped region. > > > > > + - interrupts: interrupt associated to this IP > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > + * bus: the CSI interface clock > > > > > + * mod: the CSI module clock > > > > > + * ram: the CSI DRAM clock > > > > > + - clock-names: the clock names mentioned above > > > > > + - resets: phandles to the reset line driving the CSI > > > > > + > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > 'endpoint' +node, according to the bindings defined in > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > mentioned > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > or +Bt656 for CSI1. > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > documented. > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > only. > > > > > > > > + > > > > > +Endpoint node properties for CSI1 > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > the hardware supports lane mapping. > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > the driver due to lack of documentation and lack of open-source vendor- > > > provided source code. While DT bindings are not tied to driver > > > implementations, it's not the best idea to design DT bindings without at least > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > being unsupported for now. > > > > Ack. > > > > We should still define which receiver corresponds to a given port. Probably > > 1 for CSI1 would make sense, in order to avoid changing the order the > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > What do you think? > > AFAICT it would be a completely seperate node, since they have different address > spaces, clocks and reset controls. So there's no possibility of confusion. > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > which is the undocumented part. CSI1 has its parallel data pins exposed to the > outside. Thanks for clearing up the confusion. If these are truly different kinds of devices, then don't they also deserve different compatible strings? And possibly also different DT binding documentation in a separate file. -- Regards, Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 13:36 ` Maxime Ripard 0 siblings, 0 replies; 48+ messages in thread From: Maxime Ripard @ 2018-09-28 13:36 UTC (permalink / raw) To: Sakari Ailus Cc: Chen-Yu Tsai, Laurent Pinchart, Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Sakari, Thanks for taking the time to review. On Fri, Sep 28, 2018 at 03:56:01PM +0300, Sakari Ailus wrote: > Hi Chen-Yu, > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > <sakari.ailus@linux.intel.com> wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > Hi Sakari, > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > --- > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > 1 file changed, 59 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > 100644 > > > > > > index 000000000000..2ff47a9507a6 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > @@ -0,0 +1,59 @@ > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > +------------------------------------- > > > > > > + > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > + > > > > > > +Required properties: > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > + - interrupts: interrupt associated to this IP > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > + * bus: the CSI interface clock > > > > > > + * mod: the CSI module clock > > > > > > + * ram: the CSI DRAM clock > > > > > > + - clock-names: the clock names mentioned above > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > + > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > mentioned > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > or +Bt656 for CSI1. > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > documented. > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > only. > > > > > > > > > > + > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > the hardware supports lane mapping. > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > provided source code. While DT bindings are not tied to driver > > > > implementations, it's not the best idea to design DT bindings without at least > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > being unsupported for now. > > > > > > Ack. > > > > > > We should still define which receiver corresponds to a given port. Probably > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > What do you think? > > > > AFAICT it would be a completely seperate node, since they have different address > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > outside. > > Thanks for clearing up the confusion. If these are truly different kinds of > devices, then don't they also deserve different compatible strings? And > possibly also different DT binding documentation in a separate file. It can, and will if it's ever supported, but I'm not quite sure what's confusing you about those bindings. It never claims to support CSI0, and we will only add a new document and compatible and whatever is needed when we'll have the need for it? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 13:36 ` Maxime Ripard 0 siblings, 0 replies; 48+ messages in thread From: Maxime Ripard @ 2018-09-28 13:36 UTC (permalink / raw) To: linux-arm-kernel Hi Sakari, Thanks for taking the time to review. On Fri, Sep 28, 2018 at 03:56:01PM +0300, Sakari Ailus wrote: > Hi Chen-Yu, > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > <sakari.ailus@linux.intel.com> wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > Hi Sakari, > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > --- > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > 1 file changed, 59 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > 100644 > > > > > > index 000000000000..2ff47a9507a6 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > @@ -0,0 +1,59 @@ > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > +------------------------------------- > > > > > > + > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > + > > > > > > +Required properties: > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > + - interrupts: interrupt associated to this IP > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > + * bus: the CSI interface clock > > > > > > + * mod: the CSI module clock > > > > > > + * ram: the CSI DRAM clock > > > > > > + - clock-names: the clock names mentioned above > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > + > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > mentioned > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > or +Bt656 for CSI1. > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > documented. > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > only. > > > > > > > > > > + > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > the hardware supports lane mapping. > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > provided source code. While DT bindings are not tied to driver > > > > implementations, it's not the best idea to design DT bindings without at least > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > being unsupported for now. > > > > > > Ack. > > > > > > We should still define which receiver corresponds to a given port. Probably > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > What do you think? > > > > AFAICT it would be a completely seperate node, since they have different address > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > outside. > > Thanks for clearing up the confusion. If these are truly different kinds of > devices, then don't they also deserve different compatible strings? And > possibly also different DT binding documentation in a separate file. It can, and will if it's ever supported, but I'm not quite sure what's confusing you about those bindings. It never claims to support CSI0, and we will only add a new document and compatible and whatever is needed when we'll have the need for it? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 13:36 ` Maxime Ripard 0 siblings, 0 replies; 48+ messages in thread From: Maxime Ripard @ 2018-09-28 13:36 UTC (permalink / raw) To: Sakari Ailus Cc: Chen-Yu Tsai, Laurent Pinchart, Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel, linux-s Hi Sakari, Thanks for taking the time to review. On Fri, Sep 28, 2018 at 03:56:01PM +0300, Sakari Ailus wrote: > Hi Chen-Yu, > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > Hi Sakari, > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> > > > > > > Acked-by: Sakari Ailus <sakari.ailus-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > > > > > > Signed-off-by: Yong Deng <yong.deng-+3dxTMOEIRNWk0Htik3J/w@public.gmane.org> > > > > > > --- > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > 1 file changed, 59 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > 100644 > > > > > > index 000000000000..2ff47a9507a6 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > @@ -0,0 +1,59 @@ > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > +------------------------------------- > > > > > > + > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > + > > > > > > +Required properties: > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > + - interrupts: interrupt associated to this IP > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > + * bus: the CSI interface clock > > > > > > + * mod: the CSI module clock > > > > > > + * ram: the CSI DRAM clock > > > > > > + - clock-names: the clock names mentioned above > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > + > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > mentioned > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > or +Bt656 for CSI1. > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > documented. > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > only. > > > > > > > > > > + > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > the hardware supports lane mapping. > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > provided source code. While DT bindings are not tied to driver > > > > implementations, it's not the best idea to design DT bindings without at least > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > being unsupported for now. > > > > > > Ack. > > > > > > We should still define which receiver corresponds to a given port. Probably > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > What do you think? > > > > AFAICT it would be a completely seperate node, since they have different address > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > outside. > > Thanks for clearing up the confusion. If these are truly different kinds of > devices, then don't they also deserve different compatible strings? And > possibly also different DT binding documentation in a separate file. It can, and will if it's ever supported, but I'm not quite sure what's confusing you about those bindings. It never claims to support CSI0, and we will only add a new document and compatible and whatever is needed when we'll have the need for it? Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-28 13:36 ` Maxime Ripard (?) @ 2018-09-28 13:42 ` Sakari Ailus -1 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 13:42 UTC (permalink / raw) To: Maxime Ripard Cc: Chen-Yu Tsai, Laurent Pinchart, Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi Maxime, On Fri, Sep 28, 2018 at 03:36:42PM +0200, Maxime Ripard wrote: > Hi Sakari, > > Thanks for taking the time to review. > > On Fri, Sep 28, 2018 at 03:56:01PM +0300, Sakari Ailus wrote: > > Hi Chen-Yu, > > > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > > <sakari.ailus@linux.intel.com> wrote: > > > > > > > > Hi Laurent, > > > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > > Hi Sakari, > > > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > > --- > > > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > > 1 file changed, 59 insertions(+) > > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > > 100644 > > > > > > > index 000000000000..2ff47a9507a6 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > @@ -0,0 +1,59 @@ > > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > > +------------------------------------- > > > > > > > + > > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > > + > > > > > > > +Required properties: > > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > > + - interrupts: interrupt associated to this IP > > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > > + * bus: the CSI interface clock > > > > > > > + * mod: the CSI module clock > > > > > > > + * ram: the CSI DRAM clock > > > > > > > + - clock-names: the clock names mentioned above > > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > > + > > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > > mentioned > > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > > or +Bt656 for CSI1. > > > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > > documented. > > > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > > only. > > > > > > > > > > > > + > > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > > the hardware supports lane mapping. > > > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > > provided source code. While DT bindings are not tied to driver > > > > > implementations, it's not the best idea to design DT bindings without at least > > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > > being unsupported for now. > > > > > > > > Ack. > > > > > > > > We should still define which receiver corresponds to a given port. Probably > > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > > > What do you think? > > > > > > AFAICT it would be a completely seperate node, since they have different address > > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > > outside. > > > > Thanks for clearing up the confusion. If these are truly different kinds of > > devices, then don't they also deserve different compatible strings? And > > possibly also different DT binding documentation in a separate file. > > It can, and will if it's ever supported, but I'm not quite sure what's > confusing you about those bindings. It never claims to support CSI0, > and we will only add a new document and compatible and whatever is > needed when we'll have the need for it? Apart from the endpoint node documentation, the rest appears to apply to both CSI0 and CSI1. If they're truly different kind of devices, then they do need different compatible strings, don't they? Currently they're both documented to be using the same. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 13:42 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 13:42 UTC (permalink / raw) To: linux-arm-kernel Hi Maxime, On Fri, Sep 28, 2018 at 03:36:42PM +0200, Maxime Ripard wrote: > Hi Sakari, > > Thanks for taking the time to review. > > On Fri, Sep 28, 2018 at 03:56:01PM +0300, Sakari Ailus wrote: > > Hi Chen-Yu, > > > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > > <sakari.ailus@linux.intel.com> wrote: > > > > > > > > Hi Laurent, > > > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > > Hi Sakari, > > > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > > --- > > > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > > 1 file changed, 59 insertions(+) > > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > > 100644 > > > > > > > index 000000000000..2ff47a9507a6 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > @@ -0,0 +1,59 @@ > > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > > +------------------------------------- > > > > > > > + > > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > > + > > > > > > > +Required properties: > > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > > + - interrupts: interrupt associated to this IP > > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > > + * bus: the CSI interface clock > > > > > > > + * mod: the CSI module clock > > > > > > > + * ram: the CSI DRAM clock > > > > > > > + - clock-names: the clock names mentioned above > > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > > + > > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > > mentioned > > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > > or +Bt656 for CSI1. > > > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > > documented. > > > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > > only. > > > > > > > > > > > > + > > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > > the hardware supports lane mapping. > > > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > > provided source code. While DT bindings are not tied to driver > > > > > implementations, it's not the best idea to design DT bindings without at least > > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > > being unsupported for now. > > > > > > > > Ack. > > > > > > > > We should still define which receiver corresponds to a given port. Probably > > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > > > What do you think? > > > > > > AFAICT it would be a completely seperate node, since they have different address > > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > > outside. > > > > Thanks for clearing up the confusion. If these are truly different kinds of > > devices, then don't they also deserve different compatible strings? And > > possibly also different DT binding documentation in a separate file. > > It can, and will if it's ever supported, but I'm not quite sure what's > confusing you about those bindings. It never claims to support CSI0, > and we will only add a new document and compatible and whatever is > needed when we'll have the need for it? Apart from the endpoint node documentation, the rest appears to apply to both CSI0 and CSI1. If they're truly different kind of devices, then they do need different compatible strings, don't they? Currently they're both documented to be using the same. -- Sakari Ailus sakari.ailus at linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 13:42 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 13:42 UTC (permalink / raw) To: Maxime Ripard Cc: Chen-Yu Tsai, Laurent Pinchart, Yong Deng, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree Hi Maxime, On Fri, Sep 28, 2018 at 03:36:42PM +0200, Maxime Ripard wrote: > Hi Sakari, > > Thanks for taking the time to review. > > On Fri, Sep 28, 2018 at 03:56:01PM +0300, Sakari Ailus wrote: > > Hi Chen-Yu, > > > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > > <sakari.ailus@linux.intel.com> wrote: > > > > > > > > Hi Laurent, > > > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > > Hi Sakari, > > > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > > --- > > > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > > 1 file changed, 59 insertions(+) > > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > > 100644 > > > > > > > index 000000000000..2ff47a9507a6 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > @@ -0,0 +1,59 @@ > > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > > +------------------------------------- > > > > > > > + > > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > > + > > > > > > > +Required properties: > > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > > + - interrupts: interrupt associated to this IP > > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > > + * bus: the CSI interface clock > > > > > > > + * mod: the CSI module clock > > > > > > > + * ram: the CSI DRAM clock > > > > > > > + - clock-names: the clock names mentioned above > > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > > + > > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > > mentioned > > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > > or +Bt656 for CSI1. > > > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > > documented. > > > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > > only. > > > > > > > > > > > > + > > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > > the hardware supports lane mapping. > > > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > > provided source code. While DT bindings are not tied to driver > > > > > implementations, it's not the best idea to design DT bindings without at least > > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > > being unsupported for now. > > > > > > > > Ack. > > > > > > > > We should still define which receiver corresponds to a given port. Probably > > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > > > What do you think? > > > > > > AFAICT it would be a completely seperate node, since they have different address > > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > > outside. > > > > Thanks for clearing up the confusion. If these are truly different kinds of > > devices, then don't they also deserve different compatible strings? And > > possibly also different DT binding documentation in a separate file. > > It can, and will if it's ever supported, but I'm not quite sure what's > confusing you about those bindings. It never claims to support CSI0, > and we will only add a new document and compatible and whatever is > needed when we'll have the need for it? Apart from the endpoint node documentation, the rest appears to apply to both CSI0 and CSI1. If they're truly different kind of devices, then they do need different compatible strings, don't they? Currently they're both documented to be using the same. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-28 12:56 ` Sakari Ailus (?) @ 2018-09-28 13:42 ` Chen-Yu Tsai -1 siblings, 0 replies; 48+ messages in thread From: Chen-Yu Tsai @ 2018-09-28 13:42 UTC (permalink / raw) To: Sakari Ailus Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi Hi, On Fri, Sep 28, 2018 at 8:56 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Chen-Yu, > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > <sakari.ailus@linux.intel.com> wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > Hi Sakari, > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > --- > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > 1 file changed, 59 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > 100644 > > > > > > index 000000000000..2ff47a9507a6 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > @@ -0,0 +1,59 @@ > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > +------------------------------------- > > > > > > + > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > + > > > > > > +Required properties: > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > + - interrupts: interrupt associated to this IP > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > + * bus: the CSI interface clock > > > > > > + * mod: the CSI module clock > > > > > > + * ram: the CSI DRAM clock > > > > > > + - clock-names: the clock names mentioned above > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > + > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > mentioned > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > or +Bt656 for CSI1. > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > documented. > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > only. > > > > > > > > > > + > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > the hardware supports lane mapping. > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > provided source code. While DT bindings are not tied to driver > > > > implementations, it's not the best idea to design DT bindings without at least > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > being unsupported for now. > > > > > > Ack. > > > > > > We should still define which receiver corresponds to a given port. Probably > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > What do you think? > > > > AFAICT it would be a completely seperate node, since they have different address > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > outside. > > Thanks for clearing up the confusion. If these are truly different kinds of > devices, then don't they also deserve different compatible strings? And > possibly also different DT binding documentation in a separate file. Allwinner's documents aren't particularly clear about this. AFAICT it does not say anything about them being different. The CSI section gives a register table with two base addresses. The V3S SDK listed on linux-sunxi wiki also doesn't differentiate between the two. What's more is there's actually code and register addresses for the MIPI stuff [1], though the license is questionable, as with most BSPs. It indeed seems like a CSI controller, which basically just takes whatever input it is configured for and sends it to either a downstream ISP or DRAM. There are MIPI-CSI receiver and DPHY blocks in addresses following CSI0, but we can treat them as separate hardware blocks. ChenYu [1] https://github.com/yeashen/v3s-linux-sdk/blob/master/linux-3.4/drivers/media/video/sunxi-vfe/platform/sun8iw8p1_vfe_cfg.h#L27 ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 13:42 ` Chen-Yu Tsai 0 siblings, 0 replies; 48+ messages in thread From: Chen-Yu Tsai @ 2018-09-28 13:42 UTC (permalink / raw) To: linux-arm-kernel Hi, On Fri, Sep 28, 2018 at 8:56 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Chen-Yu, > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > <sakari.ailus@linux.intel.com> wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > Hi Sakari, > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > --- > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > 1 file changed, 59 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > 100644 > > > > > > index 000000000000..2ff47a9507a6 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > @@ -0,0 +1,59 @@ > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > +------------------------------------- > > > > > > + > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > + > > > > > > +Required properties: > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > + - interrupts: interrupt associated to this IP > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > + * bus: the CSI interface clock > > > > > > + * mod: the CSI module clock > > > > > > + * ram: the CSI DRAM clock > > > > > > + - clock-names: the clock names mentioned above > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > + > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > mentioned > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > or +Bt656 for CSI1. > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > documented. > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > only. > > > > > > > > > > + > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > the hardware supports lane mapping. > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > provided source code. While DT bindings are not tied to driver > > > > implementations, it's not the best idea to design DT bindings without at least > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > being unsupported for now. > > > > > > Ack. > > > > > > We should still define which receiver corresponds to a given port. Probably > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > What do you think? > > > > AFAICT it would be a completely seperate node, since they have different address > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > outside. > > Thanks for clearing up the confusion. If these are truly different kinds of > devices, then don't they also deserve different compatible strings? And > possibly also different DT binding documentation in a separate file. Allwinner's documents aren't particularly clear about this. AFAICT it does not say anything about them being different. The CSI section gives a register table with two base addresses. The V3S SDK listed on linux-sunxi wiki also doesn't differentiate between the two. What's more is there's actually code and register addresses for the MIPI stuff [1], though the license is questionable, as with most BSPs. It indeed seems like a CSI controller, which basically just takes whatever input it is configured for and sends it to either a downstream ISP or DRAM. There are MIPI-CSI receiver and DPHY blocks in addresses following CSI0, but we can treat them as separate hardware blocks. ChenYu [1] https://github.com/yeashen/v3s-linux-sdk/blob/master/linux-3.4/drivers/media/video/sunxi-vfe/platform/sun8iw8p1_vfe_cfg.h#L27 ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 13:42 ` Chen-Yu Tsai 0 siblings, 0 replies; 48+ messages in thread From: Chen-Yu Tsai @ 2018-09-28 13:42 UTC (permalink / raw) To: Sakari Ailus Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List Hi, On Fri, Sep 28, 2018 at 8:56 PM Sakari Ailus <sakari.ailus@linux.intel.com> wrote: > > Hi Chen-Yu, > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > <sakari.ailus@linux.intel.com> wrote: > > > > > > Hi Laurent, > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > Hi Sakari, > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > --- > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > 1 file changed, 59 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > 100644 > > > > > > index 000000000000..2ff47a9507a6 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > @@ -0,0 +1,59 @@ > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > +------------------------------------- > > > > > > + > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > + > > > > > > +Required properties: > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > + - interrupts: interrupt associated to this IP > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > + * bus: the CSI interface clock > > > > > > + * mod: the CSI module clock > > > > > > + * ram: the CSI DRAM clock > > > > > > + - clock-names: the clock names mentioned above > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > + > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > mentioned > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > or +Bt656 for CSI1. > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > documented. > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > only. > > > > > > > > > > + > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > the hardware supports lane mapping. > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > provided source code. While DT bindings are not tied to driver > > > > implementations, it's not the best idea to design DT bindings without at least > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > being unsupported for now. > > > > > > Ack. > > > > > > We should still define which receiver corresponds to a given port. Probably > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > What do you think? > > > > AFAICT it would be a completely seperate node, since they have different address > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > outside. > > Thanks for clearing up the confusion. If these are truly different kinds of > devices, then don't they also deserve different compatible strings? And > possibly also different DT binding documentation in a separate file. Allwinner's documents aren't particularly clear about this. AFAICT it does not say anything about them being different. The CSI section gives a register table with two base addresses. The V3S SDK listed on linux-sunxi wiki also doesn't differentiate between the two. What's more is there's actually code and register addresses for the MIPI stuff [1], though the license is questionable, as with most BSPs. It indeed seems like a CSI controller, which basically just takes whatever input it is configured for and sends it to either a downstream ISP or DRAM. There are MIPI-CSI receiver and DPHY blocks in addresses following CSI0, but we can treat them as separate hardware blocks. ChenYu [1] https://github.com/yeashen/v3s-linux-sdk/blob/master/linux-3.4/drivers/media/video/sunxi-vfe/platform/sun8iw8p1_vfe_cfg.h#L27 ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-28 13:42 ` Chen-Yu Tsai (?) @ 2018-09-28 14:47 ` Sakari Ailus -1 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 14:47 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Fri, Sep 28, 2018 at 09:42:50PM +0800, Chen-Yu Tsai wrote: > Hi, > > On Fri, Sep 28, 2018 at 8:56 PM Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: > > > > Hi Chen-Yu, > > > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > > <sakari.ailus@linux.intel.com> wrote: > > > > > > > > Hi Laurent, > > > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > > Hi Sakari, > > > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > > --- > > > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > > 1 file changed, 59 insertions(+) > > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > > 100644 > > > > > > > index 000000000000..2ff47a9507a6 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > @@ -0,0 +1,59 @@ > > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > > +------------------------------------- > > > > > > > + > > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > > + > > > > > > > +Required properties: > > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > > + - interrupts: interrupt associated to this IP > > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > > + * bus: the CSI interface clock > > > > > > > + * mod: the CSI module clock > > > > > > > + * ram: the CSI DRAM clock > > > > > > > + - clock-names: the clock names mentioned above > > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > > + > > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > > mentioned > > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > > or +Bt656 for CSI1. > > > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > > documented. > > > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > > only. > > > > > > > > > > > > + > > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > > the hardware supports lane mapping. > > > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > > provided source code. While DT bindings are not tied to driver > > > > > implementations, it's not the best idea to design DT bindings without at least > > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > > being unsupported for now. > > > > > > > > Ack. > > > > > > > > We should still define which receiver corresponds to a given port. Probably > > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > > > What do you think? > > > > > > AFAICT it would be a completely seperate node, since they have different address > > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > > outside. > > > > Thanks for clearing up the confusion. If these are truly different kinds of > > devices, then don't they also deserve different compatible strings? And > > possibly also different DT binding documentation in a separate file. > > Allwinner's documents aren't particularly clear about this. AFAICT it does not > say anything about them being different. The CSI section gives a register table > with two base addresses. The V3S SDK listed on linux-sunxi wiki also doesn't > differentiate between the two. What's more is there's actually code and register > addresses for the MIPI stuff [1], though the license is questionable, > as with most > BSPs. It indeed seems like a CSI controller, which basically just takes whatever > input it is configured for and sends it to either a downstream ISP or DRAM. > There are MIPI-CSI receiver and DPHY blocks in addresses following CSI0, but we > can treat them as separate hardware blocks. What you could still do is to change the DT binding documentation to only apply to the parallel receiver, not the CSI-2 receiver. That'd give you more freedom going forward in case you'd later implement support for the CSI-2 receiver, too. Up to you. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 14:47 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 14:47 UTC (permalink / raw) To: linux-arm-kernel On Fri, Sep 28, 2018 at 09:42:50PM +0800, Chen-Yu Tsai wrote: > Hi, > > On Fri, Sep 28, 2018 at 8:56 PM Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: > > > > Hi Chen-Yu, > > > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > > <sakari.ailus@linux.intel.com> wrote: > > > > > > > > Hi Laurent, > > > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > > Hi Sakari, > > > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > > --- > > > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > > 1 file changed, 59 insertions(+) > > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > > 100644 > > > > > > > index 000000000000..2ff47a9507a6 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > @@ -0,0 +1,59 @@ > > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > > +------------------------------------- > > > > > > > + > > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > > + > > > > > > > +Required properties: > > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > > + - interrupts: interrupt associated to this IP > > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > > + * bus: the CSI interface clock > > > > > > > + * mod: the CSI module clock > > > > > > > + * ram: the CSI DRAM clock > > > > > > > + - clock-names: the clock names mentioned above > > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > > + > > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > > mentioned > > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > > or +Bt656 for CSI1. > > > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > > documented. > > > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > > only. > > > > > > > > > > > > + > > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > > the hardware supports lane mapping. > > > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > > provided source code. While DT bindings are not tied to driver > > > > > implementations, it's not the best idea to design DT bindings without at least > > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > > being unsupported for now. > > > > > > > > Ack. > > > > > > > > We should still define which receiver corresponds to a given port. Probably > > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > > > What do you think? > > > > > > AFAICT it would be a completely seperate node, since they have different address > > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > > outside. > > > > Thanks for clearing up the confusion. If these are truly different kinds of > > devices, then don't they also deserve different compatible strings? And > > possibly also different DT binding documentation in a separate file. > > Allwinner's documents aren't particularly clear about this. AFAICT it does not > say anything about them being different. The CSI section gives a register table > with two base addresses. The V3S SDK listed on linux-sunxi wiki also doesn't > differentiate between the two. What's more is there's actually code and register > addresses for the MIPI stuff [1], though the license is questionable, > as with most > BSPs. It indeed seems like a CSI controller, which basically just takes whatever > input it is configured for and sends it to either a downstream ISP or DRAM. > There are MIPI-CSI receiver and DPHY blocks in addresses following CSI0, but we > can treat them as separate hardware blocks. What you could still do is to change the DT binding documentation to only apply to the parallel receiver, not the CSI-2 receiver. That'd give you more freedom going forward in case you'd later implement support for the CSI-2 receiver, too. Up to you. -- Sakari Ailus sakari.ailus at linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-09-28 14:47 ` Sakari Ailus 0 siblings, 0 replies; 48+ messages in thread From: Sakari Ailus @ 2018-09-28 14:47 UTC (permalink / raw) To: Chen-Yu Tsai Cc: Laurent Pinchart, Yong Deng, Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, David Miller, Greg Kroah-Hartman, Andrew Morton, Arnd Bergmann, Hans Verkuil, Geert Uytterhoeven, Jacob Chen, Neil Armstrong, Thierry Reding, Philipp Zabel, Todor Tomov, Linux Media Mailing List On Fri, Sep 28, 2018 at 09:42:50PM +0800, Chen-Yu Tsai wrote: > Hi, > > On Fri, Sep 28, 2018 at 8:56 PM Sakari Ailus > <sakari.ailus@linux.intel.com> wrote: > > > > Hi Chen-Yu, > > > > On Fri, Sep 28, 2018 at 07:10:58PM +0800, Chen-Yu Tsai wrote: > > > On Fri, Sep 28, 2018 at 6:23 PM Sakari Ailus > > > <sakari.ailus@linux.intel.com> wrote: > > > > > > > > Hi Laurent, > > > > > > > > On Fri, Sep 28, 2018 at 12:45:12PM +0300, Laurent Pinchart wrote: > > > > > Hi Sakari, > > > > > > > > > > On Friday, 28 September 2018 12:38:33 EEST Sakari Ailus wrote: > > > > > > On Wed, Sep 26, 2018 at 04:40:04PM +0800, Yong Deng wrote: > > > > > > > Add binding documentation for Allwinner V3s CSI. > > > > > > > > > > > > > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > > > > > > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > > > > > > > > > > > I know... but I have a few more comments. > > > > > > > > > > > > > Reviewed-by: Rob Herring <robh@kernel.org> > > > > > > > Signed-off-by: Yong Deng <yong.deng@magewell.com> > > > > > > > --- > > > > > > > > > > > > > > .../devicetree/bindings/media/sun6i-csi.txt | 59 +++++++++++++++++ > > > > > > > 1 file changed, 59 insertions(+) > > > > > > > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > b/Documentation/devicetree/bindings/media/sun6i-csi.txt new file mode > > > > > > > 100644 > > > > > > > index 000000000000..2ff47a9507a6 > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > > > > > > > @@ -0,0 +1,59 @@ > > > > > > > +Allwinner V3s Camera Sensor Interface > > > > > > > +------------------------------------- > > > > > > > + > > > > > > > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > > > > > > > +interface and CSI1 is used for parallel interface. > > > > > > > + > > > > > > > +Required properties: > > > > > > > + - compatible: value must be "allwinner,sun8i-v3s-csi" > > > > > > > + - reg: base address and size of the memory-mapped region. > > > > > > > + - interrupts: interrupt associated to this IP > > > > > > > + - clocks: phandles to the clocks feeding the CSI > > > > > > > + * bus: the CSI interface clock > > > > > > > + * mod: the CSI module clock > > > > > > > + * ram: the CSI DRAM clock > > > > > > > + - clock-names: the clock names mentioned above > > > > > > > + - resets: phandles to the reset line driving the CSI > > > > > > > + > > > > > > > +Each CSI node should contain one 'port' child node with one child > > > > > > > 'endpoint' +node, according to the bindings defined in > > > > > > > +Documentation/devicetree/bindings/media/video-interfaces.txt. As > > > > > > > mentioned > > > > > > > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel > > > > > > > or +Bt656 for CSI1. > > > > > > > > > > > > Which port represents CSI0 and which one is CSI1? That needs to be > > > > > > documented. > > > > > > > > > > There are two CSI devices, named CSI0 and CSI1, with one port each. The CSI0 > > > > > device supports CSI-2 only, and the CSI1 device parallel (BT.601 or BT.656) > > > > > only. > > > > > > > > > > > > + > > > > > > > +Endpoint node properties for CSI1 > > > > > > > > > > > > How about CSI0? I'd expect at least data-lanes, and clock-lanes as well if > > > > > > the hardware supports lane mapping. > > > > > > > > > > I enquired about that too. As far as I understand, CSI0 isn't supported yet in > > > > > the driver due to lack of documentation and lack of open-source vendor- > > > > > provided source code. While DT bindings are not tied to driver > > > > > implementations, it's not the best idea to design DT bindings without at least > > > > > one working implementation to test them. I thus proposed just listing CSI0 as > > > > > being unsupported for now. > > > > > > > > Ack. > > > > > > > > We should still define which receiver corresponds to a given port. Probably > > > > 1 for CSI1 would make sense, in order to avoid changing the order the > > > > hardware already uses. 0 doesn't need to be documented no IMO. > > > > > > > > What do you think? > > > > > > AFAICT it would be a completely seperate node, since they have different address > > > spaces, clocks and reset controls. So there's no possibility of confusion. > > > > > > According to Yong, CSI0 is tied internally to some unknown MIPI CSI2-receiver, > > > which is the undocumented part. CSI1 has its parallel data pins exposed to the > > > outside. > > > > Thanks for clearing up the confusion. If these are truly different kinds of > > devices, then don't they also deserve different compatible strings? And > > possibly also different DT binding documentation in a separate file. > > Allwinner's documents aren't particularly clear about this. AFAICT it does not > say anything about them being different. The CSI section gives a register table > with two base addresses. The V3S SDK listed on linux-sunxi wiki also doesn't > differentiate between the two. What's more is there's actually code and register > addresses for the MIPI stuff [1], though the license is questionable, > as with most > BSPs. It indeed seems like a CSI controller, which basically just takes whatever > input it is configured for and sends it to either a downstream ISP or DRAM. > There are MIPI-CSI receiver and DPHY blocks in addresses following CSI0, but we > can treat them as separate hardware blocks. What you could still do is to change the DT binding documentation to only apply to the parallel receiver, not the CSI-2 receiver. That'd give you more freedom going forward in case you'd later implement support for the CSI-2 receiver, too. Up to you. -- Sakari Ailus sakari.ailus@linux.intel.com ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [linux-sunxi] [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) 2018-09-26 8:40 ` Yong Deng (?) @ 2018-10-17 7:33 ` Jagan Teki -1 siblings, 0 replies; 48+ messages in thread From: Jagan Teki @ 2018-10-17 7:33 UTC (permalink / raw) To: Yong Deng Cc: Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, davem, Greg Kroah-Hartman, akpm, arnd, Hans Verkuil, laurent.pinchart, geert, jacob-chen, Neil Armstrong, treding, Philipp Zabel, todor.tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel, linux-sunxi On Wed, Sep 26, 2018 at 2:11 PM Yong Deng <yong.deng@magewell.com> wrote: > > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt > new file mode 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child 'endpoint' > +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or > +Bt656 for CSI1. But A64 manual claimed that CSI0 is parallel (ofcourse it has only one controller). On the other-side the register space seems similar. and also is Bt656 and CCIR656 are same types? -- Jagan Teki Senior Linux Kernel Engineer | Amarula Solutions U-Boot, Linux | Upstream Maintainer Hyderabad, India. ^ permalink raw reply [flat|nested] 48+ messages in thread
* [linux-sunxi] [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-10-17 7:33 ` Jagan Teki 0 siblings, 0 replies; 48+ messages in thread From: Jagan Teki @ 2018-10-17 7:33 UTC (permalink / raw) To: linux-arm-kernel On Wed, Sep 26, 2018 at 2:11 PM Yong Deng <yong.deng@magewell.com> wrote: > > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt > new file mode 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child 'endpoint' > +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or > +Bt656 for CSI1. But A64 manual claimed that CSI0 is parallel (ofcourse it has only one controller). On the other-side the register space seems similar. and also is Bt656 and CCIR656 are same types? -- Jagan Teki Senior Linux Kernel Engineer | Amarula Solutions U-Boot, Linux | Upstream Maintainer Hyderabad, India. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [linux-sunxi] [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) @ 2018-10-17 7:33 ` Jagan Teki 0 siblings, 0 replies; 48+ messages in thread From: Jagan Teki @ 2018-10-17 7:33 UTC (permalink / raw) To: Yong Deng Cc: Maxime Ripard, Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Chen-Yu Tsai, davem, Greg Kroah-Hartman, akpm, arnd, Hans Verkuil, laurent.pinchart, geert, jacob-chen, Neil Armstrong, treding, Philipp Zabel, todor.tomov, Sakari Ailus, linux-media, devicetree, linux-arm-kernel, linux-kernel On Wed, Sep 26, 2018 at 2:11 PM Yong Deng <yong.deng@magewell.com> wrote: > > Add binding documentation for Allwinner V3s CSI. > > Acked-by: Maxime Ripard <maxime.ripard@bootlin.com> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Yong Deng <yong.deng@magewell.com> > --- > .../devicetree/bindings/media/sun6i-csi.txt | 59 ++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/sun6i-csi.txt > > diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt > new file mode 100644 > index 000000000000..2ff47a9507a6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt > @@ -0,0 +1,59 @@ > +Allwinner V3s Camera Sensor Interface > +------------------------------------- > + > +Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 > +interface and CSI1 is used for parallel interface. > + > +Required properties: > + - compatible: value must be "allwinner,sun8i-v3s-csi" > + - reg: base address and size of the memory-mapped region. > + - interrupts: interrupt associated to this IP > + - clocks: phandles to the clocks feeding the CSI > + * bus: the CSI interface clock > + * mod: the CSI module clock > + * ram: the CSI DRAM clock > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset line driving the CSI > + > +Each CSI node should contain one 'port' child node with one child 'endpoint' > +node, according to the bindings defined in > +Documentation/devicetree/bindings/media/video-interfaces.txt. As mentioned > +above, the endpoint's bus type should be MIPI CSI-2 for CSI0 and parallel or > +Bt656 for CSI1. But A64 manual claimed that CSI0 is parallel (ofcourse it has only one controller). On the other-side the register space seems similar. and also is Bt656 and CCIR656 are same types? -- Jagan Teki Senior Linux Kernel Engineer | Amarula Solutions U-Boot, Linux | Upstream Maintainer Hyderabad, India. ^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2018-10-17 7:33 UTC | newest] Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-26 8:40 [PATCH v11 1/2] dt-bindings: media: Add Allwinner V3s Camera Sensor Interface (CSI) Yong Deng 2018-09-26 8:40 ` Yong Deng 2018-09-26 8:40 ` Yong Deng 2018-09-26 10:19 ` Laurent Pinchart 2018-09-26 10:19 ` Laurent Pinchart 2018-09-26 10:19 ` Laurent Pinchart 2018-09-26 10:35 ` maxime.ripard 2018-09-26 10:35 ` maxime.ripard at bootlin.com 2018-09-26 10:35 ` maxime.ripard-LDxbnhwyfcJBDgjK7y7TUQ 2018-09-26 10:38 ` Laurent Pinchart 2018-09-26 10:38 ` Laurent Pinchart 2018-09-26 10:38 ` Laurent Pinchart 2018-09-26 10:40 ` Maxime Ripard 2018-09-26 10:40 ` Maxime Ripard 2018-09-26 10:40 ` Maxime Ripard 2018-09-27 0:59 ` Yong 2018-09-27 0:59 ` Yong 2018-09-27 0:59 ` Yong 2018-09-28 9:38 ` Sakari Ailus 2018-09-28 9:38 ` Sakari Ailus 2018-09-28 9:38 ` Sakari Ailus 2018-09-28 9:45 ` Laurent Pinchart 2018-09-28 9:45 ` Laurent Pinchart 2018-09-28 9:45 ` Laurent Pinchart 2018-09-28 10:23 ` Sakari Ailus 2018-09-28 10:23 ` Sakari Ailus 2018-09-28 10:23 ` Sakari Ailus 2018-09-28 11:10 ` Chen-Yu Tsai 2018-09-28 11:10 ` Chen-Yu Tsai 2018-09-28 11:10 ` Chen-Yu Tsai 2018-09-28 12:56 ` Sakari Ailus 2018-09-28 12:56 ` Sakari Ailus 2018-09-28 12:56 ` Sakari Ailus 2018-09-28 13:36 ` Maxime Ripard 2018-09-28 13:36 ` Maxime Ripard 2018-09-28 13:36 ` Maxime Ripard 2018-09-28 13:42 ` Sakari Ailus 2018-09-28 13:42 ` Sakari Ailus 2018-09-28 13:42 ` Sakari Ailus 2018-09-28 13:42 ` Chen-Yu Tsai 2018-09-28 13:42 ` Chen-Yu Tsai 2018-09-28 13:42 ` Chen-Yu Tsai 2018-09-28 14:47 ` Sakari Ailus 2018-09-28 14:47 ` Sakari Ailus 2018-09-28 14:47 ` Sakari Ailus 2018-10-17 7:33 ` [linux-sunxi] " Jagan Teki 2018-10-17 7:33 ` Jagan Teki 2018-10-17 7:33 ` Jagan Teki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.