Re: [PATCH RFC] media: i2c: mt9p031: add OF support
Hi,
One more point for your devicetree conversions,
On Mon, Apr 29, 2013 at 01:30:01PM +0530, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
add OF support for the mt9p031 sensor driver.
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct i2c_client *client)
+
+{
+ if (!client-dev.platform_data client-dev.of_node) {
+ struct device_node *np;
+ struct mt9p031_platform_data *pdata;
+ int ret;
+
+ np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL);
+ if (!np)
+ return NULL;
+
+ pdata = devm_kzalloc(client-dev,
+ sizeof(struct mt9p031_platform_data),
+ GFP_KERNEL);
+ if (!pdata) {
+ pr_warn(mt9p031 failed allocate memeory\n);
+ return NULL;
+ }
+ ret = of_property_read_u32(np, reset, pdata-reset);
+ if (ret == -EINVAL)
+ pdata-reset = -1;
+ else if (ret == -ENODATA)
+ return NULL;
+
+ if (of_property_read_u32(np, ext_freq, pdata-ext_freq))
+ return NULL;
+
+ if (of_property_read_u32(np, target_freq,
+ pdata-target_freq))
+ return NULL;
+
+ return pdata;
+ }
I don't know how the others see this, but IMO it would be cleaner to
first add a duplicate of the members of pdata in struct mt9p031 and then
initialize them either from pdata or from devicetree data. The
(artificial) creation of platform_data for the devicetree case adds a
new level of indirection. This may not be a problem here, but there are
cases where there is no 1:1 transcription between pdata and devicetree
possible.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
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: [PATCH RFC] media: i2c: mt9p031: add OF support
Hi Sascha,
On Tuesday 30 April 2013 08:16:25 Sascha Hauer wrote:
On Mon, Apr 29, 2013 at 01:30:01PM +0530, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
add OF support for the mt9p031 sensor driver.
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct i2c_client *client)
+
+{
+ if (!client-dev.platform_data client-dev.of_node) {
+ struct device_node *np;
+ struct mt9p031_platform_data *pdata;
+ int ret;
+
+ np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL);
+ if (!np)
+ return NULL;
+
+ pdata = devm_kzalloc(client-dev,
+sizeof(struct mt9p031_platform_data),
+GFP_KERNEL);
+ if (!pdata) {
+ pr_warn(mt9p031 failed allocate memeory\n);
+ return NULL;
+ }
+ ret = of_property_read_u32(np, reset, pdata-reset);
+ if (ret == -EINVAL)
+ pdata-reset = -1;
+ else if (ret == -ENODATA)
+ return NULL;
+
+ if (of_property_read_u32(np, ext_freq, pdata-ext_freq))
+ return NULL;
+
+ if (of_property_read_u32(np, target_freq,
+pdata-target_freq))
+ return NULL;
+
+ return pdata;
+ }
I don't know how the others see this, but IMO it would be cleaner to
first add a duplicate of the members of pdata in struct mt9p031 and then
initialize them either from pdata or from devicetree data. The
(artificial) creation of platform_data for the devicetree case adds a
new level of indirection. This may not be a problem here, but there are
cases where there is no 1:1 transcription between pdata and devicetree
possible.
I have no strong opinion on this. In the mt9p031 case it won't matter much,
but it's probably a good idea in general.
--
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
[PATCH RFC] media: i2c: mt9p031: add OF support
From: Lad, Prabhakar prabhakar.cse...@gmail.com
add OF support for the mt9p031 sensor driver.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@redhat.com
Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Rob Landley r...@landley.net
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
.../devicetree/bindings/media/i2c/mt9p031.txt | 43 ++
drivers/media/i2c/mt9p031.c| 61 +++-
2 files changed, 103 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
new file mode 100644
index 000..b985e63
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
@@ -0,0 +1,43 @@
+* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
+
+The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor with
+an active imaging pixel array of 2592H x 1944V. It incorporates sophisticated
+camera functions on-chip such as windowing, column and row skip mode, and
+snapshot mode. It is programmable through a simple two-wire serial interface.
+
+The MT9P031 is a progressive-scan sensor that generates a stream of pixel data
+at a constant frame rate. It uses an on-chip, phase-locked loop (PLL) to
+generate all internal clocks from a single master input clock running between 6
+and 27 MHz. The maximum pixel rate is 96 Mp/s, corresponding to a clock rate of
+96 MHz.
+
+Required Properties :
+- compatible : value should be either one among the following
+ (a) aptina,mt9p031-sensor for mt9p031 sensor
+ (b) aptina,mt9p031m-sensor for mt9p031m sensor
+
+- ext_freq: Input clock frequency.
+
+- target_freq: Pixel clock frequency.
+
+Optional Properties :
+-reset: Chip reset GPIO (If not specified defaults to -1)
+
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+ mt9p031@5d {
+ compatible = aptina,mt9p031-sensor;
+ reg = 0x5d;
+
+ port {
+ mt9p031_1: endpoint {
+ ext_freq = 600;
+ target_freq = 9600;
+ };
+ };
+ };
+ ...
+};
\ No newline at end of file
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 28cf95b..66078a6 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -23,11 +23,13 @@
#include linux/regulator/consumer.h
#include linux/slab.h
#include linux/videodev2.h
+#include linux/of_device.h
#include media/mt9p031.h
#include media/v4l2-chip-ident.h
#include media/v4l2-ctrls.h
#include media/v4l2-device.h
+#include media/v4l2-of.h
#include media/v4l2-subdev.h
#include aptina-pll.h
@@ -928,10 +930,66 @@ static const struct v4l2_subdev_internal_ops
mt9p031_subdev_internal_ops = {
* Driver initialization and probing
*/
+#if defined(CONFIG_OF)
+static const struct of_device_id mt9p031_of_match[] = {
+ {.compatible = aptina,mt9p031-sensor, },
+ {.compatible = aptina,mt9p031m-sensor, },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt9p031_of_match);
+
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct i2c_client *client)
+
+{
+ if (!client-dev.platform_data client-dev.of_node) {
+ struct device_node *np;
+ struct mt9p031_platform_data *pdata;
+ int ret;
+
+ np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL);
+ if (!np)
+ return NULL;
+
+ pdata = devm_kzalloc(client-dev,
+sizeof(struct mt9p031_platform_data),
+GFP_KERNEL);
+ if (!pdata) {
+ pr_warn(mt9p031 failed allocate memeory\n);
+ return NULL;
+ }
+ ret = of_property_read_u32(np, reset, pdata-reset);
+ if (ret == -EINVAL)
+ pdata-reset = -1;
+ else if (ret == -ENODATA)
+ return NULL;
+
+ if (of_property_read_u32(np, ext_freq, pdata-ext_freq))
+ return NULL;
+
+ if (of_property_read_u32(np, target_freq,
+pdata-target_freq))
+ return NULL;
+
+ return pdata;
+ }
+
+ return NULL;
+}
+#else
+#define mt9p031_of_match NULL
+
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct
Re: [PATCH RFC] media: i2c: mt9p031: add OF support
On Mon, Apr 29, 2013 at 01:30:01PM +0530, Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
add OF support for the mt9p031 sensor driver.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@redhat.com
Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Rob Landley r...@landley.net
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
---
.../devicetree/bindings/media/i2c/mt9p031.txt | 43 ++
drivers/media/i2c/mt9p031.c| 61
+++-
2 files changed, 103 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
new file mode 100644
index 000..b985e63
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
@@ -0,0 +1,43 @@
+* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
+
+The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor
with
+an active imaging pixel array of 2592H x 1944V. It incorporates sophisticated
+camera functions on-chip such as windowing, column and row skip mode, and
+snapshot mode. It is programmable through a simple two-wire serial interface.
+
+The MT9P031 is a progressive-scan sensor that generates a stream of pixel
data
+at a constant frame rate. It uses an on-chip, phase-locked loop (PLL) to
+generate all internal clocks from a single master input clock running
between 6
+and 27 MHz. The maximum pixel rate is 96 Mp/s, corresponding to a clock rate
of
+96 MHz.
+
+Required Properties :
+- compatible : value should be either one among the following
+ (a) aptina,mt9p031-sensor for mt9p031 sensor
+ (b) aptina,mt9p031m-sensor for mt9p031m sensor
+
+- ext_freq: Input clock frequency.
+
+- target_freq: Pixel clock frequency.
For devicetree properties '-' is preferred over '_'. Most devicetree
bindings we already have suggest that we shoud use 'frequency' and no
abbreviation. probably 'clock-frequency' should be used.
+
+Optional Properties :
+-reset: Chip reset GPIO (If not specified defaults to -1)
gpios must be specified as phandles, see of_get_named_gpio().
+
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+ mt9p031@5d {
+ compatible = aptina,mt9p031-sensor;
+ reg = 0x5d;
+
+ port {
+ mt9p031_1: endpoint {
+ ext_freq = 600;
+ target_freq = 9600;
+ };
+ };
+ };
+ ...
+};
\ No newline at end of file
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 28cf95b..66078a6 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -23,11 +23,13 @@
#include linux/regulator/consumer.h
#include linux/slab.h
#include linux/videodev2.h
+#include linux/of_device.h
#include media/mt9p031.h
#include media/v4l2-chip-ident.h
#include media/v4l2-ctrls.h
#include media/v4l2-device.h
+#include media/v4l2-of.h
#include media/v4l2-subdev.h
#include aptina-pll.h
@@ -928,10 +930,66 @@ static const struct v4l2_subdev_internal_ops
mt9p031_subdev_internal_ops = {
* Driver initialization and probing
*/
+#if defined(CONFIG_OF)
+static const struct of_device_id mt9p031_of_match[] = {
+ {.compatible = aptina,mt9p031-sensor, },
+ {.compatible = aptina,mt9p031m-sensor, },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt9p031_of_match);
+
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct i2c_client *client)
+
+{
+ if (!client-dev.platform_data client-dev.of_node) {
Just because the Kernel is compiled with devicetree support does not
necessarily mean you actually boot from devicetree. You must still
handle platform data properly.
+ struct device_node *np;
+ struct mt9p031_platform_data *pdata;
+ int ret;
+
+ np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL);
+ if (!np)
+ return NULL;
+
+ pdata = devm_kzalloc(client-dev,
+ sizeof(struct mt9p031_platform_data),
+ GFP_KERNEL);
+ if (!pdata) {
+ pr_warn(mt9p031 failed allocate memeory\n);
Use dev_* for messages inside drivers.
+ return NULL;
+ }
+ ret = of_property_read_u32(np, reset, pdata-reset);
+
Re: [PATCH RFC] media: i2c: mt9p031: add OF support
Hi Sascha,
Thanks for the quick review.
On Mon, Apr 29, 2013 at 1:51 PM, Sascha Hauer s.ha...@pengutronix.de wrote:
Snip
+Required Properties :
+- compatible : value should be either one among the following
+ (a) aptina,mt9p031-sensor for mt9p031 sensor
+ (b) aptina,mt9p031m-sensor for mt9p031m sensor
+
+- ext_freq: Input clock frequency.
+
+- target_freq: Pixel clock frequency.
For devicetree properties '-' is preferred over '_'. Most devicetree
bindings we already have suggest that we shoud use 'frequency' and no
abbreviation. probably 'clock-frequency' should be used.
Yeah i missed it.. following is the proposed bindings,
let me know if something is wrong with it.
Required Properties :
- compatible : value should be either one among the following
(a) aptina,mt9p031-sensor for mt9p031 sensor
(b) aptina,mt9p031m-sensor for mt9p031m sensor
- input-clock-frequency: Input clock frequency.
- pixel-clock-frequency: Pixel clock frequency.
Optional Properties :
-gpio-reset: Chip reset GPIO (If not specified defaults to -1)
Example:
i2c0@1c22000 {
...
...
mt9p031@5d {
compatible = aptina,mt9p031-sensor;
reg = 0x5d;
port {
mt9p031_1: endpoint {
input-clock-frequency = 600;
pixel-clock-frequency = 9600;
gpio-reset = gpio3 30 0;
};
};
};
...
};
+
+Optional Properties :
+-reset: Chip reset GPIO (If not specified defaults to -1)
gpios must be specified as phandles, see of_get_named_gpio().
Fixed it.
+
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+ mt9p031@5d {
+ compatible = aptina,mt9p031-sensor;
+ reg = 0x5d;
+
+ port {
+ mt9p031_1: endpoint {
+ ext_freq = 600;
+ target_freq = 9600;
+ };
+ };
+ };
+ ...
+};
\ No newline at end of file
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 28cf95b..66078a6 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -23,11 +23,13 @@
#include linux/regulator/consumer.h
#include linux/slab.h
#include linux/videodev2.h
+#include linux/of_device.h
#include media/mt9p031.h
#include media/v4l2-chip-ident.h
#include media/v4l2-ctrls.h
#include media/v4l2-device.h
+#include media/v4l2-of.h
#include media/v4l2-subdev.h
#include aptina-pll.h
@@ -928,10 +930,66 @@ static const struct v4l2_subdev_internal_ops
mt9p031_subdev_internal_ops = {
* Driver initialization and probing
*/
+#if defined(CONFIG_OF)
+static const struct of_device_id mt9p031_of_match[] = {
+ {.compatible = aptina,mt9p031-sensor, },
+ {.compatible = aptina,mt9p031m-sensor, },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt9p031_of_match);
+
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct i2c_client *client)
+
+{
+ if (!client-dev.platform_data client-dev.of_node) {
Just because the Kernel is compiled with devicetree support does not
necessarily mean you actually boot from devicetree. You must still
handle platform data properly.
OK. which one should be given higher preference client-dev.of_node
or the client-dev.platform_data ?
+ struct device_node *np;
+ struct mt9p031_platform_data *pdata;
+ int ret;
+
+ np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL);
+ if (!np)
+ return NULL;
+
+ pdata = devm_kzalloc(client-dev,
+ sizeof(struct mt9p031_platform_data),
+ GFP_KERNEL);
+ if (!pdata) {
+ pr_warn(mt9p031 failed allocate memeory\n);
Use dev_* for messages inside drivers.
Fixed it.
Regards,
--Prabhakar Lad
--
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: [PATCH RFC] media: i2c: mt9p031: add OF support
Hi Prabhakar,
On Monday 29 April 2013 16:41:07 Prabhakar Lad wrote:
On Mon, Apr 29, 2013 at 1:51 PM, Sascha Hauer wrote:
Snip
+Required Properties :
+- compatible : value should be either one among the following
+ (a) aptina,mt9p031-sensor for mt9p031 sensor
+ (b) aptina,mt9p031m-sensor for mt9p031m sensor
+
+- ext_freq: Input clock frequency.
+
+- target_freq: Pixel clock frequency.
For devicetree properties '-' is preferred over '_'. Most devicetree
bindings we already have suggest that we shoud use 'frequency' and no
abbreviation. probably 'clock-frequency' should be used.
Yeah i missed it.. following is the proposed bindings,
let me know if something is wrong with it.
Required Properties :
- compatible : value should be either one among the following
(a) aptina,mt9p031-sensor for mt9p031 sensor
(b) aptina,mt9p031m-sensor for mt9p031m sensor
What about just aptina,mt9p031 and aptina,mt9p031m ?
- input-clock-frequency: Input clock frequency.
- pixel-clock-frequency: Pixel clock frequency.
Optional Properties :
-gpio-reset: Chip reset GPIO (If not specified defaults to -1)
You can remove the (If not specified defaults to -1).
Example:
i2c0@1c22000 {
...
...
mt9p031@5d {
compatible = aptina,mt9p031-sensor;
reg = 0x5d;
port {
mt9p031_1: endpoint {
input-clock-frequency = 600;
pixel-clock-frequency = 9600;
gpio-reset = gpio3 30 0;
At least the reset GPIO should be a property of the mt9p031 node, not the
endpoint.
};
};
};
...
};
+
+Optional Properties :
+-reset: Chip reset GPIO (If not specified defaults to -1)
gpios must be specified as phandles, see of_get_named_gpio().
Fixed it.
+
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+ mt9p031@5d {
+ compatible = aptina,mt9p031-sensor;
+ reg = 0x5d;
+
+ port {
+ mt9p031_1: endpoint {
+ ext_freq = 600;
+ target_freq = 9600;
+ };
+ };
+ };
+ ...
+};
\ No newline at end of file
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 28cf95b..66078a6 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -23,11 +23,13 @@
#include linux/regulator/consumer.h
#include linux/slab.h
#include linux/videodev2.h
+#include linux/of_device.h
Please keep headers alphabetically sorted.
#include media/mt9p031.h
#include media/v4l2-chip-ident.h
#include media/v4l2-ctrls.h
#include media/v4l2-device.h
+#include media/v4l2-of.h
#include media/v4l2-subdev.h
#include aptina-pll.h
@@ -928,10 +930,66 @@ static const struct v4l2_subdev_internal_ops
mt9p031_subdev_internal_ops = {
* Driver initialization and probing
*/
+#if defined(CONFIG_OF)
+static const struct of_device_id mt9p031_of_match[] = {
+ {.compatible = aptina,mt9p031-sensor, },
+ {.compatible = aptina,mt9p031m-sensor, },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt9p031_of_match);
+
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct i2c_client *client)
Please split the line after the * and align the function name on the left:
static struct mt9p031_platform_data *
mt9p031_get_pdata(struct i2c_client *client)
+
+{
+ if (!client-dev.platform_data client-dev.of_node) {
Just because the Kernel is compiled with devicetree support does not
necessarily mean you actually boot from devicetree. You must still
handle platform data properly.
OK. which one should be given higher preference client-dev.of_node
or the client-dev.platform_data ?
of_node should have a higher priority.
--
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: [PATCH RFC] media: i2c: mt9p031: add OF support
Hi Prabhakar,
Thank you for the patch. Please see below for a couple of comments in addition
to the ones I've just sent (in reply to Sascha's e-mail).
On Monday 29 April 2013 13:30:01 Prabhakar Lad wrote:
From: Lad, Prabhakar prabhakar.cse...@gmail.com
add OF support for the mt9p031 sensor driver.
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Cc: Hans Verkuil hans.verk...@cisco.com
Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
Cc: Mauro Carvalho Chehab mche...@redhat.com
Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
Cc: Sylwester Nawrocki s.nawro...@samsung.com
Cc: Sakari Ailus sakari.ai...@iki.fi
Cc: Grant Likely grant.lik...@secretlab.ca
Cc: Rob Herring rob.herr...@calxeda.com
Cc: Rob Landley r...@landley.net
Cc: devicetree-disc...@lists.ozlabs.org
Cc: linux-...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
[snip]
---
.../devicetree/bindings/media/i2c/mt9p031.txt | 43 ++
drivers/media/i2c/mt9p031.c| 61 -
2 files changed, 103 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt new file mode
100644
index 000..b985e63
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
@@ -0,0 +1,43 @@
+* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
+
+The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor
+with an active imaging pixel array of 2592H x 1944V. It incorporates
+sophisticated camera functions on-chip such as windowing, column and row
+skip mode, and snapshot mode. It is programmable through a simple two-wire
+serial interface.
+The MT9P031 is a progressive-scan sensor that generates a stream of pixel
+data at a constant frame rate. It uses an on-chip, phase-locked loop (PLL)
+to generate all internal clocks from a single master input clock running
+between 6 and 27 MHz. The maximum pixel rate is 96 Mp/s, corresponding to
+a clock rate of 96 MHz.
Isn't this text (directly copied from the datasheet) under Aptina's copyright
?
+Required Properties :
+- compatible : value should be either one among the following
+ (a) aptina,mt9p031-sensor for mt9p031 sensor
+ (b) aptina,mt9p031m-sensor for mt9p031m sensor
+
+- ext_freq: Input clock frequency.
+
+- target_freq: Pixel clock frequency.
+
+Optional Properties :
+-reset: Chip reset GPIO (If not specified defaults to -1)
+
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+ mt9p031@5d {
+ compatible = aptina,mt9p031-sensor;
+ reg = 0x5d;
+
+ port {
+ mt9p031_1: endpoint {
+ ext_freq = 600;
+ target_freq = 9600;
+ };
+ };
+ };
+ ...
+};
\ No newline at end of file
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 28cf95b..66078a6 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -23,11 +23,13 @@
#include linux/regulator/consumer.h
#include linux/slab.h
#include linux/videodev2.h
+#include linux/of_device.h
#include media/mt9p031.h
#include media/v4l2-chip-ident.h
#include media/v4l2-ctrls.h
#include media/v4l2-device.h
+#include media/v4l2-of.h
#include media/v4l2-subdev.h
#include aptina-pll.h
@@ -928,10 +930,66 @@ static const struct v4l2_subdev_internal_ops
mt9p031_subdev_internal_ops = { * Driver initialization and probing
*/
+#if defined(CONFIG_OF)
+static const struct of_device_id mt9p031_of_match[] = {
+ {.compatible = aptina,mt9p031-sensor, },
+ {.compatible = aptina,mt9p031m-sensor, },
As you'll need to resubmit anyway, please add a space between '{' and '.'
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt9p031_of_match);
+
+static struct mt9p031_platform_data
+ *mt9p031_get_pdata(struct i2c_client *client)
+
+{
+ if (!client-dev.platform_data client-dev.of_node) {
+ struct device_node *np;
+ struct mt9p031_platform_data *pdata;
+ int ret;
+
+ np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL);
+ if (!np)
+ return NULL;
+
+ pdata = devm_kzalloc(client-dev,
+ sizeof(struct mt9p031_platform_data),
+ GFP_KERNEL);
+ if (!pdata) {
+ pr_warn(mt9p031 failed allocate memeory\n);
+ return NULL;
+ }
+ ret = of_property_read_u32(np, reset, pdata-reset);
+ if (ret == -EINVAL)
+ pdata-reset = -1;
+ else if (ret == -ENODATA)
+ return NULL;
+
+ if
Re: [PATCH RFC] media: i2c: mt9p031: add OF support
Hi Laurent,
On Mon, Apr 29, 2013 at 5:05 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
Hi Prabhakar,
Thank you for the patch. Please see below for a couple of comments in addition
to the ones I've just sent (in reply to Sascha's e-mail).
Yep fixed them all.
[snip]
---
.../devicetree/bindings/media/i2c/mt9p031.txt | 43 ++
drivers/media/i2c/mt9p031.c| 61 -
2 files changed, 103 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt new file mode
100644
index 000..b985e63
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt
@@ -0,0 +1,43 @@
+* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor
+
+The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image sensor
+with an active imaging pixel array of 2592H x 1944V. It incorporates
+sophisticated camera functions on-chip such as windowing, column and row
+skip mode, and snapshot mode. It is programmable through a simple two-wire
+serial interface.
+The MT9P031 is a progressive-scan sensor that generates a stream of pixel
+data at a constant frame rate. It uses an on-chip, phase-locked loop (PLL)
+to generate all internal clocks from a single master input clock running
+between 6 and 27 MHz. The maximum pixel rate is 96 Mp/s, corresponding to
+a clock rate of 96 MHz.
Isn't this text (directly copied from the datasheet) under Aptina's copyright
?
hmm :) do you want me change it ?
+Required Properties :
+- compatible : value should be either one among the following
+ (a) aptina,mt9p031-sensor for mt9p031 sensor
+ (b) aptina,mt9p031m-sensor for mt9p031m sensor
+
+- ext_freq: Input clock frequency.
+
+- target_freq: Pixel clock frequency.
+
+Optional Properties :
+-reset: Chip reset GPIO (If not specified defaults to -1)
+
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+ mt9p031@5d {
+ compatible = aptina,mt9p031-sensor;
+ reg = 0x5d;
+
+ port {
+ mt9p031_1: endpoint {
+ ext_freq = 600;
+ target_freq = 9600;
+ };
+ };
+ };
+ ...
+};
\ No newline at end of file
diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 28cf95b..66078a6 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -23,11 +23,13 @@
#include linux/regulator/consumer.h
#include linux/slab.h
#include linux/videodev2.h
+#include linux/of_device.h
#include media/mt9p031.h
#include media/v4l2-chip-ident.h
#include media/v4l2-ctrls.h
#include media/v4l2-device.h
+#include media/v4l2-of.h
#include media/v4l2-subdev.h
#include aptina-pll.h
@@ -928,10 +930,66 @@ static const struct v4l2_subdev_internal_ops
mt9p031_subdev_internal_ops = { * Driver initialization and probing
*/
+#if defined(CONFIG_OF)
+static const struct of_device_id mt9p031_of_match[] = {
+ {.compatible = aptina,mt9p031-sensor, },
+ {.compatible = aptina,mt9p031m-sensor, },
As you'll need to resubmit anyway, please add a space between '{' and '.'
OK
+ {},
+};
+MODULE_DEVICE_TABLE(of, mt9p031_of_match);
+
[Snip]
- struct mt9p031_platform_data *pdata = client-dev.platform_data;
+ struct mt9p031_platform_data *pdata = mt9p031_get_pdata(client);
struct i2c_adapter *adapter = to_i2c_adapter(client-dev.parent);
struct mt9p031 *mt9p031;
unsigned int i;
@@ -1072,6 +1130,7 @@ MODULE_DEVICE_TABLE(i2c, mt9p031_id);
static struct i2c_driver mt9p031_i2c_driver = {
.driver = {
+ .of_match_table = mt9p031_of_match,
You can use the of_match_ptr() macro instead of defining mt9p031_of_match as
NULL above.
Ok
Regards,
--Prabhakar Lad
--
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: [PATCH RFC] media: i2c: mt9p031: add OF support
On Monday 29 April 2013 17:18:02 Prabhakar Lad wrote: On Mon, Apr 29, 2013 at 5:05 PM, Laurent Pinchart wrote: Hi Prabhakar, Thank you for the patch. Please see below for a couple of comments in addition to the ones I've just sent (in reply to Sascha's e-mail). Yep fixed them all. [snip] --- .../devicetree/bindings/media/i2c/mt9p031.txt | 43 ++ drivers/media/i2c/mt9p031.c| 61 +- 2 files changed, 103 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/i2c/mt9p031.txt diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt new file mode 100644 index 000..b985e63 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt @@ -0,0 +1,43 @@ +* Aptina 1/2.5-Inch 5Mp CMOS Digital Image Sensor + +The Aptina MT9P031 is a 1/2.5-inch CMOS active pixel digital image +sensor with an active imaging pixel array of 2592H x 1944V. It +incorporates sophisticated camera functions on-chip such as windowing, +column and row skip mode, and snapshot mode. It is programmable through +a simple two-wire serial interface. +The MT9P031 is a progressive-scan sensor that generates a stream of +pixel data at a constant frame rate. It uses an on-chip, phase-locked +loop (PLL) to generate all internal clocks from a single master input +clock running between 6 and 27 MHz. The maximum pixel rate is 96 Mp/s, +corresponding to a clock rate of 96 MHz. Isn't this text (directly copied from the datasheet) under Aptina's copyright ? hmm :) do you want me change it ? From a personal point of view I don't care much, and I don't think Aptina would either, but I'd rather be safe than sorry on such matters, so it would probably be a good idea to change the text. One or two sentences should be enough. -- 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
