Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-11 Thread Sakari Ailus
Hi Laurent,

On Wed, Mar 11, 2015 at 08:09:11PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday 11 March 2015 13:04:43 Sakari Ailus wrote:
> > On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
> > > From: Benoit Parrot 
> > > 
> > > this patch adds support for omnivision's ov2659
> > > sensor, the driver supports following features:
> > > 1: Asynchronous probing
> > > 2: DT support
> > > 3: Media controller support
> > > 
> > > Signed-off-by: Benoit Parrot 
> > > Signed-off-by: Lad, Prabhakar 
> > > ---
> > > 
> > >  Sorry quick new version, I need to get this merged for next
> > >  merge window.
> > >  
> > >  Changes for v4:
> > >  1: Renamed target frequency property to 'link-frequencies'
> > > as per Sakari's suggestion.
> > >  
> > >  2: Changed the copyright to "GPL v2"
> > >  
> > >  v3: https://patchwork.kernel.org/patch/5959401/
> > >  v2: https://patchwork.kernel.org/patch/5859801/
> > >  v1: https://patchwork.linuxtv.org/patch/27919/
> > >  
> > >  .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
> > >  MAINTAINERS|   10 +
> > >  drivers/media/i2c/Kconfig  |   11 +
> > >  drivers/media/i2c/Makefile |1 +
> > >  drivers/media/i2c/ov2659.c | 1439 +++
> > >  include/media/ov2659.h |   33 +
> > >  6 files changed, 1532 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > >  create mode 100644 drivers/media/i2c/ov2659.c
> > >  create mode 100644 include/media/ov2659.h
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > > b/Documentation/devicetree/bindings/media/i2c/ov2659.txt new file mode
> > > 100644
> > > index 000..a655500
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > > @@ -0,0 +1,38 @@
> > > +* OV2659 1/5-Inch 2Mp SOC Camera
> > > +
> > > +The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size
> > > of +1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor
> > > supports +multiple resolutions output, such as UXGA, SVGA, 720p. It also
> > > can support +YUV422, RGB565/555 or raw RGB output formats.
> > > +
> > > +Required Properties:
> > > +- compatible: Must be "ovti,ov2659"
> > > +- reg: I2C slave address
> > > +- clocks: reference to the xvclk input clock.
> > > +- clock-names: should be "xvclk".
> > > +- link-frequencies: target pixel clock frequency.
> > > +
> > > +For further reading on port node refer to
> > > +Documentation/devicetree/bindings/media/video-interfaces.txt.
> > > +
> > > +Example:
> > > +
> > > + i2c0@1c22000 {
> > > + ...
> > > + ...
> > > +  ov2659@30 {
> > > + compatible = "ovti,ov2659";
> > > + reg = <0x30>;
> > > +
> > > + clocks = <&clk_ov2659 0>;
> > > + clock-names = "xvclk";
> > > +
> > > + port {
> > > + ov2659_0: endpoint {
> > > + remote-endpoint = <&vpfe_ep>;
> > > + link-frequencies = <7000>;
> > 
> > link-frequencies = /bits/ 64 <7000>;
> > 
> > > + };
> > > + };
> > > + };
> > > + ...
> > > + };
> 
> [snip]
> 
> > > new file mode 100644
> > > index 000..487cb19
> > > --- /dev/null
> > > +++ b/include/media/ov2659.h
> > > @@ -0,0 +1,33 @@
> > > +/*
> > > + * Omnivision OV2659 CMOS Image Sensor driver
> > > + *
> > > + * Copyright (C) 2015 Texas Instruments, Inc.
> > > + *
> > > + * Benoit Parrot 
> > > + * Lad, Prabhakar 
> > > + *
> > > + * This program is free software; you may redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; version 2 of the License.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > > + * SOFTWARE.
> > > + */
> > > +
> > > +#ifndef OV2659_H
> > > +#define OV2659_H
> > > +
> > > +/**
> > > + * struct ov2659_platform_data - ov2659 driver platform data
> > > + * @link_frequency: target pixel clock frequency
> > > + */
> > > +struct ov2659_platform_data {
> > > + unsigned int link_frequency;
> > 
> > Shouldn't you have xvclk_frequency here as well? In most cases you need to
> > set explicitly to a certain value in order to achieve the required link
> > frequency.

Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-11 Thread Laurent Pinchart
Hi Prabhakar,

On Wednesday 11 March 2015 19:40:13 Lad, Prabhakar wrote:
> On Wed, Mar 11, 2015 at 6:22 PM, Laurent Pinchart wrote:
> > On Tuesday 10 March 2015 03:35:57 Sakari Ailus wrote:
> >> On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
> >> ...
> >> 
> >> > +static struct ov2659_platform_data *
> >> > +ov2659_get_pdata(struct i2c_client *client)
> >> > +{
> >> > +   struct ov2659_platform_data *pdata;
> >> > +   struct device_node *endpoint;
> >> > +   int ret;
> >> > +
> >> > +   if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) {
> >> > +   dev_err(&client->dev, "ov2659_get_pdata: DT Node found\n");
> >> > +   return client->dev.platform_data;
> >> > +   }
> >> > +
> >> > +   endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
> >> > +   if (!endpoint)
> >> > +   return NULL;
> >> > +
> >> > +   pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> >> > +   if (!pdata)
> >> > +   goto done;
> >> > +
> >> > +   ret = of_property_read_u32(endpoint, "link-frequencies",
> >> > +  &pdata->link_frequency);
> >> 
> >> This is actually documented as being a 64-bit array.
> > 
> > The main purpose of link-frequencies is to restrict the link frequencies
> > acceptable in the system due to EMI requirements. One link frequency
> > should be selected in the array based on the desired format and frame
> > rate. This is usually done by exposing the frequency to userspace through
> > a writable V4L2_CID_LINK_FREQ control, and exposing the resulting pixel
> > rate as a read- only V4L2_CID_PIXEL_RATE control.
> > 
> > V4L2_CID_PIXEL_RATE is mandatory to use the sensor with several drivers
> > (including omap3isp and omap4iss), so it should really be implemented.
> 
> Even if the sensor supports only one frequency the control needs to be
> implemented ?

The V4L2_CID_PIXEL_RATE must be implemented even for sensors supporting a 
single pixel rate, yes. The reason is that the receiver needs to know the 
sensor pixel rate in order to configure its clocking.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-11 Thread Lad, Prabhakar
Hi Laurent,

Thanks for the review.

On Wed, Mar 11, 2015 at 6:22 PM, Laurent Pinchart
 wrote:
> On Tuesday 10 March 2015 03:35:57 Sakari Ailus wrote:
>> On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
>> ...
>>
>> > +static struct ov2659_platform_data *
>> > +ov2659_get_pdata(struct i2c_client *client)
>> > +{
>> > +   struct ov2659_platform_data *pdata;
>> > +   struct device_node *endpoint;
>> > +   int ret;
>> > +
>> > +   if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) {
>> > +   dev_err(&client->dev, "ov2659_get_pdata: DT Node found\n");
>> > +   return client->dev.platform_data;
>> > +   }
>> > +
>> > +   endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
>> > +   if (!endpoint)
>> > +   return NULL;
>> > +
>> > +   pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
>> > +   if (!pdata)
>> > +   goto done;
>> > +
>> > +   ret = of_property_read_u32(endpoint, "link-frequencies",
>> > +  &pdata->link_frequency);
>>
>> This is actually documented as being a 64-bit array.
>
> The main purpose of link-frequencies is to restrict the link frequencies
> acceptable in the system due to EMI requirements. One link frequency should be
> selected in the array based on the desired format and frame rate. This is
> usually done by exposing the frequency to userspace through a writable
> V4L2_CID_LINK_FREQ control, and exposing the resulting pixel rate as a read-
> only V4L2_CID_PIXEL_RATE control.
>
> V4L2_CID_PIXEL_RATE is mandatory to use the sensor with several drivers
> (including omap3isp and omap4iss), so it should really be implemented.
>
Even if the sensor supports only one frequency the control needs to be
implemented ?

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-11 Thread Laurent Pinchart
On Tuesday 10 March 2015 03:35:57 Sakari Ailus wrote:
> On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
> ...
> 
> > +static struct ov2659_platform_data *
> > +ov2659_get_pdata(struct i2c_client *client)
> > +{
> > +   struct ov2659_platform_data *pdata;
> > +   struct device_node *endpoint;
> > +   int ret;
> > +
> > +   if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) {
> > +   dev_err(&client->dev, "ov2659_get_pdata: DT Node found\n");
> > +   return client->dev.platform_data;
> > +   }
> > +
> > +   endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
> > +   if (!endpoint)
> > +   return NULL;
> > +
> > +   pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> > +   if (!pdata)
> > +   goto done;
> > +
> > +   ret = of_property_read_u32(endpoint, "link-frequencies",
> > +  &pdata->link_frequency);
> 
> This is actually documented as being a 64-bit array.

The main purpose of link-frequencies is to restrict the link frequencies 
acceptable in the system due to EMI requirements. One link frequency should be 
selected in the array based on the desired format and frame rate. This is 
usually done by exposing the frequency to userspace through a writable 
V4L2_CID_LINK_FREQ control, and exposing the resulting pixel rate as a read-
only V4L2_CID_PIXEL_RATE control.

V4L2_CID_PIXEL_RATE is mandatory to use the sensor with several drivers 
(including omap3isp and omap4iss), so it should really be implemented.

> The smiapp wasn't even reading it from the endpoint node. Oh well...

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-11 Thread Laurent Pinchart
Hi Sakari,

On Wednesday 11 March 2015 13:04:43 Sakari Ailus wrote:
> On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
> > From: Benoit Parrot 
> > 
> > this patch adds support for omnivision's ov2659
> > sensor, the driver supports following features:
> > 1: Asynchronous probing
> > 2: DT support
> > 3: Media controller support
> > 
> > Signed-off-by: Benoit Parrot 
> > Signed-off-by: Lad, Prabhakar 
> > ---
> > 
> >  Sorry quick new version, I need to get this merged for next
> >  merge window.
> >  
> >  Changes for v4:
> >  1: Renamed target frequency property to 'link-frequencies'
> > as per Sakari's suggestion.
> >  
> >  2: Changed the copyright to "GPL v2"
> >  
> >  v3: https://patchwork.kernel.org/patch/5959401/
> >  v2: https://patchwork.kernel.org/patch/5859801/
> >  v1: https://patchwork.linuxtv.org/patch/27919/
> >  
> >  .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
> >  MAINTAINERS|   10 +
> >  drivers/media/i2c/Kconfig  |   11 +
> >  drivers/media/i2c/Makefile |1 +
> >  drivers/media/i2c/ov2659.c | 1439 +++
> >  include/media/ov2659.h |   33 +
> >  6 files changed, 1532 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
> >  create mode 100644 drivers/media/i2c/ov2659.c
> >  create mode 100644 include/media/ov2659.h
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > b/Documentation/devicetree/bindings/media/i2c/ov2659.txt new file mode
> > 100644
> > index 000..a655500
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > @@ -0,0 +1,38 @@
> > +* OV2659 1/5-Inch 2Mp SOC Camera
> > +
> > +The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size
> > of +1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor
> > supports +multiple resolutions output, such as UXGA, SVGA, 720p. It also
> > can support +YUV422, RGB565/555 or raw RGB output formats.
> > +
> > +Required Properties:
> > +- compatible: Must be "ovti,ov2659"
> > +- reg: I2C slave address
> > +- clocks: reference to the xvclk input clock.
> > +- clock-names: should be "xvclk".
> > +- link-frequencies: target pixel clock frequency.
> > +
> > +For further reading on port node refer to
> > +Documentation/devicetree/bindings/media/video-interfaces.txt.
> > +
> > +Example:
> > +
> > +   i2c0@1c22000 {
> > +   ...
> > +   ...
> > +ov2659@30 {
> > +   compatible = "ovti,ov2659";
> > +   reg = <0x30>;
> > +
> > +   clocks = <&clk_ov2659 0>;
> > +   clock-names = "xvclk";
> > +
> > +   port {
> > +   ov2659_0: endpoint {
> > +   remote-endpoint = <&vpfe_ep>;
> > +   link-frequencies = <7000>;
> 
> link-frequencies = /bits/ 64 <7000>;
> 
> > +   };
> > +   };
> > +   };
> > +   ...
> > +   };

[snip]

> > new file mode 100644
> > index 000..487cb19
> > --- /dev/null
> > +++ b/include/media/ov2659.h
> > @@ -0,0 +1,33 @@
> > +/*
> > + * Omnivision OV2659 CMOS Image Sensor driver
> > + *
> > + * Copyright (C) 2015 Texas Instruments, Inc.
> > + *
> > + * Benoit Parrot 
> > + * Lad, Prabhakar 
> > + *
> > + * This program is free software; you may redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; version 2 of the License.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#ifndef OV2659_H
> > +#define OV2659_H
> > +
> > +/**
> > + * struct ov2659_platform_data - ov2659 driver platform data
> > + * @link_frequency: target pixel clock frequency
> > + */
> > +struct ov2659_platform_data {
> > +   unsigned int link_frequency;
> 
> Shouldn't you have xvclk_frequency here as well? In most cases you need to
> set explicitly to a certain value in order to achieve the required link
> frequency.

I'm not sure about that. In the DT case setting the xvclk clock frequency is 
done outside of the driver (through assigned-clock-rates for instance). 
Wouldn't it make sense to apply the same logic for non-DT platforms and let 
the platform configure the clock ?

> > +};
> > +#endif /* OV2659_H */

-- 

Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-11 Thread Lad, Prabhakar
Hi Sakari,

Thanks for the review.

On Wed, Mar 11, 2015 at 11:04 AM, Sakari Ailus  wrote:
> Hi Prabhakar,
>
> On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
>> From: Benoit Parrot 
>>
>> this patch adds support for omnivision's ov2659
>> sensor, the driver supports following features:
>> 1: Asynchronous probing
>> 2: DT support
>> 3: Media controller support
>>
>> Signed-off-by: Benoit Parrot 
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>  Sorry quick new version, I need to get this merged for next
>>  merge window.
>>
>>  Changes for v4:
>>  1: Renamed target frequency property to 'link-frequencies'
>> as per Sakari's suggestion.
>>  2: Changed the copyright to "GPL v2"
>>
>>  v3: https://patchwork.kernel.org/patch/5959401/
>>  v2: https://patchwork.kernel.org/patch/5859801/
>>  v1: https://patchwork.linuxtv.org/patch/27919/
>>
>>  .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
>>  MAINTAINERS|   10 +
>>  drivers/media/i2c/Kconfig  |   11 +
>>  drivers/media/i2c/Makefile |1 +
>>  drivers/media/i2c/ov2659.c | 1439 
>> 
>>  include/media/ov2659.h |   33 +
>>  6 files changed, 1532 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
>>  create mode 100644 drivers/media/i2c/ov2659.c
>>  create mode 100644 include/media/ov2659.h
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt 
>> b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
>> new file mode 100644
>> index 000..a655500
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
>> @@ -0,0 +1,38 @@
>> +* OV2659 1/5-Inch 2Mp SOC Camera
>> +
>> +The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size of
>> +1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor supports
>> +multiple resolutions output, such as UXGA, SVGA, 720p. It also can support
>> +YUV422, RGB565/555 or raw RGB output formats.
>> +
>> +Required Properties:
>> +- compatible: Must be "ovti,ov2659"
>> +- reg: I2C slave address
>> +- clocks: reference to the xvclk input clock.
>> +- clock-names: should be "xvclk".
>> +- link-frequencies: target pixel clock frequency.
>> +
>> +For further reading on port node refer to
>> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> +
>> +Example:
>> +
>> + i2c0@1c22000 {
>> + ...
>> + ...
>> +  ov2659@30 {
>> + compatible = "ovti,ov2659";
>> + reg = <0x30>;
>> +
>> + clocks = <&clk_ov2659 0>;
>> + clock-names = "xvclk";
>> +
>> + port {
>> + ov2659_0: endpoint {
>> + remote-endpoint = <&vpfe_ep>;
>> + link-frequencies = <7000>;
>
> link-frequencies = /bits/ 64 <7000>;
>
OK

>> + };
>> + };
>> + };
>> + ...
>> + };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index ddc5a8c..4006cc8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8910,6 +8910,16 @@ T: git 
>> git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
>>  S:   Maintained
>>  F:   drivers/media/platform/am437x/
>>
>> +OV2659 OMNIVISION SENSOR DRIVER
>> +M:   Lad, Prabhakar 
>> +L:   linux-me...@vger.kernel.org
>> +W:   http://linuxtv.org/
>> +Q:   http://patchwork.linuxtv.org/project/linux-media/list/
>> +T:   git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
>> +S:   Maintained
>> +F:   drivers/media/i2c/ov2659.c
>> +F:   include/media/ov2659.h
>> +
>>  SIS 190 ETHERNET DRIVER
>>  M:   Francois Romieu 
>>  L:   net...@vger.kernel.org
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index da58c9b..6f30ea7 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -466,6 +466,17 @@ config VIDEO_APTINA_PLL
>>  config VIDEO_SMIAPP_PLL
>>   tristate
>>
>> +config VIDEO_OV2659
>> + tristate "OmniVision OV2659 sensor support"
>> + depends on VIDEO_V4L2 && I2C
>> + depends on MEDIA_CAMERA_SUPPORT
>> + ---help---
>> +   This is a Video4Linux2 sensor-level driver for the OmniVision
>> +   OV2659 camera.
>> +
>> +   To compile this driver as a module, choose M here: the
>> +   module will be called ov2659.
>> +
>>  config VIDEO_OV7640
>>   tristate "OmniVision OV7640 sensor support"
>>   depends on I2C && VIDEO_V4L2
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 98589001..f165fae 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -77,3 +77,4 @@ obj-$(CONFIG_VIDEO_SMIAPP_PLL)  += smiapp-pll.o
>>  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
>>  obj-$(CONFIG_VIDEO_IR_I2C)  +

Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-11 Thread Sakari Ailus
Hi Prabhakar,

On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
> From: Benoit Parrot 
> 
> this patch adds support for omnivision's ov2659
> sensor, the driver supports following features:
> 1: Asynchronous probing
> 2: DT support
> 3: Media controller support
> 
> Signed-off-by: Benoit Parrot 
> Signed-off-by: Lad, Prabhakar 
> ---
>  Sorry quick new version, I need to get this merged for next
>  merge window.
> 
>  Changes for v4:
>  1: Renamed target frequency property to 'link-frequencies'
> as per Sakari's suggestion.
>  2: Changed the copyright to "GPL v2"
> 
>  v3: https://patchwork.kernel.org/patch/5959401/
>  v2: https://patchwork.kernel.org/patch/5859801/
>  v1: https://patchwork.linuxtv.org/patch/27919/
>  
>  .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
>  MAINTAINERS|   10 +
>  drivers/media/i2c/Kconfig  |   11 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/ov2659.c | 1439 
> 
>  include/media/ov2659.h |   33 +
>  6 files changed, 1532 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
>  create mode 100644 drivers/media/i2c/ov2659.c
>  create mode 100644 include/media/ov2659.h
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt 
> b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> new file mode 100644
> index 000..a655500
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> @@ -0,0 +1,38 @@
> +* OV2659 1/5-Inch 2Mp SOC Camera
> +
> +The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size of
> +1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor supports
> +multiple resolutions output, such as UXGA, SVGA, 720p. It also can support
> +YUV422, RGB565/555 or raw RGB output formats.
> +
> +Required Properties:
> +- compatible: Must be "ovti,ov2659"
> +- reg: I2C slave address
> +- clocks: reference to the xvclk input clock.
> +- clock-names: should be "xvclk".
> +- link-frequencies: target pixel clock frequency.
> +
> +For further reading on port node refer to
> +Documentation/devicetree/bindings/media/video-interfaces.txt.
> +
> +Example:
> +
> + i2c0@1c22000 {
> + ...
> + ...
> +  ov2659@30 {
> + compatible = "ovti,ov2659";
> + reg = <0x30>;
> +
> + clocks = <&clk_ov2659 0>;
> + clock-names = "xvclk";
> +
> + port {
> + ov2659_0: endpoint {
> + remote-endpoint = <&vpfe_ep>;
> + link-frequencies = <7000>;

link-frequencies = /bits/ 64 <7000>;

> + };
> + };
> + };
> + ...
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ddc5a8c..4006cc8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8910,6 +8910,16 @@ T: git 
> git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
>  S:   Maintained
>  F:   drivers/media/platform/am437x/
>  
> +OV2659 OMNIVISION SENSOR DRIVER
> +M:   Lad, Prabhakar 
> +L:   linux-me...@vger.kernel.org
> +W:   http://linuxtv.org/
> +Q:   http://patchwork.linuxtv.org/project/linux-media/list/
> +T:   git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
> +S:   Maintained
> +F:   drivers/media/i2c/ov2659.c
> +F:   include/media/ov2659.h
> +
>  SIS 190 ETHERNET DRIVER
>  M:   Francois Romieu 
>  L:   net...@vger.kernel.org
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index da58c9b..6f30ea7 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -466,6 +466,17 @@ config VIDEO_APTINA_PLL
>  config VIDEO_SMIAPP_PLL
>   tristate
>  
> +config VIDEO_OV2659
> + tristate "OmniVision OV2659 sensor support"
> + depends on VIDEO_V4L2 && I2C
> + depends on MEDIA_CAMERA_SUPPORT
> + ---help---
> +   This is a Video4Linux2 sensor-level driver for the OmniVision
> +   OV2659 camera.
> +
> +   To compile this driver as a module, choose M here: the
> +   module will be called ov2659.
> +
>  config VIDEO_OV7640
>   tristate "OmniVision OV7640 sensor support"
>   depends on I2C && VIDEO_V4L2
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 98589001..f165fae 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -77,3 +77,4 @@ obj-$(CONFIG_VIDEO_SMIAPP_PLL)  += smiapp-pll.o
>  obj-$(CONFIG_VIDEO_AK881X)   += ak881x.o
>  obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
> +obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
> diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
> new file mode 100644
> index 000.

Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-09 Thread Sakari Ailus
Hi Prabhakar,

On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
> From: Benoit Parrot 
> 
> this patch adds support for omnivision's ov2659
> sensor, the driver supports following features:
> 1: Asynchronous probing
> 2: DT support
> 3: Media controller support
> 
> Signed-off-by: Benoit Parrot 
> Signed-off-by: Lad, Prabhakar 

Now that DT support is included, could you document it as well? There's a
single proprerty to document.

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-09 Thread Sakari Ailus
Hi Prabhakar,

On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
...
> +static struct ov2659_platform_data *
> +ov2659_get_pdata(struct i2c_client *client)
> +{
> + struct ov2659_platform_data *pdata;
> + struct device_node *endpoint;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) {
> + dev_err(&client->dev, "ov2659_get_pdata: DT Node found\n");
> + return client->dev.platform_data;
> + }
> +
> + endpoint = of_graph_get_next_endpoint(client->dev.of_node, NULL);
> + if (!endpoint)
> + return NULL;
> +
> + pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + goto done;
> +
> + ret = of_property_read_u32(endpoint, "link-frequencies",
> +&pdata->link_frequency);

This is actually documented as being a 64-bit array.

The smiapp wasn't even reading it from the endpoint node. Oh well...

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

2015-03-08 Thread Lad Prabhakar
From: Benoit Parrot 

this patch adds support for omnivision's ov2659
sensor, the driver supports following features:
1: Asynchronous probing
2: DT support
3: Media controller support

Signed-off-by: Benoit Parrot 
Signed-off-by: Lad, Prabhakar 
---
 Sorry quick new version, I need to get this merged for next
 merge window.

 Changes for v4:
 1: Renamed target frequency property to 'link-frequencies'
as per Sakari's suggestion.
 2: Changed the copyright to "GPL v2"

 v3: https://patchwork.kernel.org/patch/5959401/
 v2: https://patchwork.kernel.org/patch/5859801/
 v1: https://patchwork.linuxtv.org/patch/27919/
 
 .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
 MAINTAINERS|   10 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2659.c | 1439 
 include/media/ov2659.h |   33 +
 6 files changed, 1532 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
 create mode 100644 drivers/media/i2c/ov2659.c
 create mode 100644 include/media/ov2659.h

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
new file mode 100644
index 000..a655500
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
@@ -0,0 +1,38 @@
+* OV2659 1/5-Inch 2Mp SOC Camera
+
+The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size of
+1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor supports
+multiple resolutions output, such as UXGA, SVGA, 720p. It also can support
+YUV422, RGB565/555 or raw RGB output formats.
+
+Required Properties:
+- compatible: Must be "ovti,ov2659"
+- reg: I2C slave address
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+- link-frequencies: target pixel clock frequency.
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+   i2c0@1c22000 {
+   ...
+   ...
+ov2659@30 {
+   compatible = "ovti,ov2659";
+   reg = <0x30>;
+
+   clocks = <&clk_ov2659 0>;
+   clock-names = "xvclk";
+
+   port {
+   ov2659_0: endpoint {
+   remote-endpoint = <&vpfe_ep>;
+   link-frequencies = <7000>;
+   };
+   };
+   };
+   ...
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index ddc5a8c..4006cc8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8910,6 +8910,16 @@ T:   git 
git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
 S: Maintained
 F: drivers/media/platform/am437x/
 
+OV2659 OMNIVISION SENSOR DRIVER
+M: Lad, Prabhakar 
+L: linux-me...@vger.kernel.org
+W: http://linuxtv.org/
+Q: http://patchwork.linuxtv.org/project/linux-media/list/
+T: git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
+S: Maintained
+F: drivers/media/i2c/ov2659.c
+F: include/media/ov2659.h
+
 SIS 190 ETHERNET DRIVER
 M: Francois Romieu 
 L: net...@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index da58c9b..6f30ea7 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -466,6 +466,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_OV2659
+   tristate "OmniVision OV2659 sensor support"
+   depends on VIDEO_V4L2 && I2C
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2659 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2659.
+
 config VIDEO_OV7640
tristate "OmniVision OV7640 sensor support"
depends on I2C && VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 98589001..f165fae 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -77,3 +77,4 @@ obj-$(CONFIG_VIDEO_SMIAPP_PLL)+= smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
+obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
new file mode 100644
index 000..eca96b7
--- /dev/null
+++ b/drivers/media/i2c/ov2659.c
@@ -0,0 +1,1439 @@
+/*
+ * Omnivision OV2659 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2015 Texas Instruments, Inc.
+ *
+ * Benoit Parrot 
+ * Lad, Prabhakar 
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under th