Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
On 07/31/2012 01:05 PM, Laurent Pinchart wrote: What about CSI receivers that can reroute the lanes internally ? We would need to specify lane indices for each lane then, maybe with something like clock-lane =0; data-lanes =2 3 1; Sounds good to me. And the clock-lane could be made optional, as not all devices would need it. However, as far as I can see, there is currently no generic API for handling this kind of data structure. E.g. number of cells for the interrupts property is specified with an additional #interrupt-cells property. It would have been much easier to handle something like: data-lanes = 2, 3, 1; i.e. an array of the lane indexes. I'm fine with that. ...on a second thought: shouldn't this be handled by pinctrl? Or is it some CSI-2 module internal lane switching, not the global SoC pin function configuration? On the hardware I came across, it's handled by the CSI2 receiver, not the SoC pinmux feature. Same here. Is there any hardware known to mux those MIPI-CSI D-PHY high speed differential signals with general purpose IO pins ? Or are there mostly dedicated pins used ? However, if there are cases the lane configuration is done solely at CSI2 receiver level, there seems little point in involving the pinctrl API. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Sylwester, On Tuesday 17 July 2012 20:16:23 Sylwester Nawrocki wrote: On 07/16/2012 10:55 AM, Guennadi Liakhovetski wrote: Hi Sylwester Thanks for your comments to my RFC and for pointing out to this your earlier patch series. Unfortunately, I missed in in May, let me try to provide some thoughts about this, we should really try to converge our proposals. Maybe a discussion at KS would help too. Thank you for the review. I was happy to see your RFC, as previously there seemed to be not much interest in DT among the media guys. Certainly, we need to work on a common approach to ensure interoperability of existing drivers and to avoid having people inventing different bindings for common features. I would also expect some share of device specific bindings, as diversity of media devices is significant. I'd be great to discuss these things at KS, especially support for proper suspend/resume sequences. Also having common sessions with other subsystems folks, like ASoC, for example, might be a good idea. I'm not sure if I'll be travelling to the KS though. :) On Fri, 25 May 2012, Sylwester Nawrocki wrote: s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC (camera host interface DMA engine and image processor). This patch adds support for instantiating the MIPI-CSIS devices from DT and parsing all SoC and board specific properties from device tree. The MIPI DPHY control callback is now called directly from within the driver, the platform code must ensure this callback does the right thing for each SoC. The cell-index property is used to ensure proper signal routing, from physical camera port, through MIPI-CSI2 receiver to the DMA engine (FIMC?). It's also helpful in exposing the device topology in user space at /dev/media? devnode (Media Controller API). This patch also defines a common property (data-lanes) for MIPI-CSI receivers and transmitters. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- Documentation/devicetree/bindings/video/mipi.txt |5 + .../bindings/video/samsung-mipi-csis.txt | 47 ++ drivers/media/video/s5p-fimc/mipi-csis.c | 97 +++- drivers/media/video/s5p-fimc/mipi-csis.h |1 + 4 files changed, 126 insertions(+), 24 deletions(-) create mode 100644 Documentation/devicetree/bindings/video/mipi.txt create mode 100644 Documentation/devicetree/bindings/video/samsung-mipi-csis.txt diff --git a/Documentation/devicetree/bindings/video/mipi.txt b/Documentation/devicetree/bindings/video/mipi.txt new file mode 100644 index 000..5aed285 --- /dev/null +++ b/Documentation/devicetree/bindings/video/mipi.txt @@ -0,0 +1,5 @@ +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters + + - data-lanes : number of differential data lanes wired and actively used in + communication between the transmitter and the receiver, this + excludes the clock lane; Wouldn't it be better to use the standard bus-width DT property? I can't see any problems with using bus-width. It seems sufficient and could indeed be better, without a need to invent new MIPI-CSI specific names. That was my first RFC on that and my perspective wasn't probably broad enough. :) What about CSI receivers that can reroute the lanes internally ? We would need to specify lane indices for each lane then, maybe with something like clock-lane = 0; data-lanes = 2 3 1; For receivers that can't reroute lanes internally, the data-lanes property would be need to specify lanes in sequence. data-lanes = 1 2 3; -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Sylwester, On Thursday 26 July 2012 21:51:30 Sylwester Nawrocki wrote: On 07/26/2012 04:38 PM, Laurent Pinchart wrote: --- /dev/null +++ b/Documentation/devicetree/bindings/video/mipi.txt @@ -0,0 +1,5 @@ +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters + + - data-lanes : number of differential data lanes wired and actively used in +communication between the transmitter and the receiver, this +excludes the clock lane; Wouldn't it be better to use the standard bus-width DT property? I can't see any problems with using bus-width. It seems sufficient and could indeed be better, without a need to invent new MIPI-CSI specific names. That was my first RFC on that and my perspective wasn't probably broad enough. :) What about CSI receivers that can reroute the lanes internally ? We would need to specify lane indices for each lane then, maybe with something like clock-lane =0; data-lanes =2 3 1; Sounds good to me. And the clock-lane could be made optional, as not all devices would need it. However, as far as I can see, there is currently no generic API for handling this kind of data structure. E.g. number of cells for the interrupts property is specified with an additional #interrupt-cells property. It would have been much easier to handle something like: data-lanes = 2, 3, 1; i.e. an array of the lane indexes. I'm fine with that. For receivers that can't reroute lanes internally, the data-lanes property would be need to specify lanes in sequence. data-lanes =1 2 3; In this case we would be only interested in the number of cells in this property, but how it could be retrieved ? With an array, it could have been calculated from property length returned by of_property_find() (divided by sizof(u32)). Agreed. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Guennadi, On 07/16/2012 10:55 AM, Guennadi Liakhovetski wrote: Hi Sylwester Thanks for your comments to my RFC and for pointing out to this your earlier patch series. Unfortunately, I missed in in May, let me try to provide some thoughts about this, we should really try to converge our proposals. Maybe a discussion at KS would help too. Thank you for the review. I was happy to see your RFC, as previously there seemed to be not much interest in DT among the media guys. Certainly, we need to work on a common approach to ensure interoperability of existing drivers and to avoid having people inventing different bindings for common features. I would also expect some share of device specific bindings, as diversity of media devices is significant. I'd be great to discuss these things at KS, especially support for proper suspend/resume sequences. Also having common sessions with other subsystems folks, like ASoC, for example, might be a good idea. I'm not sure if I'll be travelling to the KS though. :) On Fri, 25 May 2012, Sylwester Nawrocki wrote: s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC (camera host interface DMA engine and image processor). This patch adds support for instantiating the MIPI-CSIS devices from DT and parsing all SoC and board specific properties from device tree. The MIPI DPHY control callback is now called directly from within the driver, the platform code must ensure this callback does the right thing for each SoC. The cell-index property is used to ensure proper signal routing, from physical camera port, through MIPI-CSI2 receiver to the DMA engine (FIMC?). It's also helpful in exposing the device topology in user space at /dev/media? devnode (Media Controller API). This patch also defines a common property (data-lanes) for MIPI-CSI receivers and transmitters. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- Documentation/devicetree/bindings/video/mipi.txt |5 + .../bindings/video/samsung-mipi-csis.txt | 47 ++ drivers/media/video/s5p-fimc/mipi-csis.c | 97 +++- drivers/media/video/s5p-fimc/mipi-csis.h |1 + 4 files changed, 126 insertions(+), 24 deletions(-) create mode 100644 Documentation/devicetree/bindings/video/mipi.txt create mode 100644 Documentation/devicetree/bindings/video/samsung-mipi-csis.txt diff --git a/Documentation/devicetree/bindings/video/mipi.txt b/Documentation/devicetree/bindings/video/mipi.txt new file mode 100644 index 000..5aed285 --- /dev/null +++ b/Documentation/devicetree/bindings/video/mipi.txt @@ -0,0 +1,5 @@ +Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters + + - data-lanes : number of differential data lanes wired and actively used in +communication between the transmitter and the receiver, this +excludes the clock lane; Wouldn't it be better to use the standard bus-width DT property? I can't see any problems with using bus-width. It seems sufficient and could indeed be better, without a need to invent new MIPI-CSI specific names. That was my first RFC on that and my perspective wasn't probably broad enough. :) diff --git a/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt new file mode 100644 index 000..7bce6f4 --- /dev/null +++ b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt @@ -0,0 +1,47 @@ +Samsung S5P/EXYNOS SoC MIPI-CSI2 receiver (MIPI CSIS) +- + +Required properties: + +- compatible - one of : +samsung,s5pv210-csis, +samsung,exynos4210-csis, +samsung,exynos4212-csis, +samsung,exynos4412-csis; +- reg : physical base address and size of the device memory mapped registers; +- interrupts : should contain MIPI CSIS interrupt; the format of the +interrupt specifier depends on the interrupt controller; +- cell-index : the hardware instance index; Not sure whether this is absolutely needed... Wouldn't it be sufficient to just enumerate them during probing? As Grant pointed to me, the cell-index property is something that we should be avoiding. But I needed something like this in the driver, to differentiate between the multiple IP instances. I cannot simply assign the indexes in random way to the hardware instances. Each of the MIPI-CSI Slaves has different properties (e.g. supporting max. 2 or 4 data lanes). Additionally, a particular MIPI-CSI Slave instance is hard-wired inside the SoC to the FIMC input mux (cross-bar) and physical video port. IOW, an image sensor/video port/MIPI-CSIS instance assignment is fixed. If two MIPI-CSI image sensors are connected, one of them will work only with MIPI-CSIS0, and the other only with
Re: [RFC/PATCH 02/13] media: s5p-csis: Add device tree support
Hi Sylwester
Thanks for your comments to my RFC and for pointing out to this your
earlier patch series. Unfortunately, I missed in in May, let me try to
provide some thoughts about this, we should really try to converge our
proposals. Maybe a discussion at KS would help too.
On Fri, 25 May 2012, Sylwester Nawrocki wrote:
s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
(camera host interface DMA engine and image processor). This patch
adds support for instantiating the MIPI-CSIS devices from DT and
parsing all SoC and board specific properties from device tree.
The MIPI DPHY control callback is now called directly from within
the driver, the platform code must ensure this callback does the
right thing for each SoC.
The cell-index property is used to ensure proper signal routing,
from physical camera port, through MIPI-CSI2 receiver to the DMA
engine (FIMC?). It's also helpful in exposing the device topology
in user space at /dev/media? devnode (Media Controller API).
This patch also defines a common property (data-lanes) for MIPI-CSI
receivers and transmitters.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
Documentation/devicetree/bindings/video/mipi.txt |5 +
.../bindings/video/samsung-mipi-csis.txt | 47 ++
drivers/media/video/s5p-fimc/mipi-csis.c | 97
+++-
drivers/media/video/s5p-fimc/mipi-csis.h |1 +
4 files changed, 126 insertions(+), 24 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/mipi.txt
create mode 100644
Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
diff --git a/Documentation/devicetree/bindings/video/mipi.txt
b/Documentation/devicetree/bindings/video/mipi.txt
new file mode 100644
index 000..5aed285
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/mipi.txt
@@ -0,0 +1,5 @@
+Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters
+
+ - data-lanes : number of differential data lanes wired and actively used in
+ communication between the transmitter and the receiver, this
+ excludes the clock lane;
Wouldn't it be better to use the standard bus-width DT property?
diff --git a/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
new file mode 100644
index 000..7bce6f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
@@ -0,0 +1,47 @@
+Samsung S5P/EXYNOS SoC MIPI-CSI2 receiver (MIPI CSIS)
+-
+
+Required properties:
+
+- compatible - one of :
+ samsung,s5pv210-csis,
+ samsung,exynos4210-csis,
+ samsung,exynos4212-csis,
+ samsung,exynos4412-csis;
+- reg : physical base address and size of the device memory mapped registers;
+- interrupts : should contain MIPI CSIS interrupt; the format of the
+ interrupt specifier depends on the interrupt controller;
+- cell-index : the hardware instance index;
Not sure whether this is absolutely needed... Wouldn't it be sufficient to
just enumerate them during probing?
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, the
default
+ value when this property is not specified is 166 MHz;
+- data-lanes : number of physical MIPI-CSI2 lanes used;
ditto - bus-width?
+- samsung,csis-hs-settle : differential receiver (HS-RX) settle time;
+- vddio-supply: MIPI CSIS I/O and PLL voltage supply (e.g. 1.8V);
+- vddcore-supply : MIPI CSIS Core voltage supply (e.g. 1.1V).
+
+Example:
+
+ reg0: regulator@0 {
+ };
+
+ reg1: regulator@1 {
+ };
+
+/* SoC properties */
+
+ csis@1188 {
+ compatible = samsung,exynos4210-csis;
+ reg = 0x1188 0x1000;
+ interrupts = 0 78 0;
+ cell-index = 0;
+ };
+
+/* Board properties */
+
+ csis@1188 {
+ clock-frequency = 16600;
+ data-lanes = 2;
+ samsung,csis-hs-settle = 12;
+ vddio-supply = reg0;
+ vddcore-supply = reg1;
+ };
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 02/13] media: s5p-csis: Add device tree support
s5p-csis is platform device driver for MIPI-CSI frontend to the FIMC
(camera host interface DMA engine and image processor). This patch
adds support for instantiating the MIPI-CSIS devices from DT and
parsing all SoC and board specific properties from device tree.
The MIPI DPHY control callback is now called directly from within
the driver, the platform code must ensure this callback does the
right thing for each SoC.
The cell-index property is used to ensure proper signal routing,
from physical camera port, through MIPI-CSI2 receiver to the DMA
engine (FIMC?). It's also helpful in exposing the device topology
in user space at /dev/media? devnode (Media Controller API).
This patch also defines a common property (data-lanes) for MIPI-CSI
receivers and transmitters.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
Documentation/devicetree/bindings/video/mipi.txt |5 +
.../bindings/video/samsung-mipi-csis.txt | 47 ++
drivers/media/video/s5p-fimc/mipi-csis.c | 97 +++-
drivers/media/video/s5p-fimc/mipi-csis.h |1 +
4 files changed, 126 insertions(+), 24 deletions(-)
create mode 100644 Documentation/devicetree/bindings/video/mipi.txt
create mode 100644
Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
diff --git a/Documentation/devicetree/bindings/video/mipi.txt
b/Documentation/devicetree/bindings/video/mipi.txt
new file mode 100644
index 000..5aed285
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/mipi.txt
@@ -0,0 +1,5 @@
+Common properties of MIPI-CSI1 and MIPI-CSI2 receivers and transmitters
+
+ - data-lanes : number of differential data lanes wired and actively used in
+ communication between the transmitter and the receiver, this
+ excludes the clock lane;
diff --git a/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
new file mode 100644
index 000..7bce6f4
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/samsung-mipi-csis.txt
@@ -0,0 +1,47 @@
+Samsung S5P/EXYNOS SoC MIPI-CSI2 receiver (MIPI CSIS)
+-
+
+Required properties:
+
+- compatible - one of :
+ samsung,s5pv210-csis,
+ samsung,exynos4210-csis,
+ samsung,exynos4212-csis,
+ samsung,exynos4412-csis;
+- reg : physical base address and size of the device memory mapped registers;
+- interrupts : should contain MIPI CSIS interrupt; the format of the
+ interrupt specifier depends on the interrupt controller;
+- cell-index : the hardware instance index;
+- clock-frequency : The IP's main (system bus) clock frequency in Hz, the
default
+ value when this property is not specified is 166 MHz;
+- data-lanes : number of physical MIPI-CSI2 lanes used;
+- samsung,csis-hs-settle : differential receiver (HS-RX) settle time;
+- vddio-supply: MIPI CSIS I/O and PLL voltage supply (e.g. 1.8V);
+- vddcore-supply : MIPI CSIS Core voltage supply (e.g. 1.1V).
+
+Example:
+
+ reg0: regulator@0 {
+ };
+
+ reg1: regulator@1 {
+ };
+
+/* SoC properties */
+
+ csis@1188 {
+ compatible = samsung,exynos4210-csis;
+ reg = 0x1188 0x1000;
+ interrupts = 0 78 0;
+ cell-index = 0;
+ };
+
+/* Board properties */
+
+ csis@1188 {
+ clock-frequency = 16600;
+ data-lanes = 2;
+ samsung,csis-hs-settle = 12;
+ vddio-supply = reg0;
+ vddcore-supply = reg1;
+ };
diff --git a/drivers/media/video/s5p-fimc/mipi-csis.c
b/drivers/media/video/s5p-fimc/mipi-csis.c
index 2f73d9e..ffb820e 100644
--- a/drivers/media/video/s5p-fimc/mipi-csis.c
+++ b/drivers/media/video/s5p-fimc/mipi-csis.c
@@ -19,6 +19,7 @@
#include linux/kernel.h
#include linux/memory.h
#include linux/module.h
+#include linux/of.h
#include linux/platform_device.h
#include linux/pm_runtime.h
#include linux/regulator/consumer.h
@@ -80,10 +81,11 @@ static char *csi_clock_name[] = {
[CSIS_CLK_GATE] = csis,
};
#define NUM_CSIS_CLOCKSARRAY_SIZE(csi_clock_name)
+#define DEFAULT_SCLK_CSIS_FREQ 16600UL
static const char * const csis_supply_name[] = {
- vdd11, /* 1.1V or 1.2V (s5pc100) MIPI CSI suppply */
- vdd18, /* VDD 1.8V and MIPI CSI PLL supply */
+ vddcore, /* CSIS Core (1.0V, 1.1V or 1.2V) suppply */
+ vddio,/* CSIS I/O and PLL (1.8V) supply */
};
#define CSIS_NUM_SUPPLIES ARRAY_SIZE(csis_supply_name)
@@ -99,11 +101,15 @@ enum {
*protecting @format and @flags members
* @pads: CSIS pads array
* @sd: v4l2_subdev associated with CSIS device instance
+ * @index: the hardware instance index
* @pdev: CSIS platform device
* @regs:
