| Message ID | 1367475754-19477-1-git-send-email-prabhakar.csengg@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, May 02, 2013 at 11:52:34AM +0530, Prabhakar Lad wrote: > From: Lad, Prabhakar <prabhakar.csengg@gmail.com> > > add OF support for the mt9p031 sensor driver. > Alongside this patch sorts the header inclusion alphabetically. > > Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> > Cc: Hans Verkuil <hans.verkuil@cisco.com> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Mauro Carvalho Chehab <mchehab@redhat.com> > Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de> > Cc: Sylwester Nawrocki <s.nawrocki@samsung.com> > Cc: Sakari Ailus <sakari.ailus@iki.fi> > Cc: Grant Likely <grant.likely@secretlab.ca> > Cc: Sascha Hauer <s.hauer@pengutronix.de> > Cc: Rob Herring <rob.herring@calxeda.com> > Cc: Rob Landley <rob@landley.net> > Cc: devicetree-discuss@lists.ozlabs.org > Cc: davinci-linux-open-source@linux.davincidsp.com > Cc: linux-doc@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > Changes for v2: > 1: Used '-' instead of '_' for device properties. > 2: Specified gpio reset pin as phandle in device node. > 3: Handle platform data properly even if kernel is compiled with > devicetree support. > 4: Used dev_* for messages in drivers instead of pr_*. > 5: Changed compatible property to "aptina,mt9p031" and "aptina,mt9p031m". > 6: Sorted the header inclusion alphabetically and fixed some minor code nits. > > .../devicetree/bindings/media/i2c/mt9p031.txt | 37 ++++++++++++ > drivers/media/i2c/mt9p031.c | 62 +++++++++++++++++++- > 2 files changed, 97 insertions(+), 2 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 0000000..cbe352b > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt > @@ -0,0 +1,37 @@ > +* 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 array size of 2592H x 1944V. It is programmable through a simple > +two-wire serial interface. > + > +Required Properties : > +- compatible : value should be either one among the following > + (a) "aptina,mt9p031" for mt9p031 sensor > + (b) "aptina,mt9p031m" for mt9p031m sensor > + > +- input-clock-frequency : Input clock frequency. > + > +- pixel-clock-frequency : Pixel clock frequency. > + > +Optional Properties : > +-gpio-reset: Chip reset GPIO > + > +Example: > + > +i2c0@1c22000 { > + ... > + ... > + mt9p031@5d { > + compatible = "aptina,mt9p031"; > + reg = <0x5d>; > + gpio-reset = <&gpio3 30 0>; > + > + port { > + mt9p031_1: endpoint { > + input-clock-frequency = <6000000>; > + pixel-clock-frequency = <96000000>; > + }; > + }; > + }; > + ... > +}; > diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c > index 28cf95b..8ce3561 100644 > --- a/drivers/media/i2c/mt9p031.c > +++ b/drivers/media/i2c/mt9p031.c > @@ -16,9 +16,11 @@ > #include <linux/delay.h> > #include <linux/device.h> > #include <linux/gpio.h> > -#include <linux/module.h> > #include <linux/i2c.h> > #include <linux/log2.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/of_gpio.h> > #include <linux/pm.h> > #include <linux/regulator/consumer.h> > #include <linux/slab.h> > @@ -28,6 +30,7 @@ > #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 +931,57 @@ static const struct v4l2_subdev_internal_ops mt9p031_subdev_internal_ops = { > * Driver initialization and probing > */ > > +#if defined(CONFIG_OF) > +static struct mt9p031_platform_data * > + mt9p031_get_pdata(struct i2c_client *client) > + > +{ > + if (client->dev.of_node) { By inverting the logic here and returning immediately you can safe an indention level for the bulk of this function. > + struct device_node *np; > + struct mt9p031_platform_data *pdata; > + > + 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) { > + dev_err(&client->dev, > + "mt9p031 failed allocate memeory\n"); > + return NULL; s/memeory/memory/ Better drop this message completely. If you are really out of memory you'll notice it quite fast anyway. > + } > + pdata->reset = of_get_named_gpio(client->dev.of_node, > + "gpio-reset", 0); > + if (!gpio_is_valid(pdata->reset)) > + pdata->reset = -1; > + > + if (of_property_read_u32(np, "input-clock-frequency", > + &pdata->ext_freq)) > + return NULL; > + > + if (of_property_read_u32(np, "pixel-clock-frequency", > + &pdata->target_freq)) > + return NULL; returning NULL here means that when these properties are missing the driver bails out with the message "No platform data\n" which is not very helpful for users. How about just ignoring this here and return pdata? The driver will probably print a more useful message later when it notices that the clock params are invalid. > + > + return pdata; > + } > + > + return client->dev.platform_data; > +} > +#else > +static struct mt9p031_platform_data * > + mt9p031_get_pdata(struct i2c_client *client) > +{ > + return client->dev.platform_data; > +} > +#endif > + > static int mt9p031_probe(struct i2c_client *client, > const struct i2c_device_id *did) > { > - 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; > @@ -1070,8 +1120,16 @@ static const struct i2c_device_id mt9p031_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, mt9p031_id); > > +static const struct of_device_id mt9p031_of_match[] = { > + { .compatible = "aptina,mt9p031", }, > + { .compatible = "aptina,mt9p031m", }, > + {}, > +}; I would have expected something like: static const struct of_device_id mt9p031_of_match[] = { { .compatible = "aptina,mt9p031-sensor", .data = (void *)MT9P031_MODEL_COLOR, }, { .compatible = "aptina,mt9p031m-sensor", .data = (void *)MT9P031_MODEL_MONOCHROME, }, { /* sentinel */ }, }; of_id = of_match_device(mt9p031_of_match, &client->dev); if (of_id) mt9p031->model = (enum mt9p031_model)of_id->data; To handle monochrome sensors. Sascha
Hi Sascha, Thanks for the quick review. On Thu, May 2, 2013 at 12:25 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Thu, May 02, 2013 at 11:52:34AM +0530, Prabhakar Lad wrote: >> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> >> >> add OF support for the mt9p031 sensor driver. >> Alongside this patch sorts the header inclusion alphabetically. [Snip] >> +#if defined(CONFIG_OF) >> +static struct mt9p031_platform_data * >> + mt9p031_get_pdata(struct i2c_client *client) >> + >> +{ >> + if (client->dev.of_node) { > > By inverting the logic here and returning immediately you can safe an > indention level for the bulk of this function. > OK >> + struct device_node *np; >> + struct mt9p031_platform_data *pdata; >> + >> + 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) { >> + dev_err(&client->dev, >> + "mt9p031 failed allocate memeory\n"); >> + return NULL; > > s/memeory/memory/ > > Better drop this message completely. If you are really out of memory > you'll notice it quite fast anyway. > alright I'll drop the message. >> + } >> + pdata->reset = of_get_named_gpio(client->dev.of_node, >> + "gpio-reset", 0); >> + if (!gpio_is_valid(pdata->reset)) >> + pdata->reset = -1; >> + >> + if (of_property_read_u32(np, "input-clock-frequency", >> + &pdata->ext_freq)) >> + return NULL; >> + >> + if (of_property_read_u32(np, "pixel-clock-frequency", >> + &pdata->target_freq)) >> + return NULL; > > returning NULL here means that when these properties are missing the > driver bails out with the message "No platform data\n" which is not > very helpful for users. How about just ignoring this here and return > pdata? The driver will probably print a more useful message later when > it notices that the clock params are invalid. > Yes would be good idea of not returning NULL. >> + >> + return pdata; >> + } >> + >> + return client->dev.platform_data; >> +} >> +#else >> +static struct mt9p031_platform_data * >> + mt9p031_get_pdata(struct i2c_client *client) >> +{ >> + return client->dev.platform_data; >> +} >> +#endif >> + >> static int mt9p031_probe(struct i2c_client *client, >> const struct i2c_device_id *did) >> { >> - 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; >> @@ -1070,8 +1120,16 @@ static const struct i2c_device_id mt9p031_id[] = { >> }; >> MODULE_DEVICE_TABLE(i2c, mt9p031_id); >> >> +static const struct of_device_id mt9p031_of_match[] = { >> + { .compatible = "aptina,mt9p031", }, >> + { .compatible = "aptina,mt9p031m", }, >> + {}, >> +}; > > I would have expected something like: > > static const struct of_device_id mt9p031_of_match[] = { > { > .compatible = "aptina,mt9p031-sensor", > .data = (void *)MT9P031_MODEL_COLOR, > }, { > .compatible = "aptina,mt9p031m-sensor", > .data = (void *)MT9P031_MODEL_MONOCHROME, > }, { > /* sentinel */ > }, > }; > > of_id = of_match_device(mt9p031_of_match, &client->dev); > if (of_id) > mt9p031->model = (enum mt9p031_model)of_id->data; > > To handle monochrome sensors. > OK will do the same. Regards, --Prabhakar Lad
Hi Prabhakar, Thank you for the patch. On Thursday 02 May 2013 12:34:25 Prabhakar Lad wrote: > On Thu, May 2, 2013 at 12:25 PM, Sascha Hauer wrote: > > On Thu, May 02, 2013 at 11:52:34AM +0530, Prabhakar Lad wrote: > >> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> > >> > >> add OF support for the mt9p031 sensor driver. > >> Alongside this patch sorts the header inclusion alphabetically. > > [Snip] > > >> +#if defined(CONFIG_OF) > >> +static struct mt9p031_platform_data * > >> + mt9p031_get_pdata(struct i2c_client *client) Please align this line on the left. > >> +{ > >> + if (client->dev.of_node) { > > > > By inverting the logic here and returning immediately you can safe an > > indention level for the bulk of this function. > > OK > > >> + struct device_node *np; > >> + struct mt9p031_platform_data *pdata; > >> + > >> + 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) { > >> + dev_err(&client->dev, > >> + "mt9p031 failed allocate memeory\n"); > >> + return NULL; > > > > s/memeory/memory/ > > > > Better drop this message completely. If you are really out of memory > > you'll notice it quite fast anyway. > > alright I'll drop the message. > > >> + } > >> + pdata->reset = of_get_named_gpio(client->dev.of_node, > >> + "gpio-reset", 0); > >> + if (!gpio_is_valid(pdata->reset)) > >> + pdata->reset = -1; I've just sent a patch that converts the manual -1 checks in the driver to gpio_is_valid(). You can thus drop these two lines. > >> + if (of_property_read_u32(np, "input-clock-frequency", > >> + &pdata->ext_freq)) > >> + return NULL; > >> + > >> + if (of_property_read_u32(np, "pixel-clock-frequency", > >> + &pdata->target_freq)) > >> + return NULL; > > > > returning NULL here means that when these properties are missing the > > driver bails out with the message "No platform data\n" which is not > > very helpful for users. How about just ignoring this here and return > > pdata? The driver will probably print a more useful message later when > > it notices that the clock params are invalid. > > Yes would be good idea of not returning NULL. > > >> + > >> + return pdata; > >> + } > >> + > >> + return client->dev.platform_data; > >> +} > >> +#else > >> +static struct mt9p031_platform_data * > >> + mt9p031_get_pdata(struct i2c_client *client) > >> +{ > >> + return client->dev.platform_data; > >> +} > >> +#endif > >> + > >> > >> static int mt9p031_probe(struct i2c_client *client, > >> const struct i2c_device_id *did) > >> { > >> - 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; > >> @@ -1070,8 +1120,16 @@ static const struct i2c_device_id mt9p031_id[] = { > >> > >> }; > >> MODULE_DEVICE_TABLE(i2c, mt9p031_id); > >> > >> +static const struct of_device_id mt9p031_of_match[] = { > >> + { .compatible = "aptina,mt9p031", }, > >> + { .compatible = "aptina,mt9p031m", }, > >> + {}, > >> +}; > > > > I would have expected something like: > > > > static const struct of_device_id mt9p031_of_match[] = { > > { > > .compatible = "aptina,mt9p031-sensor", > > .data = (void *)MT9P031_MODEL_COLOR, > > }, { > > .compatible = "aptina,mt9p031m-sensor", > > .data = (void *)MT9P031_MODEL_MONOCHROME, > > }, { > > /* sentinel */ > > }, > > }; > > > > of_id = of_match_device(mt9p031_of_match, &client->dev); > > if (of_id) > > mt9p031->model = (enum mt9p031_model)of_id->data; > > > > To handle monochrome sensors. > > OK will do the same. And please guard the table with #ifdef CONFIG_OF.
Hi Laurent, Thanks for the review. On Thu, May 2, 2013 at 4:32 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Prabhakar, > > Thank you for the patch. > > On Thursday 02 May 2013 12:34:25 Prabhakar Lad wrote: >> On Thu, May 2, 2013 at 12:25 PM, Sascha Hauer wrote: >> > On Thu, May 02, 2013 at 11:52:34AM +0530, Prabhakar Lad wrote: >> >> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> >> >> >> >> add OF support for the mt9p031 sensor driver. >> >> Alongside this patch sorts the header inclusion alphabetically. >> >> [Snip] >> >> >> +#if defined(CONFIG_OF) >> >> +static struct mt9p031_platform_data * >> >> + mt9p031_get_pdata(struct i2c_client *client) > > Please align this line on the left. > OK >> >> +{ >> >> + if (client->dev.of_node) { >> > >> > By inverting the logic here and returning immediately you can safe an >> > indention level for the bulk of this function. >> >> OK >> >> >> + struct device_node *np; >> >> + struct mt9p031_platform_data *pdata; >> >> + >> >> + 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) { >> >> + dev_err(&client->dev, >> >> + "mt9p031 failed allocate memeory\n"); >> >> + return NULL; >> > >> > s/memeory/memory/ >> > >> > Better drop this message completely. If you are really out of memory >> > you'll notice it quite fast anyway. >> >> alright I'll drop the message. >> >> >> + } >> >> + pdata->reset = of_get_named_gpio(client->dev.of_node, >> >> + "gpio-reset", 0); >> >> + if (!gpio_is_valid(pdata->reset)) >> >> + pdata->reset = -1; > > I've just sent a patch that converts the manual -1 checks in the driver to > gpio_is_valid(). You can thus drop these two lines. > OK I'll drop this check. >> >> + if (of_property_read_u32(np, "input-clock-frequency", >> >> + &pdata->ext_freq)) >> >> + return NULL; >> >> + >> >> + if (of_property_read_u32(np, "pixel-clock-frequency", >> >> + &pdata->target_freq)) >> >> + return NULL; >> > >> > returning NULL here means that when these properties are missing the >> > driver bails out with the message "No platform data\n" which is not >> > very helpful for users. How about just ignoring this here and return >> > pdata? The driver will probably print a more useful message later when >> > it notices that the clock params are invalid. >> >> Yes would be good idea of not returning NULL. >> >> >> + >> >> + return pdata; >> >> + } >> >> + >> >> + return client->dev.platform_data; >> >> +} >> >> +#else >> >> +static struct mt9p031_platform_data * >> >> + mt9p031_get_pdata(struct i2c_client *client) >> >> +{ >> >> + return client->dev.platform_data; >> >> +} >> >> +#endif >> >> + >> >> >> >> static int mt9p031_probe(struct i2c_client *client, >> >> const struct i2c_device_id *did) >> >> { >> >> - 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; >> >> @@ -1070,8 +1120,16 @@ static const struct i2c_device_id mt9p031_id[] = { >> >> >> >> }; >> >> MODULE_DEVICE_TABLE(i2c, mt9p031_id); >> >> >> >> +static const struct of_device_id mt9p031_of_match[] = { >> >> + { .compatible = "aptina,mt9p031", }, >> >> + { .compatible = "aptina,mt9p031m", }, >> >> + {}, >> >> +}; >> > >> > I would have expected something like: >> > >> > static const struct of_device_id mt9p031_of_match[] = { >> > { >> > .compatible = "aptina,mt9p031-sensor", >> > .data = (void *)MT9P031_MODEL_COLOR, >> > }, { >> > .compatible = "aptina,mt9p031m-sensor", >> > .data = (void *)MT9P031_MODEL_MONOCHROME, >> > }, { >> > /* sentinel */ >> > }, >> > }; >> > >> > of_id = of_match_device(mt9p031_of_match, &client->dev); >> > if (of_id) >> > mt9p031->model = (enum mt9p031_model)of_id->data; >> > >> > To handle monochrome sensors. >> >> OK will do the same. > > And please guard the table with #ifdef CONFIG_OF. > But guarding the table #ifdef CONFIG_OF would cause compilation failure for below code when CONFIG_OF is undefined in probe of_id = of_match_device(of_match_ptr(mt9p031_of_match), &client->dev); if (of_id) mt9p031->model = (enum mt9p031_model)of_id->data; and also in mt9p031_i2c_driver structure, of_match_table = of_match_ptr(mt9p031_of_match), which force me to define mt9p031_of_match to NULL when CONFIG_OF is undefined Regards, --Prabhakar Lad
Hi Prabhakar, On Thursday 02 May 2013 18:48:37 Prabhakar Lad wrote: > On Thu, May 2, 2013 at 4:32 PM, Laurent Pinchart wrote: > > On Thursday 02 May 2013 12:34:25 Prabhakar Lad wrote: > >> On Thu, May 2, 2013 at 12:25 PM, Sascha Hauer wrote: > >> > On Thu, May 02, 2013 at 11:52:34AM +0530, Prabhakar Lad wrote: > >> >> From: Lad, Prabhakar <prabhakar.csengg@gmail.com> > >> >> > >> >> add OF support for the mt9p031 sensor driver. > >> >> Alongside this patch sorts the header inclusion alphabetically. [snip] > >> >> @@ -1070,8 +1120,16 @@ static const struct i2c_device_id mt9p031_id[] > >> >> = { > >> >> }; > >> >> MODULE_DEVICE_TABLE(i2c, mt9p031_id); > >> >> > >> >> +static const struct of_device_id mt9p031_of_match[] = { > >> >> + { .compatible = "aptina,mt9p031", }, > >> >> + { .compatible = "aptina,mt9p031m", }, > >> >> + {}, > >> >> +}; > >> > > >> > I would have expected something like: > >> > > >> > static const struct of_device_id mt9p031_of_match[] = { > >> > { > >> > .compatible = "aptina,mt9p031-sensor", > >> > .data = (void *)MT9P031_MODEL_COLOR, > >> > }, { > >> > .compatible = "aptina,mt9p031m-sensor", > >> > .data = (void *)MT9P031_MODEL_MONOCHROME, > >> > }, { > >> > /* sentinel */ > >> > }, > >> > }; > >> > > >> > of_id = of_match_device(mt9p031_of_match, &client->dev); > >> > if (of_id) > >> > mt9p031->model = (enum mt9p031_model)of_id->data; > >> > > >> > To handle monochrome sensors. > >> > >> OK will do the same. > > > > And please guard the table with #ifdef CONFIG_OF. > > But guarding the table #ifdef CONFIG_OF would cause compilation failure > for below code when CONFIG_OF is undefined in probe > > of_id = of_match_device(of_match_ptr(mt9p031_of_match), > &client->dev); > if (of_id) > mt9p031->model = (enum mt9p031_model)of_id->data; You could guard the above code with an #ifdef CONFIG_OF as well. > and also in mt9p031_i2c_driver structure, > of_match_table = of_match_ptr(mt9p031_of_match), > > which force me to define mt9p031_of_match to NULL when > CONFIG_OF is undefined of_match_ptr is defined as NULL when CONFIG_OF is disabled.
On Thursday 02 May 2013 08:55:18 Sascha Hauer wrote: > > +#if defined(CONFIG_OF) > > +static struct mt9p031_platform_data * > > + mt9p031_get_pdata(struct i2c_client *client) > > + > > +{ > > + if (client->dev.of_node) { > > By inverting the logic here and returning immediately you can safe an > indention level for the bulk of this function. Right, also make this if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) return client->dev.platform_data; Then the rest of the function gets discarded by the compiler when CONFIG_OF is not set, and you can kill the #ifdef around the function. Arnd
diff --git a/Documentation/devicetree/bindings/media/i2c/mt9p031.txt b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt new file mode 100644 index 0000000..cbe352b --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/mt9p031.txt @@ -0,0 +1,37 @@ +* 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 array size of 2592H x 1944V. It is programmable through a simple +two-wire serial interface. + +Required Properties : +- compatible : value should be either one among the following + (a) "aptina,mt9p031" for mt9p031 sensor + (b) "aptina,mt9p031m" for mt9p031m sensor + +- input-clock-frequency : Input clock frequency. + +- pixel-clock-frequency : Pixel clock frequency. + +Optional Properties : +-gpio-reset: Chip reset GPIO + +Example: + +i2c0@1c22000 { + ... + ... + mt9p031@5d { + compatible = "aptina,mt9p031"; + reg = <0x5d>; + gpio-reset = <&gpio3 30 0>; + + port { + mt9p031_1: endpoint { + input-clock-frequency = <6000000>; + pixel-clock-frequency = <96000000>; + }; + }; + }; + ... +}; diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 28cf95b..8ce3561 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -16,9 +16,11 @@ #include <linux/delay.h> #include <linux/device.h> #include <linux/gpio.h> -#include <linux/module.h> #include <linux/i2c.h> #include <linux/log2.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_gpio.h> #include <linux/pm.h> #include <linux/regulator/consumer.h> #include <linux/slab.h> @@ -28,6 +30,7 @@ #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 +931,57 @@ static const struct v4l2_subdev_internal_ops mt9p031_subdev_internal_ops = { * Driver initialization and probing */ +#if defined(CONFIG_OF) +static struct mt9p031_platform_data * + mt9p031_get_pdata(struct i2c_client *client) + +{ + if (client->dev.of_node) { + struct device_node *np; + struct mt9p031_platform_data *pdata; + + 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) { + dev_err(&client->dev, + "mt9p031 failed allocate memeory\n"); + return NULL; + } + pdata->reset = of_get_named_gpio(client->dev.of_node, + "gpio-reset", 0); + if (!gpio_is_valid(pdata->reset)) + pdata->reset = -1; + + if (of_property_read_u32(np, "input-clock-frequency", + &pdata->ext_freq)) + return NULL; + + if (of_property_read_u32(np, "pixel-clock-frequency", + &pdata->target_freq)) + return NULL; + + return pdata; + } + + return client->dev.platform_data; +} +#else +static struct mt9p031_platform_data * + mt9p031_get_pdata(struct i2c_client *client) +{ + return client->dev.platform_data; +} +#endif + static int mt9p031_probe(struct i2c_client *client, const struct i2c_device_id *did) { - 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; @@ -1070,8 +1120,16 @@ static const struct i2c_device_id mt9p031_id[] = { }; MODULE_DEVICE_TABLE(i2c, mt9p031_id); +static const struct of_device_id mt9p031_of_match[] = { + { .compatible = "aptina,mt9p031", }, + { .compatible = "aptina,mt9p031m", }, + {}, +}; +MODULE_DEVICE_TABLE(of, mt9p031_of_match); + static struct i2c_driver mt9p031_i2c_driver = { .driver = { + .of_match_table = of_match_ptr(mt9p031_of_match), .name = "mt9p031", }, .probe = mt9p031_probe,