Re: [RFC/PATCH 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Laurent and Sylwester, My apologies for the late answer. On Fri, Jul 27, 2012 at 12:50:13AM +0200, Laurent Pinchart wrote: Hi Sylwester, On Thursday 26 July 2012 22:39:31 Sylwester Nawrocki wrote: On 07/26/2012 05:21 PM, Laurent Pinchart wrote: On Friday 25 May 2012 21:52:48 Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 + 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; +- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601; +- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V); +- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V); +- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); + +Optional properties: + +- clock-frequency : the IP's main (system bus) clock frequency in Hz, the default Is that the input clock frequency ? Can't it vary ? Instead of accessing the Yes, the description is incorrect in this patch, it should read: +- clock-frequency : the sensor's master clock frequency in Hz; and be a required property. As in this patch: https://github.com/snawrocki/linux/commit/e8a5f890dec0d7414b656bb1d1ac97d5e7 abe563 It could vary (as this is a PLL input frequency), so probably a range would be a better alternative. Given that host device won't always be able to set this exact value... A range sounds good, or perhaps a list of ranges. Sakari, what would you need for the SMIA++ driver ? Typically the sensor's external clock is derived from another clock using a divisor. This means there's usually quite limited selection of possible clock frequencies since the sensors usually have a small range of possible frequencies such as 6 -- 27 MHz or even less. Also it's important to choose the frequency in such a way that it doesn't interfere with other devices in the system. The frequency also must be picked so that one can achieve the desired highest data transfer rate. The sensors are also quite flexible in their internal clock tree configuration, but in the situation where the desired data rate is close to sensor limits there may be additional constraints. Still it's always possible to come up with a best value for the board while other values are inferior. For these reasons I see little point in providing anything else than just a single value for the external clock frequency. This value is board-specific. sensor clock frequency from the FIMC driver you should reference a clock in the sensor DT node. That obviously requires generic clock support, which might not be available for your platform yet (that's one of the reasons the OMAP3 ISP driver doesn't support DT yet). I agree it might be better, but waiting unknown number of kernel releases for the platforms to get converted to common clock API is not a good alternative either. I guess we could have some transitional solutions while other subsystems are getting adapted. I agree, we need an interim solution. The SMIA++ driver allows the platform data to have either the clock name or a function pointer to set the clock frequency. If the clock name is there it'll be used instead. The function pointer can be removed once it's no longer needed. Kind regards, -- Sakari Ailus e-mail: sakari.ai...@iki.fi jabber/XMPP/Gmail: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
On Thu, 26 Jul 2012, Laurent Pinchart wrote: Hi Sylwester, On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git-tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. Hrm... That's how early pre-subdev versions of soc-camera used to work, that's where all the device_video_probe() functions come from. But then we switched to dynamic i2c device registration. Do we want to switch all drivers back now?... Couldn't we temporarily use references from subdevs to hosts until the clock API is available? 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-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Guennadi, On Tuesday 31 July 2012 11:56:44 Guennadi Liakhovetski wrote: On Thu, 26 Jul 2012, Laurent Pinchart wrote: On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git-tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. Hrm... That's how early pre-subdev versions of soc-camera used to work, that's where all the device_video_probe() functions come from. But then we switched to dynamic i2c device registration. Do we want to switch all drivers back now?... Couldn't we temporarily use references from subdevs to hosts until the clock API is available? I don't think that requires a reference from subdevs to hosts in the DT. The subdev will need the host to be probed before a clock can be available so you won't be able to access the hardware in the probe() function in the generic case. You will need to wait until the registered() subdev operation is called, at which point the host can be accessed through the v4l2_device. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
On Tue, 31 Jul 2012, Laurent Pinchart wrote: Hi Guennadi, On Tuesday 31 July 2012 11:56:44 Guennadi Liakhovetski wrote: On Thu, 26 Jul 2012, Laurent Pinchart wrote: On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git-tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. Hrm... That's how early pre-subdev versions of soc-camera used to work, that's where all the device_video_probe() functions come from. But then we switched to dynamic i2c device registration. Do we want to switch all drivers back now?... Couldn't we temporarily use references from subdevs to hosts until the clock API is available? I don't think that requires a reference from subdevs to hosts in the DT. The subdev will need the host to be probed before a clock can be available so you won't be able to access the hardware in the probe() function in the generic case. You will need to wait until the registered() subdev operation is called, at which point the host can be accessed through the v4l2_device. Sure, I understand, but that's exactly what we wanted to avoid - succeeding client's i2c .probe() without even touching the hardware. 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-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Guennadi, On Tuesday 31 July 2012 13:14:13 Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Laurent Pinchart wrote: On Tuesday 31 July 2012 11:56:44 Guennadi Liakhovetski wrote: On Thu, 26 Jul 2012, Laurent Pinchart wrote: On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git-tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. Hrm... That's how early pre-subdev versions of soc-camera used to work, that's where all the device_video_probe() functions come from. But then we switched to dynamic i2c device registration. Do we want to switch all drivers back now?... Couldn't we temporarily use references from subdevs to hosts until the clock API is available? I don't think that requires a reference from subdevs to hosts in the DT. The subdev will need the host to be probed before a clock can be available so you won't be able to access the hardware in the probe() function in the generic case. You will need to wait until the registered() subdev operation is called, at which point the host can be accessed through the v4l2_device. Sure, I understand, but that's exactly what we wanted to avoid - succeeding client's i2c .probe() without even touching the hardware. But should we allow host probe() to succeed if the sensor isn't present ? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
On Tue, 31 Jul 2012, Laurent Pinchart wrote: Hi Guennadi, On Tuesday 31 July 2012 13:14:13 Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Laurent Pinchart wrote: On Tuesday 31 July 2012 11:56:44 Guennadi Liakhovetski wrote: On Thu, 26 Jul 2012, Laurent Pinchart wrote: On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git-tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. Hrm... That's how early pre-subdev versions of soc-camera used to work, that's where all the device_video_probe() functions come from. But then we switched to dynamic i2c device registration. Do we want to switch all drivers back now?... Couldn't we temporarily use references from subdevs to hosts until the clock API is available? I don't think that requires a reference from subdevs to hosts in the DT. The subdev will need the host to be probed before a clock can be available so you won't be able to access the hardware in the probe() function in the generic case. You will need to wait until the registered() subdev operation is called, at which point the host can be accessed through the v4l2_device. Sure, I understand, but that's exactly what we wanted to avoid - succeeding client's i2c .probe() without even touching the hardware. But should we allow host probe() to succeed if the sensor isn't present ? I think we should, yes. The host hardware is there and functional - whether or not all or some of the clients are failing. Theoretically clients can also be hot-plugged. Whether and how many video device nodes we create, that's a different question. 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-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Guennadi, On Tuesday 31 July 2012 13:29:55 Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Laurent Pinchart wrote: On Tuesday 31 July 2012 13:14:13 Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Laurent Pinchart wrote: On Tuesday 31 July 2012 11:56:44 Guennadi Liakhovetski wrote: On Thu, 26 Jul 2012, Laurent Pinchart wrote: On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git- tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. Hrm... That's how early pre-subdev versions of soc-camera used to work, that's where all the device_video_probe() functions come from. But then we switched to dynamic i2c device registration. Do we want to switch all drivers back now?... Couldn't we temporarily use references from subdevs to hosts until the clock API is available? I don't think that requires a reference from subdevs to hosts in the DT. The subdev will need the host to be probed before a clock can be available so you won't be able to access the hardware in the probe() function in the generic case. You will need to wait until the registered() subdev operation is called, at which point the host can be accessed through the v4l2_device. Sure, I understand, but that's exactly what we wanted to avoid - succeeding client's i2c .probe() without even touching the hardware. But should we allow host probe() to succeed if the sensor isn't present ? I think we should, yes. The host hardware is there and functional - whether or not all or some of the clients are failing. Theoretically clients can also be hot-plugged. Whether and how many video device nodes we create, that's a different question. I think I can agree with you on this (although I could change my mind if this architecture turns out to result in unsolvable technical issues). That will involve a lot of work though. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
On Tue, 31 Jul 2012, Laurent Pinchart wrote: Hi Guennadi, On Tuesday 31 July 2012 13:29:55 Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Laurent Pinchart wrote: On Tuesday 31 July 2012 13:14:13 Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Laurent Pinchart wrote: On Tuesday 31 July 2012 11:56:44 Guennadi Liakhovetski wrote: On Thu, 26 Jul 2012, Laurent Pinchart wrote: On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.t xt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git- tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. Hrm... That's how early pre-subdev versions of soc-camera used to work, that's where all the device_video_probe() functions come from. But then we switched to dynamic i2c device registration. Do we want to switch all drivers back now?... Couldn't we temporarily use references from subdevs to hosts until the clock API is available? I don't think that requires a reference from subdevs to hosts in the DT. The subdev will need the host to be probed before a clock can be available so you won't be able to access the hardware in the probe() function in the generic case. You will need to wait until the registered() subdev operation is called, at which point the host can be accessed through the v4l2_device. Sure, I understand, but that's exactly what we wanted to avoid - succeeding client's i2c .probe() without even touching the hardware. But should we allow host probe() to succeed if the sensor isn't present ? I think we should, yes. The host hardware is there and functional - whether or not all or some of the clients are failing. Theoretically clients can also be hot-plugged. Whether and how many video device nodes we create, that's a different question. I think I can agree with you on this (although I could change my mind if this architecture turns out to result in unsolvable technical issues). That will involve a lot of work though. There's however at least one more gotcha that occurs to me with this approach: if clients fail to probe, how do we find out about that and turn clocks back off? One improvement to turning clocks on immediately in host's probe() is to only do it in a BUS_NOTIFY_BIND_DRIVER notifier. But how do we find out, that probing failed? No notifier is called in this case. We could use a time-out, but that's ugly. I think, we could ever request a new notifier for this case. We could also require client
Re: [RFC/PATCH 09/13] media: s5k6aa: Add support for device tree based instantiation
On 07/31/2012 02:26 PM, Guennadi Liakhovetski wrote: But should we allow host probe() to succeed if the sensor isn't present ? I think we should, yes. The host hardware is there and functional - whether or not all or some of the clients are failing. Theoretically clients can also be hot-plugged. Whether and how many video device nodes we create, that's a different question. I think I can agree with you on this (although I could change my mind if this architecture turns out to result in unsolvable technical issues). That will involve a lot of work though. There's however at least one more gotcha that occurs to me with this approach: if clients fail to probe, how do we find out about that and turn clocks back off? One improvement to turning clocks on immediately in Hmm, wouldn't it be the client that turns a clock on/off when needed ? I'd like to preserve this functionality, so client drivers can have full control on the power up/down sequences. While we are trying to improve the current situation... host's probe() is to only do it in a BUS_NOTIFY_BIND_DRIVER notifier. But how do we find out, that probing failed? No notifier is called in this case. We could use a time-out, but that's ugly. I think, we could ever request a new notifier for this case. We could also require client drivers to call a V4L2 function in this case, but that's not very pretty either. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
On Tue, 31 Jul 2012, Sylwester Nawrocki wrote: On 07/31/2012 02:26 PM, Guennadi Liakhovetski wrote: But should we allow host probe() to succeed if the sensor isn't present ? I think we should, yes. The host hardware is there and functional - whether or not all or some of the clients are failing. Theoretically clients can also be hot-plugged. Whether and how many video device nodes we create, that's a different question. I think I can agree with you on this (although I could change my mind if this architecture turns out to result in unsolvable technical issues). That will involve a lot of work though. There's however at least one more gotcha that occurs to me with this approach: if clients fail to probe, how do we find out about that and turn clocks back off? One improvement to turning clocks on immediately in Hmm, wouldn't it be the client that turns a clock on/off when needed ? I'd like to preserve this functionality, so client drivers can have full control on the power up/down sequences. While we are trying to improve the current situation... Eventually, when the clock API is available - yes, the client would just call clk_enable() / clk_disable(). But for now isn't it host's job to do that? Or you want to add new API for that? Thanks Guennadi host's probe() is to only do it in a BUS_NOTIFY_BIND_DRIVER notifier. But how do we find out, that probing failed? No notifier is called in this case. We could use a time-out, but that's ugly. I think, we could ever request a new notifier for this case. We could also require client drivers to call a V4L2 function in this case, but that's not very pretty either. --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
On 07/31/2012 02:59 PM, Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Sylwester Nawrocki wrote: On 07/31/2012 02:26 PM, Guennadi Liakhovetski wrote: But should we allow host probe() to succeed if the sensor isn't present ? I think we should, yes. The host hardware is there and functional - whether or not all or some of the clients are failing. Theoretically clients can also be hot-plugged. Whether and how many video device nodes we create, that's a different question. I think I can agree with you on this (although I could change my mind if this architecture turns out to result in unsolvable technical issues). That will involve a lot of work though. There's however at least one more gotcha that occurs to me with this approach: if clients fail to probe, how do we find out about that and turn clocks back off? One improvement to turning clocks on immediately in Hmm, wouldn't it be the client that turns a clock on/off when needed ? I'd like to preserve this functionality, so client drivers can have full control on the power up/down sequences. While we are trying to improve the current situation... Eventually, when the clock API is available - yes, the client would just call clk_enable() / clk_disable(). But for now isn't it host's job to do that? Or you want to add new API for that? Indeed, looking at existing drivers, the clocks' handling is now mostly done in the host drivers. Only the omap3isp appears to do the right thing, i.e. delegating control over the clock to client drivers, but it does it with platform_data callbacks. We've already discussed adding a new API for that, http://www.mail-archive.com/linux-media@vger.kernel.org/msg35359.html However using common clock API and binding a clock to client device (either DT based or not) sounds like a best approach to me. Waiting for the common clock API to be generally available maybe we could add some clock ops at the v4l2_device ? Just a humble suggestion, I'm not sure whether it is really good and needed or not. -- Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Sylwester, On Tuesday 31 July 2012 15:28:52 Sylwester Nawrocki wrote: On 07/31/2012 02:59 PM, Guennadi Liakhovetski wrote: On Tue, 31 Jul 2012, Sylwester Nawrocki wrote: On 07/31/2012 02:26 PM, Guennadi Liakhovetski wrote: But should we allow host probe() to succeed if the sensor isn't present ? I think we should, yes. The host hardware is there and functional - whether or not all or some of the clients are failing. Theoretically clients can also be hot-plugged. Whether and how many video device nodes we create, that's a different question. I think I can agree with you on this (although I could change my mind if this architecture turns out to result in unsolvable technical issues). That will involve a lot of work though. There's however at least one more gotcha that occurs to me with this approach: if clients fail to probe, how do we find out about that and turn clocks back off? One improvement to turning clocks on immediately in Hmm, wouldn't it be the client that turns a clock on/off when needed ? I'd like to preserve this functionality, so client drivers can have full control on the power up/down sequences. While we are trying to improve the current situation... Eventually, when the clock API is available - yes, the client would just call clk_enable() / clk_disable(). But for now isn't it host's job to do that? Or you want to add new API for that? Indeed, looking at existing drivers, the clocks' handling is now mostly done in the host drivers. Only the omap3isp appears to do the right thing, i.e. delegating control over the clock to client drivers, but it does it with platform_data callbacks. We've already discussed adding a new API for that, http://www.mail-archive.com/linux-media@vger.kernel.org/msg35359.html However using common clock API and binding a clock to client device (either DT based or not) sounds like a best approach to me. Waiting for the common clock API to be generally available maybe we could add some clock ops at the v4l2_device ? Just a humble suggestion, I'm not sure whether it is really good and needed or not. I'm fine with that (or something similar) as an interim solution. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Sylwester, On Wednesday 18 July 2012 11:18:33 Sylwester Nawrocki wrote: On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git-tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. That's how I've envisioned the DT bindings for sensors as well, this sounds good. The real challenge will be to get hold of the subdev to register it without race conditions. +- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601; If this is a boolean property, it cannot be required? Otherwise, as discussed in a different patch comment, this property might not be required, and if it is, I would use the same definition for both the camera interface and for sensors. Yeah, that's an omission, a leftover from previous versions. It should have been 'video-bus-type', for this series. Here is the updated binding documentation: 8-- Required properties: - compatible : samsung,s5k6aafx; - reg : base address of the device on I2C bus; - vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V); - vdda-supply : analog circuits voltage supply 2.8V (2.6V to 3.0V); - vdd_reg-supply : regulator input voltage supply 1.8V (1.7V to 1.9V) or 2.8V (2.6V to 3.0); - vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V) or 2.8V (2.5V to 3.1V); - video-bus-type : sensor video output bus type, must be one of: itu-601, mipi-csi2; - clock-frequency : the sensor's master clock frequency in Hz; Optional properties: - data-lanes : number of physical MIPI-CSI2 data lanes used (default is 2 when this property is not specified); - gpios : specifies gpios connected to the sensor's reset reset (RSTN) and standby (STBYN) pins, the order of gpios is: RSTN, STBYN; If any of these gpios is not used, its specifier shall be 0; - samsung,s5k6aa-gpio-lvls : this property (bits [1:0]) specifies an active state of the S5K6AAFX reset and stand-by signals; the meaning of bits is: bit[1:0] = [RST, STBY], e.g. bit[1] = 0 indicates the GPIO driving the S5K6AAFX's standby pin should be set to 1 to bring the sensor out from stand-by to normal operation state. - samsung,s5k6aa-hflip : sets the default horizontal image flipping; - samsung,s5k6aa-vflip : sets the default vertical image flipping; 8-- +- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V); +- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V); +- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); I think, underscores in property names are generally frowned upon. Indeed, I felt something might be wrong here;) It's in this form because I just copied regulator supply names from the existing driver. And I didn't want to change them, to avoid updating them in all board files where they're used. Thanks for pointing it out, I'll fix that in next iteration. +- vddio-supply : I/O voltage supply 1.8V
Re: [RFC/PATCH 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Sylwester,
On Friday 25 May 2012 21:52:48 Sylwester Nawrocki wrote:
The driver initializes all board related properties except the s_power()
callback to board code. The platforms that require this callback are not
supported by this driver yet for CONFIG_OF=y.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
.../bindings/camera/samsung-s5k6aafx.txt | 57 +
drivers/media/video/s5k6aa.c | 129 -
2 files changed, 146 insertions(+), 40 deletions(-)
create mode 100644
Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file
mode 100644
index 000..6685a9c
--- /dev/null
+++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
@@ -0,0 +1,57 @@
+Samsung S5K6AAFX camera sensor
+--
+
+Required properties:
+
+- compatible : samsung,s5k6aafx;
+- reg : base address of the device on I2C bus;
+- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601;
+- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V);
+- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V);
+- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V)
+or 2.8V (2.6V to 3.0);
+- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V)
+ or 2.8V (2.5V to 3.1V);
+
+Optional properties:
+
+- clock-frequency : the IP's main (system bus) clock frequency in Hz, the
default
Is that the input clock frequency ? Can't it vary ? Instead of accessing the
sensor clock frequency from the FIMC driver you should reference a clock in
the sensor DT node. That obviously requires generic clock support, which might
not be available for your platform yet (that's one of the reasons the OMAP3
ISP driver doesn't support DT yet).
+ value when this property is not specified is 24 MHz;
+- data-lanes : number of physical lanes used (default 2 if not specified);
+- gpio-stby : specifies the S5K6AA_STBY GPIO
+- gpio-rst : specifies the S5K6AA_RST GPIO
+- samsung,s5k6aa-inv-stby : set inverted S5K6AA_STBY GPIO level;
+- samsung,s5k6aa-inv-rst : set inverted S5K6AA_RST GPIO level;
+- samsung,s5k6aa-hflip : set the default horizontal image flipping;
+- samsung,s5k6aa-vflip : set the default vertical image flipping;
+
+
+Example:
+
+ gpl2: gpio-controller@0 {
+ };
+
+ reg0: regulator@0 {
+ };
+
+ reg1: regulator@1 {
+ };
+
+ reg2: regulator@2 {
+ };
+
+ reg3: regulator@3 {
+ };
+
+ s5k6aafx@3c {
+ compatible = samsung,s5k6aafx;
+ reg = 0x3c;
+ clock-frequency = 2400;
+ gpio-rst = gpl2 0 2 0 3;
+ gpio-stby = gpl2 1 2 0 3;
+ video-itu-601-bus;
+ vdd_core-supply = reg0;
+ vdda-supply = reg1;
+ vdd_reg-supply = reg2;
+ vddio-supply = reg3;
+ };
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Laurent, On 07/26/2012 05:21 PM, Laurent Pinchart wrote: On Friday 25 May 2012 21:52:48 Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 - 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; +- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601; +- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V); +- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V); +- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); + +Optional properties: + +- clock-frequency : the IP's main (system bus) clock frequency in Hz, the default Is that the input clock frequency ? Can't it vary ? Instead of accessing the Yes, the description is incorrect in this patch, it should read: +- clock-frequency : the sensor's master clock frequency in Hz; and be a required property. As in this patch: https://github.com/snawrocki/linux/commit/e8a5f890dec0d7414b656bb1d1ac97d5e7abe563 It could vary (as this is a PLL input frequency), so probably a range would be a better alternative. Given that host device won't always be able to set this exact value... sensor clock frequency from the FIMC driver you should reference a clock in the sensor DT node. That obviously requires generic clock support, which might not be available for your platform yet (that's one of the reasons the OMAP3 ISP driver doesn't support DT yet). I agree it might be better, but waiting unknown number of kernel releases for the platforms to get converted to common clock API is not a good alternative either. I guess we could have some transitional solutions while other subsystems are getting adapted. Yet we need to specify the clock frequency range per sensor, so 1. either we specify it at a sensor node and host device driver references it, or 2. it could be added to a sensor specific child node of a host device mode, and then only the host would reference it, and sensor would reference a clock in its DT node; I guess it's not a problem that in most cases the camera host device is a clock provider. -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Sylwester, On Thursday 26 July 2012 22:39:31 Sylwester Nawrocki wrote: On 07/26/2012 05:21 PM, Laurent Pinchart wrote: On Friday 25 May 2012 21:52:48 Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 + 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; +- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601; +- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V); +- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V); +- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); +- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); + +Optional properties: + +- clock-frequency : the IP's main (system bus) clock frequency in Hz, the default Is that the input clock frequency ? Can't it vary ? Instead of accessing the Yes, the description is incorrect in this patch, it should read: +- clock-frequency : the sensor's master clock frequency in Hz; and be a required property. As in this patch: https://github.com/snawrocki/linux/commit/e8a5f890dec0d7414b656bb1d1ac97d5e7 abe563 It could vary (as this is a PLL input frequency), so probably a range would be a better alternative. Given that host device won't always be able to set this exact value... A range sounds good, or perhaps a list of ranges. Sakari, what would you need for the SMIA++ driver ? sensor clock frequency from the FIMC driver you should reference a clock in the sensor DT node. That obviously requires generic clock support, which might not be available for your platform yet (that's one of the reasons the OMAP3 ISP driver doesn't support DT yet). I agree it might be better, but waiting unknown number of kernel releases for the platforms to get converted to common clock API is not a good alternative either. I guess we could have some transitional solutions while other subsystems are getting adapted. I agree, we need an interim solution. Yet we need to specify the clock frequency range per sensor, so 1. either we specify it at a sensor node and host device driver references it, or 2. it could be added to a sensor specific child node of a host device mode, and then only the host would reference it, and sensor would reference a clock in its DT node; I guess it's not a problem that in most cases the camera host device is a clock provider. The sensor will need to configure the clock rate, so a (list of) clock frequency range(s) will be needed in the sensor node anyway. As an interim solution the host can access that property. When the platform will be ported to the common clock API no modification to the DT will be needed. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media 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 09/13] media: s5k6aa: Add support for device tree based instantiation
Hi Guennadi, On 07/16/2012 11:42 AM, Guennadi Liakhovetski wrote: On Fri, 25 May 2012, Sylwester Nawrocki wrote: The driver initializes all board related properties except the s_power() callback to board code. The platforms that require this callback are not supported by this driver yet for CONFIG_OF=y. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Bartlomiej Zolnierkiewiczb.zolnier...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com --- .../bindings/camera/samsung-s5k6aafx.txt | 57 + drivers/media/video/s5k6aa.c | 129 ++-- 2 files changed, 146 insertions(+), 40 deletions(-) create mode 100644 Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt new file mode 100644 index 000..6685a9c --- /dev/null +++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt @@ -0,0 +1,57 @@ +Samsung S5K6AAFX camera sensor +-- + +Required properties: + +- compatible : samsung,s5k6aafx; +- reg : base address of the device on I2C bus; You said you ended up putting your sensors outside of I2C busses, is this one of changes, that are present in your git-tree but not in this series? No, I must have been not clear enough on that. Our idea was to keep I2C slave device nodes as an I2C controller's child nodes, according to the current convention. The 'sensor' nodes (the 'camera''s children) would only contain a phandle to a respective I2C slave node. This implies that we cannot access I2C bus in I2C client's device probe() callback. An actual H/W access could begin only from within and after invocation of v4l2_subdev .registered callback.. +- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601; If this is a boolean property, it cannot be required? Otherwise, as discussed in a different patch comment, this property might not be required, and if it is, I would use the same definition for both the camera interface and for sensors. Yeah, that's an omission, a leftover from previous versions. It should have been 'video-bus-type', for this series. Here is the updated binding documentation: 8-- Required properties: - compatible : samsung,s5k6aafx; - reg : base address of the device on I2C bus; - vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V); - vdda-supply : analog circuits voltage supply 2.8V (2.6V to 3.0V); - vdd_reg-supply : regulator input voltage supply 1.8V (1.7V to 1.9V) or 2.8V (2.6V to 3.0); - vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V) or 2.8V (2.5V to 3.1V); - video-bus-type : sensor video output bus type, must be one of: itu-601, mipi-csi2; - clock-frequency : the sensor's master clock frequency in Hz; Optional properties: - data-lanes : number of physical MIPI-CSI2 data lanes used (default is 2 when this property is not specified); - gpios : specifies gpios connected to the sensor's reset reset (RSTN) and standby (STBYN) pins, the order of gpios is: RSTN, STBYN; If any of these gpios is not used, its specifier shall be 0; - samsung,s5k6aa-gpio-lvls : this property (bits [1:0]) specifies an active state of the S5K6AAFX reset and stand-by signals; the meaning of bits is: bit[1:0] = [RST, STBY], e.g. bit[1] = 0 indicates the GPIO driving the S5K6AAFX's standby pin should be set to 1 to bring the sensor out from stand-by to normal operation state. - samsung,s5k6aa-hflip : sets the default horizontal image flipping; - samsung,s5k6aa-vflip : sets the default vertical image flipping; 8-- +- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V); +- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V); +- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V) + or 2.8V (2.6V to 3.0); I think, underscores in property names are generally frowned upon. Indeed, I felt something might be wrong here;) It's in this form because I just copied regulator supply names from the existing driver. And I didn't want to change them, to avoid updating them in all board files where they're used. Thanks for pointing it out, I'll fix that in next iteration. +- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V) + or 2.8V (2.5V to 3.1V); + +Optional properties: + +- clock-frequency : the IP's main (system bus) clock frequency in Hz, the default +value
Re: [RFC/PATCH 09/13] media: s5k6aa: Add support for device tree based instantiation
On Fri, 25 May 2012, Sylwester Nawrocki wrote:
The driver initializes all board related properties except the s_power()
callback to board code. The platforms that require this callback are not
supported by this driver yet for CONFIG_OF=y.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
.../bindings/camera/samsung-s5k6aafx.txt | 57 +
drivers/media/video/s5k6aa.c | 129
++--
2 files changed, 146 insertions(+), 40 deletions(-)
create mode 100644
Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
new file mode 100644
index 000..6685a9c
--- /dev/null
+++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
@@ -0,0 +1,57 @@
+Samsung S5K6AAFX camera sensor
+--
+
+Required properties:
+
+- compatible : samsung,s5k6aafx;
+- reg : base address of the device on I2C bus;
You said you ended up putting your sensors outside of I2C busses, is this
one of changes, that are present in your git-tree but not in this series?
+- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601;
If this is a boolean property, it cannot be required? Otherwise, as
discussed in a different patch comment, this property might not be
required, and if it is, I would use the same definition for both the
camera interface and for sensors.
+- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V);
+- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V);
+- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V)
+or 2.8V (2.6V to 3.0);
I think, underscores in property names are generally frowned upon.
+- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V)
+ or 2.8V (2.5V to 3.1V);
+
+Optional properties:
+
+- clock-frequency : the IP's main (system bus) clock frequency in Hz, the
default
+ value when this property is not specified is 24 MHz;
+- data-lanes : number of physical lanes used (default 2 if not specified);
bus-width?
+- gpio-stby : specifies the S5K6AA_STBY GPIO
+- gpio-rst : specifies the S5K6AA_RST GPIO
From Documentation/devicetree/bindings/gpio/gpio.txt:
quote
GPIO properties should be named [name-]gpios.
/quote
+- samsung,s5k6aa-inv-stby : set inverted S5K6AA_STBY GPIO level;
+- samsung,s5k6aa-inv-rst : set inverted S5K6AA_RST GPIO level;
Isn't this provided by GPIO flags as described in include/linux/of_gpio.h
by using the OF_GPIO_ACTIVE_LOW bit?
+- samsung,s5k6aa-hflip : set the default horizontal image flipping;
+- samsung,s5k6aa-vflip : set the default vertical image flipping;
This is a board property, specifying how the sensor is wired and mounted
on the casing, right? IIRC, libv4l has a database of these for USB
cameras. So, maybe it belongs to the user-space for embedded systems too?
Or at least this shouldn't be Samsung-specific?
+
+
+Example:
+
+ gpl2: gpio-controller@0 {
+ };
+
+ reg0: regulator@0 {
+ };
+
+ reg1: regulator@1 {
+ };
+
+ reg2: regulator@2 {
+ };
+
+ reg3: regulator@3 {
+ };
+
+ s5k6aafx@3c {
+ compatible = samsung,s5k6aafx;
+ reg = 0x3c;
+ clock-frequency = 2400;
+ gpio-rst = gpl2 0 2 0 3;
+ gpio-stby = gpl2 1 2 0 3;
+ video-itu-601-bus;
+ vdd_core-supply = reg0;
+ vdda-supply = reg1;
+ vdd_reg-supply = reg2;
+ vddio-supply = reg3;
+ };
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-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC/PATCH 09/13] media: s5k6aa: Add support for device tree based instantiation
The driver initializes all board related properties except the s_power()
callback to board code. The platforms that require this callback are not
supported by this driver yet for CONFIG_OF=y.
Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
Signed-off-by: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
.../bindings/camera/samsung-s5k6aafx.txt | 57 +
drivers/media/video/s5k6aa.c | 129 ++--
2 files changed, 146 insertions(+), 40 deletions(-)
create mode 100644
Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
diff --git a/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
new file mode 100644
index 000..6685a9c
--- /dev/null
+++ b/Documentation/devicetree/bindings/camera/samsung-s5k6aafx.txt
@@ -0,0 +1,57 @@
+Samsung S5K6AAFX camera sensor
+--
+
+Required properties:
+
+- compatible : samsung,s5k6aafx;
+- reg : base address of the device on I2C bus;
+- video-itu-601-bus : parallel bus with HSYNC and VSYNC - ITU-R BT.601;
+- vdd_core-supply : digital core voltage supply 1.5V (1.4V to 1.6V);
+- vdda-supply : analog power voltage supply 2.8V (2.6V to 3.0V);
+- vdd_reg-supply : regulator input power voltage supply 1.8V (1.7V to 1.9V)
+ or 2.8V (2.6V to 3.0);
+- vddio-supply : I/O voltage supply 1.8V (1.65V to 1.95V)
+or 2.8V (2.5V to 3.1V);
+
+Optional properties:
+
+- clock-frequency : the IP's main (system bus) clock frequency in Hz, the
default
+ value when this property is not specified is 24 MHz;
+- data-lanes : number of physical lanes used (default 2 if not specified);
+- gpio-stby : specifies the S5K6AA_STBY GPIO
+- gpio-rst : specifies the S5K6AA_RST GPIO
+- samsung,s5k6aa-inv-stby : set inverted S5K6AA_STBY GPIO level;
+- samsung,s5k6aa-inv-rst : set inverted S5K6AA_RST GPIO level;
+- samsung,s5k6aa-hflip : set the default horizontal image flipping;
+- samsung,s5k6aa-vflip : set the default vertical image flipping;
+
+
+Example:
+
+ gpl2: gpio-controller@0 {
+ };
+
+ reg0: regulator@0 {
+ };
+
+ reg1: regulator@1 {
+ };
+
+ reg2: regulator@2 {
+ };
+
+ reg3: regulator@3 {
+ };
+
+ s5k6aafx@3c {
+ compatible = samsung,s5k6aafx;
+ reg = 0x3c;
+ clock-frequency = 2400;
+ gpio-rst = gpl2 0 2 0 3;
+ gpio-stby = gpl2 1 2 0 3;
+ video-itu-601-bus;
+ vdd_core-supply = reg0;
+ vdda-supply = reg1;
+ vdd_reg-supply = reg2;
+ vddio-supply = reg3;
+ };
diff --git a/drivers/media/video/s5k6aa.c b/drivers/media/video/s5k6aa.c
index 6625e46..ed172bb 100644
--- a/drivers/media/video/s5k6aa.c
+++ b/drivers/media/video/s5k6aa.c
@@ -20,6 +20,7 @@
#include linux/i2c.h
#include linux/media.h
#include linux/module.h
+#include linux/of_gpio.h
#include linux/regulator/consumer.h
#include linux/slab.h
@@ -232,14 +233,14 @@ struct s5k6aa {
struct media_pad pad;
enum v4l2_mbus_type bus_type;
- u8 mipi_lanes;
+ u32 mipi_lanes;
int (*s_power)(int enable);
struct regulator_bulk_data supplies[S5K6AA_NUM_SUPPLIES];
struct s5k6aa_gpio gpio[GPIO_NUM];
/* external master clock frequency */
- unsigned long mclk_frequency;
+ u32 mclk_frequency;
/* ISP internal master clock frequency */
u16 clk_fop;
/* output pixel clock frequency range */
@@ -1519,68 +1520,109 @@ static void s5k6aa_free_gpios(struct s5k6aa *s5k6aa)
}
}
-static int s5k6aa_configure_gpios(struct s5k6aa *s5k6aa,
- const struct s5k6aa_platform_data *pdata)
+static int s5k6aa_configure_gpios(struct s5k6aa *s5k6aa, struct device *dev)
{
- const struct s5k6aa_gpio *gpio = pdata-gpio_stby;
+ const struct s5k6aa_platform_data *pdata = dev-platform_data;
+ struct device_node *np = dev-of_node;
+ const struct s5k6aa_gpio *pgpio;
+ struct s5k6aa_gpio gpio = { 0 };
int ret;
s5k6aa-gpio[STBY].gpio = -EINVAL;
s5k6aa-gpio[RST].gpio = -EINVAL;
- ret = s5k6aa_configure_gpio(gpio-gpio, gpio-level, S5K6AA_STBY);
+ if (np) {
+ gpio.gpio = of_get_named_gpio(np, gpio-stby, 0);
+ if (!of_get_property(np, samsung,s5k6aa-inv-stby, NULL))
+ gpio.level = 1;
+ }
+ pgpio = np ? gpio : pdata-gpio_stby;
+ ret = s5k6aa_configure_gpio(pgpio-gpio, pgpio-level, S5K6AA_STBY);
if (ret) {
s5k6aa_free_gpios(s5k6aa);
return ret;
}
- s5k6aa-gpio[STBY] = *gpio;
- if (gpio_is_valid(gpio-gpio))
- gpio_set_value(gpio-gpio, 0);
+
