[PATCH] fbdev: goldfishfb: use devres api

2015-02-20 Thread Prabhakar Lad
From: "Lad, Prabhakar" 

this patch does the following:
a> uses devm_kzalloc() instead of kzalloc and cleanup the error path
b> uses devm_ioremap() instead of ioremap and cleanup the error path
c> uses devm_request_irq() instead of request_irq and cleanup the error path

Signed-off-by: Lad, Prabhakar 
---
 Note: This patch is compile tested only and applies on linux-next.

 drivers/video/fbdev/goldfishfb.c | 61 +++-
 1 file changed, 22 insertions(+), 39 deletions(-)

diff --git a/drivers/video/fbdev/goldfishfb.c b/drivers/video/fbdev/goldfishfb.c
index 7f6c9e6..841514d 100644
--- a/drivers/video/fbdev/goldfishfb.c
+++ b/drivers/video/fbdev/goldfishfb.c
@@ -188,31 +188,25 @@ static int goldfish_fb_probe(struct platform_device *pdev)
u32 width, height;
dma_addr_t fbpaddr;
 
-   fb = kzalloc(sizeof(*fb), GFP_KERNEL);
-   if (fb == NULL) {
-   ret = -ENOMEM;
-   goto err_fb_alloc_failed;
-   }
+   fb = devm_kzalloc(>dev, sizeof(*fb), GFP_KERNEL);
+   if (fb == NULL)
+   return -ENOMEM;
+
spin_lock_init(>lock);
init_waitqueue_head(>wait);
platform_set_drvdata(pdev, fb);
 
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (r == NULL) {
-   ret = -ENODEV;
-   goto err_no_io_base;
-   }
-   fb->reg_base = ioremap(r->start, PAGE_SIZE);
-   if (fb->reg_base == NULL) {
-   ret = -ENOMEM;
-   goto err_no_io_base;
-   }
+   if (r == NULL)
+   return -ENODEV;
+
+   fb->reg_base = devm_ioremap(>dev, r->start, PAGE_SIZE);
+   if (fb->reg_base == NULL)
+   return -ENOMEM;
 
fb->irq = platform_get_irq(pdev, 0);
-   if (fb->irq <= 0) {
-   ret = -ENODEV;
-   goto err_no_irq;
-   }
+   if (fb->irq <= 0)
+   return -ENODEV;
 
width = readl(fb->reg_base + FB_GET_WIDTH);
height = readl(fb->reg_base + FB_GET_HEIGHT);
@@ -249,43 +243,34 @@ static int goldfish_fb_probe(struct platform_device *pdev)
, GFP_KERNEL);
pr_debug("allocating frame buffer %d * %d, got %p\n",
width, height, fb->fb.screen_base);
-   if (fb->fb.screen_base == NULL) {
-   ret = -ENOMEM;
-   goto err_alloc_screen_base_failed;
-   }
+   if (fb->fb.screen_base == NULL)
+   return -ENOMEM;
+
fb->fb.fix.smem_start = fbpaddr;
fb->fb.fix.smem_len = framesize;
 
ret = fb_set_var(>fb, >fb.var);
if (ret)
-   goto err_fb_set_var_failed;
+   goto error;
 
-   ret = request_irq(fb->irq, goldfish_fb_interrupt, IRQF_SHARED,
-   pdev->name, fb);
+   ret = devm_request_irq(>dev, fb->irq, goldfish_fb_interrupt,
+  IRQF_SHARED, pdev->name, fb);
if (ret)
-   goto err_request_irq_failed;
+   goto error;
 
writel(FB_INT_BASE_UPDATE_DONE, fb->reg_base + FB_INT_ENABLE);
goldfish_fb_pan_display(>fb.var, >fb); /* updates base */
 
ret = register_framebuffer(>fb);
if (ret)
-   goto err_register_framebuffer_failed;
+   goto error;
+
return 0;
 
-err_register_framebuffer_failed:
-   free_irq(fb->irq, fb);
-err_request_irq_failed:
-err_fb_set_var_failed:
+error:
dma_free_coherent(>dev, framesize,
(void *)fb->fb.screen_base,
fb->fb.fix.smem_start);
-err_alloc_screen_base_failed:
-err_no_irq:
-   iounmap(fb->reg_base);
-err_no_io_base:
-   kfree(fb);
-err_fb_alloc_failed:
return ret;
 }
 
@@ -296,11 +281,9 @@ static int goldfish_fb_remove(struct platform_device *pdev)
 
framesize = fb->fb.var.xres_virtual * fb->fb.var.yres_virtual * 2;
unregister_framebuffer(>fb);
-   free_irq(fb->irq, fb);
 
dma_free_coherent(>dev, framesize, (void *)fb->fb.screen_base,
fb->fb.fix.smem_start);
-   iounmap(fb->reg_base);
return 0;
 }
 
-- 
1.9.1

--
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] fbdev: goldfishfb: use devres api

2015-02-20 Thread Prabhakar Lad
From: Lad, Prabhakar prabhakar.cse...@gmail.com

this patch does the following:
a uses devm_kzalloc() instead of kzalloc and cleanup the error path
b uses devm_ioremap() instead of ioremap and cleanup the error path
c uses devm_request_irq() instead of request_irq and cleanup the error path

Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 Note: This patch is compile tested only and applies on linux-next.

 drivers/video/fbdev/goldfishfb.c | 61 +++-
 1 file changed, 22 insertions(+), 39 deletions(-)

diff --git a/drivers/video/fbdev/goldfishfb.c b/drivers/video/fbdev/goldfishfb.c
index 7f6c9e6..841514d 100644
--- a/drivers/video/fbdev/goldfishfb.c
+++ b/drivers/video/fbdev/goldfishfb.c
@@ -188,31 +188,25 @@ static int goldfish_fb_probe(struct platform_device *pdev)
u32 width, height;
dma_addr_t fbpaddr;
 
-   fb = kzalloc(sizeof(*fb), GFP_KERNEL);
-   if (fb == NULL) {
-   ret = -ENOMEM;
-   goto err_fb_alloc_failed;
-   }
+   fb = devm_kzalloc(pdev-dev, sizeof(*fb), GFP_KERNEL);
+   if (fb == NULL)
+   return -ENOMEM;
+
spin_lock_init(fb-lock);
init_waitqueue_head(fb-wait);
platform_set_drvdata(pdev, fb);
 
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-   if (r == NULL) {
-   ret = -ENODEV;
-   goto err_no_io_base;
-   }
-   fb-reg_base = ioremap(r-start, PAGE_SIZE);
-   if (fb-reg_base == NULL) {
-   ret = -ENOMEM;
-   goto err_no_io_base;
-   }
+   if (r == NULL)
+   return -ENODEV;
+
+   fb-reg_base = devm_ioremap(pdev-dev, r-start, PAGE_SIZE);
+   if (fb-reg_base == NULL)
+   return -ENOMEM;
 
fb-irq = platform_get_irq(pdev, 0);
-   if (fb-irq = 0) {
-   ret = -ENODEV;
-   goto err_no_irq;
-   }
+   if (fb-irq = 0)
+   return -ENODEV;
 
width = readl(fb-reg_base + FB_GET_WIDTH);
height = readl(fb-reg_base + FB_GET_HEIGHT);
@@ -249,43 +243,34 @@ static int goldfish_fb_probe(struct platform_device *pdev)
fbpaddr, GFP_KERNEL);
pr_debug(allocating frame buffer %d * %d, got %p\n,
width, height, fb-fb.screen_base);
-   if (fb-fb.screen_base == NULL) {
-   ret = -ENOMEM;
-   goto err_alloc_screen_base_failed;
-   }
+   if (fb-fb.screen_base == NULL)
+   return -ENOMEM;
+
fb-fb.fix.smem_start = fbpaddr;
fb-fb.fix.smem_len = framesize;
 
ret = fb_set_var(fb-fb, fb-fb.var);
if (ret)
-   goto err_fb_set_var_failed;
+   goto error;
 
-   ret = request_irq(fb-irq, goldfish_fb_interrupt, IRQF_SHARED,
-   pdev-name, fb);
+   ret = devm_request_irq(pdev-dev, fb-irq, goldfish_fb_interrupt,
+  IRQF_SHARED, pdev-name, fb);
if (ret)
-   goto err_request_irq_failed;
+   goto error;
 
writel(FB_INT_BASE_UPDATE_DONE, fb-reg_base + FB_INT_ENABLE);
goldfish_fb_pan_display(fb-fb.var, fb-fb); /* updates base */
 
ret = register_framebuffer(fb-fb);
if (ret)
-   goto err_register_framebuffer_failed;
+   goto error;
+
return 0;
 
-err_register_framebuffer_failed:
-   free_irq(fb-irq, fb);
-err_request_irq_failed:
-err_fb_set_var_failed:
+error:
dma_free_coherent(pdev-dev, framesize,
(void *)fb-fb.screen_base,
fb-fb.fix.smem_start);
-err_alloc_screen_base_failed:
-err_no_irq:
-   iounmap(fb-reg_base);
-err_no_io_base:
-   kfree(fb);
-err_fb_alloc_failed:
return ret;
 }
 
@@ -296,11 +281,9 @@ static int goldfish_fb_remove(struct platform_device *pdev)
 
framesize = fb-fb.var.xres_virtual * fb-fb.var.yres_virtual * 2;
unregister_framebuffer(fb-fb);
-   free_irq(fb-irq, fb);
 
dma_free_coherent(pdev-dev, framesize, (void *)fb-fb.screen_base,
fb-fb.fix.smem_start);
-   iounmap(fb-reg_base);
return 0;
 }
 
-- 
1.9.1

--
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 2/2] fbdev: ssd1307fb: check return value while setting offset

2015-01-13 Thread Prabhakar Lad
On Tue, Jan 13, 2015 at 1:39 PM, Maxime Ripard
 wrote:
> On Tue, Jan 13, 2015 at 11:53:01AM +0000, Prabhakar Lad wrote:
>> On Tue, Jan 13, 2015 at 11:43 AM, Tomi Valkeinen  
>> wrote:
>> > On 08/01/15 10:17, Lad, Prabhakar wrote:
>> >> this patch checks the return value of write command while
>> >> setting the display offset.
>> >>
>> >> Signed-off-by: Lad, Prabhakar 
>> >> ---
>> >>  drivers/video/fbdev/ssd1307fb.c | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/video/fbdev/ssd1307fb.c 
>> >> b/drivers/video/fbdev/ssd1307fb.c
>> >> index 70e3ce8..a7ecaa6 100644
>> >> --- a/drivers/video/fbdev/ssd1307fb.c
>> >> +++ b/drivers/video/fbdev/ssd1307fb.c
>> >> @@ -342,7 +342,7 @@ static int ssd1307fb_ssd1306_init(struct 
>> >> ssd1307fb_par *par)
>> >>
>> >>   /* set display offset value */
>> >>   ret = ssd1307fb_write_cmd(par->client, 
>> >> SSD1307FB_SET_DISPLAY_OFFSET);
>> >> - ret = ssd1307fb_write_cmd(par->client, 0x20);
>> >> + ret = ret & ssd1307fb_write_cmd(par->client, 0x20);
>> >>   if (ret < 0)
>> >>   return ret;
>> >
>> > Hrm, what's that supposed to do? If both calls to ssd1307fb_write_cmd()
>> > return an error, they are anded, resulting in a garbage error code...
>> >
>> Agreed I have just aligned this to rest of the code in this function.
>>
>> Maxime if you are OK I'll post a patch returning error then and there ?
>
> I'm not sure what the question is, but Tomi is right, having two
> returns seems the right thing to do.
>
OK I'll post a new patch having two separate returns.

Thanks,
--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 2/2] fbdev: ssd1307fb: check return value while setting offset

2015-01-13 Thread Prabhakar Lad
On Tue, Jan 13, 2015 at 11:43 AM, Tomi Valkeinen  wrote:
> On 08/01/15 10:17, Lad, Prabhakar wrote:
>> this patch checks the return value of write command while
>> setting the display offset.
>>
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>  drivers/video/fbdev/ssd1307fb.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/ssd1307fb.c 
>> b/drivers/video/fbdev/ssd1307fb.c
>> index 70e3ce8..a7ecaa6 100644
>> --- a/drivers/video/fbdev/ssd1307fb.c
>> +++ b/drivers/video/fbdev/ssd1307fb.c
>> @@ -342,7 +342,7 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par 
>> *par)
>>
>>   /* set display offset value */
>>   ret = ssd1307fb_write_cmd(par->client, SSD1307FB_SET_DISPLAY_OFFSET);
>> - ret = ssd1307fb_write_cmd(par->client, 0x20);
>> + ret = ret & ssd1307fb_write_cmd(par->client, 0x20);
>>   if (ret < 0)
>>   return ret;
>
> Hrm, what's that supposed to do? If both calls to ssd1307fb_write_cmd()
> return an error, they are anded, resulting in a garbage error code...
>
Agreed I have just aligned this to rest of the code in this function.

Maxime if you are OK I'll post a patch returning error then and there ?

Thanks,
--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 2/2] fbdev: ssd1307fb: check return value while setting offset

2015-01-13 Thread Prabhakar Lad
On Tue, Jan 13, 2015 at 11:43 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On 08/01/15 10:17, Lad, Prabhakar wrote:
 this patch checks the return value of write command while
 setting the display offset.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  drivers/video/fbdev/ssd1307fb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/video/fbdev/ssd1307fb.c 
 b/drivers/video/fbdev/ssd1307fb.c
 index 70e3ce8..a7ecaa6 100644
 --- a/drivers/video/fbdev/ssd1307fb.c
 +++ b/drivers/video/fbdev/ssd1307fb.c
 @@ -342,7 +342,7 @@ static int ssd1307fb_ssd1306_init(struct ssd1307fb_par 
 *par)

   /* set display offset value */
   ret = ssd1307fb_write_cmd(par-client, SSD1307FB_SET_DISPLAY_OFFSET);
 - ret = ssd1307fb_write_cmd(par-client, 0x20);
 + ret = ret  ssd1307fb_write_cmd(par-client, 0x20);
   if (ret  0)
   return ret;

 Hrm, what's that supposed to do? If both calls to ssd1307fb_write_cmd()
 return an error, they are anded, resulting in a garbage error code...

Agreed I have just aligned this to rest of the code in this function.

Maxime if you are OK I'll post a patch returning error then and there ?

Thanks,
--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 2/2] fbdev: ssd1307fb: check return value while setting offset

2015-01-13 Thread Prabhakar Lad
On Tue, Jan 13, 2015 at 1:39 PM, Maxime Ripard
maxime.rip...@free-electrons.com wrote:
 On Tue, Jan 13, 2015 at 11:53:01AM +, Prabhakar Lad wrote:
 On Tue, Jan 13, 2015 at 11:43 AM, Tomi Valkeinen tomi.valkei...@ti.com 
 wrote:
  On 08/01/15 10:17, Lad, Prabhakar wrote:
  this patch checks the return value of write command while
  setting the display offset.
 
  Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
  ---
   drivers/video/fbdev/ssd1307fb.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)
 
  diff --git a/drivers/video/fbdev/ssd1307fb.c 
  b/drivers/video/fbdev/ssd1307fb.c
  index 70e3ce8..a7ecaa6 100644
  --- a/drivers/video/fbdev/ssd1307fb.c
  +++ b/drivers/video/fbdev/ssd1307fb.c
  @@ -342,7 +342,7 @@ static int ssd1307fb_ssd1306_init(struct 
  ssd1307fb_par *par)
 
/* set display offset value */
ret = ssd1307fb_write_cmd(par-client, 
  SSD1307FB_SET_DISPLAY_OFFSET);
  - ret = ssd1307fb_write_cmd(par-client, 0x20);
  + ret = ret  ssd1307fb_write_cmd(par-client, 0x20);
if (ret  0)
return ret;
 
  Hrm, what's that supposed to do? If both calls to ssd1307fb_write_cmd()
  return an error, they are anded, resulting in a garbage error code...
 
 Agreed I have just aligned this to rest of the code in this function.

 Maxime if you are OK I'll post a patch returning error then and there ?

 I'm not sure what the question is, but Tomi is right, having two
 returns seems the right thing to do.

OK I'll post a new patch having two separate returns.

Thanks,
--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] media: ti-vpe: Use mem-to-mem ioctl helpers

2014-12-19 Thread Prabhakar Lad
Hi Hans,

On Fri, Dec 19, 2014 at 4:42 PM, Hans Verkuil  wrote:
> Hi Prabhakar,
>
> I haven't seen any movement with Nikhil's patches, so it is probably better if
> you just post a new version of this patch based on the latest media_tree.
>
I was about to do that, as I was not able to utilize my holidays
in proper way :D

Thanks,
--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] media: ti-vpe: Use mem-to-mem ioctl helpers

2014-12-19 Thread Prabhakar Lad
Hi Hans,

On Fri, Dec 19, 2014 at 4:42 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi Prabhakar,

 I haven't seen any movement with Nikhil's patches, so it is probably better if
 you just post a new version of this patch based on the latest media_tree.

I was about to do that, as I was not able to utilize my holidays
in proper way :D

Thanks,
--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] misc: ioc4: fix variable may be used uninitialized warning

2014-12-08 Thread Prabhakar Lad
On Mon, Dec 8, 2014 at 1:18 PM, Richard Leitner  wrote:
> On Mon, 08 Dec 2014 13:42:47 +0100
> Arnd Bergmann  wrote:
>
>> On Monday 08 December 2014 13:27:20 Richard Leitner wrote:
>> >
>> > As far as I can tell 'start' cannot really be used uninitialized
>> > here, but for the sanity of gcc output explicitly initialize it.
>> > Same goes for the 'end' variable.
>>
>> Prabhakar Lad also sent a patch for this already, which was lacking
>> a good patch description. Your patch does this slightly better but
>> still fails to explain how you concluded it was safe and you don't
>> really explain why you initialize the 'end' variable that we don't
>> even get a warning about.
>
> Oops, I'm sorry, I haven't seen the patch and the answers to it.
>
> According to the comments by Andrew a simplification of this code
> section would be nice. I think it should be possible to do this in a
> way that the initialize-to-zero won't be needed anymore.
>
> Prabhakar Lad, are you working on this already?
> If not I'll take a look at it.
>
Definitely you can go ahead, I am busy with other stuff.

Thanks,
--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] misc: ioc4: fix variable may be used uninitialized warning

2014-12-08 Thread Prabhakar Lad
On Mon, Dec 8, 2014 at 1:18 PM, Richard Leitner d...@g0hl1n.net wrote:
 On Mon, 08 Dec 2014 13:42:47 +0100
 Arnd Bergmann a...@arndb.de wrote:

 On Monday 08 December 2014 13:27:20 Richard Leitner wrote:
 
  As far as I can tell 'start' cannot really be used uninitialized
  here, but for the sanity of gcc output explicitly initialize it.
  Same goes for the 'end' variable.

 Prabhakar Lad also sent a patch for this already, which was lacking
 a good patch description. Your patch does this slightly better but
 still fails to explain how you concluded it was safe and you don't
 really explain why you initialize the 'end' variable that we don't
 even get a warning about.

 Oops, I'm sorry, I haven't seen the patch and the answers to it.

 According to the comments by Andrew a simplification of this code
 section would be nice. I think it should be possible to do this in a
 way that the initialize-to-zero won't be needed anymore.

 Prabhakar Lad, are you working on this already?
 If not I'll take a look at it.

Definitely you can go ahead, I am busy with other stuff.

Thanks,
--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 v3] media: platform: add VPFE capture driver support for AM437X

2014-12-05 Thread Prabhakar Lad
Hi Hans,

On Fri, Dec 5, 2014 at 12:24 PM, Hans Verkuil  wrote:
> Hi Prabhakar,
>
> Sorry, there are still a few items that need to be fixed.
> If you can make a v4 with these issues addressed, then I can still make a
> pull request, although it depends on Mauro whether it is still accepted for
> 3.19.
>
OK will post a v4 tonight fixing all the below issues.

FYI: Looking at the response of Mauro on 'soc-camera: 1st set for 3.19'
he wont accept it!

Thanks,
--Prabhakar Lad

> On 12/04/2014 12:12 AM, Lad, Prabhakar wrote:
>> From: Benoit Parrot 
>>
>> This patch adds Video Processing Front End (VPFE) driver for
>> AM437X family of devices
>> Driver supports the following:
>> - V4L2 API using MMAP buffer access based on videobuf2 api
>> - Asynchronous sensor/decoder sub device registration
>> - DT support
>
> Just to confirm: this driver only supports SDTV formats? No HDTV?
> I didn't see any VIDIOC_*_DV_TIMINGS support, so I assume it really
> isn't supported.
>
>>
>> Signed-off-by: Benoit Parrot 
>> Signed-off-by: Darren Etheridge 
>> Signed-off-by: Lad, Prabhakar 
>> ---
>
> 
>
>> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
>> b/drivers/media/platform/am437x/am437x-vpfe.c
>> new file mode 100644
>> index 000..25863e8
>> --- /dev/null
>> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
>
> 
>
>> +
>> +static int
>> +cmp_v4l2_format(const struct v4l2_format *lhs, const struct v4l2_format 
>> *rhs)
>> +{
>> + return lhs->type == rhs->type &&
>> + lhs->fmt.pix.width == rhs->fmt.pix.width &&
>> + lhs->fmt.pix.height == rhs->fmt.pix.height &&
>> + lhs->fmt.pix.pixelformat == rhs->fmt.pix.pixelformat &&
>> + lhs->fmt.pix.field == rhs->fmt.pix.field &&
>> + lhs->fmt.pix.colorspace == rhs->fmt.pix.colorspace;
>
> Add a check for pix.ycbcr_enc and pix.quantization.
>
OK

> 
>
>> +/*
>> + * vpfe_release : This function is based on the vb2_fop_release
>> + * helper function.
>> + * It has been augmented to handle module power management,
>> + * by disabling/enabling h/w module fcntl clock when necessary.
>> + */
>> +static int vpfe_release(struct file *file)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> + int ret;
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_release\n");
>> +
>> + ret = _vb2_fop_release(file, NULL);
>
> This isn't going to work. _vb2_fop_release calls v4l2_fh_release(), so
> the v4l2_fh_is_singular_file(file) will be wrong and you release the fh
> once too many.
>
> I would do this:
>
> if (!v4l2_fh_is_singular_file(file))
> return vb2_fop_release(file);
> mutex_lock(>lock);
> ret = _vb2_fop_release(file, NULL);
> vpfe_ccdc_close(>ccdc, vpfe->pdev);
> mutex_unlock(>lock);
> return ret;
>
>> +
>> + if (v4l2_fh_is_singular_file(file)) {
>> + mutex_lock(>lock);
>> + vpfe_ccdc_close(>ccdc, vpfe->pdev);
>> + v4l2_fh_release(file);
>> + mutex_unlock(>lock);
>> + }
>> +
>> + return ret;
>> +}
>
> 
>
>> +static int vpfe_enum_size(struct file *file, void  *priv,
>> +   struct v4l2_frmsizeenum *fsize)
>> +{
>> + struct vpfe_device *vpfe = video_drvdata(file);
>> + struct v4l2_subdev_frame_size_enum fse;
>> + struct vpfe_subdev_info *sdinfo;
>> + struct v4l2_mbus_framefmt mbus;
>> + struct v4l2_pix_format pix;
>> + struct vpfe_fmt *fmt;
>> + int ret;
>> +
>> + vpfe_dbg(2, vpfe, "vpfe_enum_size\n");
>> +
>> + /* check for valid format */
>> + fmt = find_format_by_pix(fsize->pixel_format);
>> + if (!fmt) {
>> + vpfe_dbg(3, vpfe, "Invalid pixel code: %x, default used 
>> instead\n",
>> + fsize->pixel_format);
>> + return -EINVAL;
>> + }
>> +
>> + memset(fsize->reserved, 0x0, sizeof(fsize->reserved));
>> +
>> + sdinfo = vpfe->current_subdev;
>> + if (!sdinfo->sd)
>> + return -EINVAL;
>> +
>> + memset(, 0x0, sizeof(pix));
>> + /* Construct pix from parameter and use default for the rest */
>> + pix.pixelformat = fsize->pixel_format;
>> + pix.width = 640;
>>

Re: [PATCH v3] media: platform: add VPFE capture driver support for AM437X

2014-12-05 Thread Prabhakar Lad
Hi Hans,

On Fri, Dec 5, 2014 at 12:24 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi Prabhakar,

 Sorry, there are still a few items that need to be fixed.
 If you can make a v4 with these issues addressed, then I can still make a
 pull request, although it depends on Mauro whether it is still accepted for
 3.19.

OK will post a v4 tonight fixing all the below issues.

FYI: Looking at the response of Mauro on 'soc-camera: 1st set for 3.19'
he wont accept it!

Thanks,
--Prabhakar Lad

 On 12/04/2014 12:12 AM, Lad, Prabhakar wrote:
 From: Benoit Parrot bpar...@ti.com

 This patch adds Video Processing Front End (VPFE) driver for
 AM437X family of devices
 Driver supports the following:
 - V4L2 API using MMAP buffer access based on videobuf2 api
 - Asynchronous sensor/decoder sub device registration
 - DT support

 Just to confirm: this driver only supports SDTV formats? No HDTV?
 I didn't see any VIDIOC_*_DV_TIMINGS support, so I assume it really
 isn't supported.


 Signed-off-by: Benoit Parrot bpar...@ti.com
 Signed-off-by: Darren Etheridge detheri...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---

 snip

 diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
 b/drivers/media/platform/am437x/am437x-vpfe.c
 new file mode 100644
 index 000..25863e8
 --- /dev/null
 +++ b/drivers/media/platform/am437x/am437x-vpfe.c

 snip

 +
 +static int
 +cmp_v4l2_format(const struct v4l2_format *lhs, const struct v4l2_format 
 *rhs)
 +{
 + return lhs-type == rhs-type 
 + lhs-fmt.pix.width == rhs-fmt.pix.width 
 + lhs-fmt.pix.height == rhs-fmt.pix.height 
 + lhs-fmt.pix.pixelformat == rhs-fmt.pix.pixelformat 
 + lhs-fmt.pix.field == rhs-fmt.pix.field 
 + lhs-fmt.pix.colorspace == rhs-fmt.pix.colorspace;

 Add a check for pix.ycbcr_enc and pix.quantization.

OK

 snip

 +/*
 + * vpfe_release : This function is based on the vb2_fop_release
 + * helper function.
 + * It has been augmented to handle module power management,
 + * by disabling/enabling h/w module fcntl clock when necessary.
 + */
 +static int vpfe_release(struct file *file)
 +{
 + struct vpfe_device *vpfe = video_drvdata(file);
 + int ret;
 +
 + vpfe_dbg(2, vpfe, vpfe_release\n);
 +
 + ret = _vb2_fop_release(file, NULL);

 This isn't going to work. _vb2_fop_release calls v4l2_fh_release(), so
 the v4l2_fh_is_singular_file(file) will be wrong and you release the fh
 once too many.

 I would do this:

 if (!v4l2_fh_is_singular_file(file))
 return vb2_fop_release(file);
 mutex_lock(vpfe-lock);
 ret = _vb2_fop_release(file, NULL);
 vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 mutex_unlock(vpfe-lock);
 return ret;

 +
 + if (v4l2_fh_is_singular_file(file)) {
 + mutex_lock(vpfe-lock);
 + vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 + v4l2_fh_release(file);
 + mutex_unlock(vpfe-lock);
 + }
 +
 + return ret;
 +}

 snip

 +static int vpfe_enum_size(struct file *file, void  *priv,
 +   struct v4l2_frmsizeenum *fsize)
 +{
 + struct vpfe_device *vpfe = video_drvdata(file);
 + struct v4l2_subdev_frame_size_enum fse;
 + struct vpfe_subdev_info *sdinfo;
 + struct v4l2_mbus_framefmt mbus;
 + struct v4l2_pix_format pix;
 + struct vpfe_fmt *fmt;
 + int ret;
 +
 + vpfe_dbg(2, vpfe, vpfe_enum_size\n);
 +
 + /* check for valid format */
 + fmt = find_format_by_pix(fsize-pixel_format);
 + if (!fmt) {
 + vpfe_dbg(3, vpfe, Invalid pixel code: %x, default used 
 instead\n,
 + fsize-pixel_format);
 + return -EINVAL;
 + }
 +
 + memset(fsize-reserved, 0x0, sizeof(fsize-reserved));
 +
 + sdinfo = vpfe-current_subdev;
 + if (!sdinfo-sd)
 + return -EINVAL;
 +
 + memset(pix, 0x0, sizeof(pix));
 + /* Construct pix from parameter and use default for the rest */
 + pix.pixelformat = fsize-pixel_format;
 + pix.width = 640;
 + pix.height = 480;
 + pix.colorspace = V4L2_COLORSPACE_SRGB;
 + pix.field = V4L2_FIELD_NONE;
 + pix_to_mbus(vpfe, pix, mbus);
 +
 + memset(fse, 0x0, sizeof(fse));
 + fse.index = fsize-index;
 + fse.pad = 0;
 + fse.code = mbus.code;
 + ret = v4l2_subdev_call(sdinfo-sd, pad, enum_frame_size, NULL, fse);

 FYI: strictly speaking this is wrong since this op theoretically expects a
 v4l2_subdev_fh pointer instead of a NULL argument. However, you do not have
 an alternative right now. As you know, I've been working on fixing this, so
 if that gets accepted, then you need to update this code as well in a later
 patch.

 + if (ret)
 + return -EINVAL;
 +
 + vpfe_dbg(1, vpfe, vpfe_enum_size: index: %d code: %x W:[%d,%d] 
 H:[%d,%d]\n,
 + fse.index, fse.code, fse.min_width, fse.max_width,
 + fse.min_height, fse.max_height);
 +
 + fsize

Re: [PATCH] misc: suppress build warning

2014-12-04 Thread Prabhakar Lad
On Thu, Dec 4, 2014 at 2:59 PM, Arnd Bergmann  wrote:
> On Thursday 04 December 2014 14:38:30 Lad, Prabhakar wrote:
>> this patch fixes following build warning:
>>
>> drivers/misc/ioc4.c: In function ‘ioc4_probe’:
>> drivers/misc/ioc4.c:194:16: warning: ‘start’ may be used uninitialized in 
>> this function [-Wmaybe-uninitialized]
>>   period = (end - start) /
>> ^
>> drivers/misc/ioc4.c:148:11: note: ‘start’ was declared here
>>   uint64_t start, end, period;
>>
>> Signed-off-by: Lad, Prabhakar 
>
> Please explain why the compiler thinks there is a bug, why you
> are sure that there isn't, and why you picked '0' as the
> initialization value.
>
Its a false positive, to suppress the warning '0' was picked.

Thanks,
--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] misc: suppress build warning

2014-12-04 Thread Prabhakar Lad
On Thu, Dec 4, 2014 at 2:59 PM, Arnd Bergmann a...@arndb.de wrote:
 On Thursday 04 December 2014 14:38:30 Lad, Prabhakar wrote:
 this patch fixes following build warning:

 drivers/misc/ioc4.c: In function ‘ioc4_probe’:
 drivers/misc/ioc4.c:194:16: warning: ‘start’ may be used uninitialized in 
 this function [-Wmaybe-uninitialized]
   period = (end - start) /
 ^
 drivers/misc/ioc4.c:148:11: note: ‘start’ was declared here
   uint64_t start, end, period;

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com

 Please explain why the compiler thinks there is a bug, why you
 are sure that there isn't, and why you picked '0' as the
 initialization value.

Its a false positive, to suppress the warning '0' was picked.

Thanks,
--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 1/3] video: fbdev: vt8623fb: suppress build warning

2014-12-03 Thread Prabhakar Lad
On Wed, Dec 3, 2014 at 11:49 AM, Tomi Valkeinen  wrote:
> On 27/11/14 00:07, Lad, Prabhakar wrote:
>> this patch fixes following build warning:
>> drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
>> drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer 
>> of different size [-Wint-to-pointer-cast]
>>   par->state.vgabase = (void __iomem *) vga_res.start;
>>^
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>  drivers/video/fbdev/vt8623fb.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
>> index 5c7cbc6..ea7f056 100644
>> --- a/drivers/video/fbdev/vt8623fb.c
>> +++ b/drivers/video/fbdev/vt8623fb.c
>> @@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const 
>> struct pci_device_id *id)
>>
>>   pcibios_bus_to_resource(dev->bus, _res, _reg);
>>
>> - par->state.vgabase = (void __iomem *) vga_res.start;
>> + par->state.vgabase = (void __iomem *) (unsigned long) vga_res.start;
>
> This does look quite ugly... Where does the warning come from in the
> first place. Isn't vga_res.start (resource_size_t) the size of a pointer?
>
Yes looks ugly, I am not sure what you meant from 'where does this warning
come from' its in the commit message.

Thanks,
--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 1/3] video: fbdev: vt8623fb: suppress build warning

2014-12-03 Thread Prabhakar Lad
On Wed, Dec 3, 2014 at 11:49 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On 27/11/14 00:07, Lad, Prabhakar wrote:
 this patch fixes following build warning:
 drivers/video/fbdev/vt8623fb.c: In function ‘vt8623_pci_probe’:
 drivers/video/fbdev/vt8623fb.c:734:23: warning: cast to pointer from integer 
 of different size [-Wint-to-pointer-cast]
   par-state.vgabase = (void __iomem *) vga_res.start;
^
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  drivers/video/fbdev/vt8623fb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
 index 5c7cbc6..ea7f056 100644
 --- a/drivers/video/fbdev/vt8623fb.c
 +++ b/drivers/video/fbdev/vt8623fb.c
 @@ -731,7 +731,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const 
 struct pci_device_id *id)

   pcibios_bus_to_resource(dev-bus, vga_res, bus_reg);

 - par-state.vgabase = (void __iomem *) vga_res.start;
 + par-state.vgabase = (void __iomem *) (unsigned long) vga_res.start;

 This does look quite ugly... Where does the warning come from in the
 first place. Isn't vga_res.start (resource_size_t) the size of a pointer?

Yes looks ugly, I am not sure what you meant from 'where does this warning
come from' its in the commit message.

Thanks,
--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 v8 08/50] virtio: memory access APIs

2014-12-02 Thread Prabhakar Lad
Hi Michael,

Thanks for the patch.

On Mon, Dec 1, 2014 at 4:03 PM, Michael S. Tsirkin  wrote:
> virtio 1.0 makes all memory structures LE, so
[Snip]
> +/*
> + * Low-level memory accessors for handling virtio in modern little endian 
> and in
> + * compatibility native endian format.
> + */
> +
> +static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val)
> +{
> +   if (little_endian)
> +   return le16_to_cpu((__force __le16)val);
> +   else
> +   return (__force u16)val;
> +}
> +
> +static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
> +{
> +   if (little_endian)
> +   return (__force __virtio16)cpu_to_le16(val);
> +   else
> +   return (__force __virtio16)val;
> +}
> +
> +static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
> +{
> +   if (little_endian)
> +   return le32_to_cpu((__force __le32)val);
> +   else
> +   return (__force u32)val;
> +}
> +
> +static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
> +{
> +   if (little_endian)
> +   return (__force __virtio32)cpu_to_le32(val);
> +   else
> +   return (__force __virtio32)val;
> +}
> +
> +static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
> +{
> +   if (little_endian)
> +   return le64_to_cpu((__force __le64)val);
> +   else
> +   return (__force u64)val;
> +}
> +
> +static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)
> +{
> +   if (little_endian)
> +   return (__force __virtio64)cpu_to_le64(val);
> +   else
> +   return (__force __virtio64)val;
> +}
> +

Nitpicking, could remove the else for the all above functions and
align the return appropriately ?

Apart from that patch looks good.

Acked-by: Lad, Prabhakar 

Thanks,
--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 v2] media: platform: add VPFE capture driver support for AM437X

2014-12-02 Thread Prabhakar Lad
556s
Buffer: 0 Sequence: 80 Field: None Timestamp: 75.819436s
Buffer: 1 Sequence: 81 Field: None Timestamp: 75.842317s
Buffer: 2 Sequence: 82 Field: None Timestamp: 75.865197s
Buffer: 3 Sequence: 83 Field: None Timestamp: 75.888077s
Buffer: 0 Sequence: 84 Field: None Timestamp: 75.910959s
Buffer: 1 Sequence: 85 Field: None Timestamp: 75.933838s
Buffer: 2 Sequence: 86 Field: None Timestamp: 75.956718s
Buffer: 3 Sequence: 87 Field: None Timestamp: 75.979599s
Buffer: 0 Sequence: 88 Field: None Timestamp: 76.002479s
Buffer: 1 Sequence: 89 Field: None Timestamp: 76.025360s
Buffer: 2 Sequence: 90 Field: None Timestamp: 76.048243s
Buffer: 3 Sequence: 91 Field: None Timestamp: 76.071122s
Buffer: 0 Sequence: 92 Field: None Timestamp: 76.094000s
Buffer: 1 Sequence: 93 Field: None Timestamp: 76.116881s
Buffer: 2 Sequence: 94 Field: None Timestamp: 76.139761s
Buffer: 3 Sequence: 95 Field: None Timestamp: 76.162642s
Buffer: 0 Sequence: 96 Field: None Timestamp: 76.185522s
Buffer: 1 Sequence: 97 Field: None Timestamp: 76.208402s
Buffer: 2 Sequence: 98 Field: None Timestamp: 76.231284s
Buffer: 3 Sequence: 99 Field: None Timestamp: 76.254163s
Buffer: 0 Sequence: 100 Field: None Timestamp: 76.277044s
Buffer: 1 Sequence: 101 Field: None Timestamp: 76.299924s
Buffer: 2 Sequence: 102 Field: None Timestamp: 76.322805s
Buffer: 3 Sequence: 103 Field: None Timestamp: 76.345685s
Buffer: 0 Sequence: 104 Field: None Timestamp: 76.368565s
Buffer: 1 Sequence: 105 Field: None Timestamp: 76.391447s
Buffer: 2 Sequence: 106 Field: None Timestamp: 76.414326s
Buffer: 3 Sequence: 107 Field: None Timestamp: 76.437206s
Buffer: 0 Sequence: 108 Field: None Timestamp: 76.460087s
Buffer: 1 Sequence: 109 Field: None Timestamp: 76.482967s
Buffer: 2 Sequence: 110 Field: None Timestamp: 76.505847s
Buffer: 3 Sequence: 111 Field: None Timestamp: 76.528727s
Buffer: 0 Sequence: 112 Field: None Timestamp: 76.551607s
Buffer: 1 Sequence: 113 Field: None Timestamp: 76.574488s
Buffer: 2 Sequence: 114 Field: None Timestamp: 76.597369s
Buffer: 3 Sequence: 115 Field: None Timestamp: 76.620250s
Buffer: 0 Sequence: 116 Field: None Timestamp: 76.643129s
Buffer: 1 Sequence: 117 Field: None Timestamp: 76.666010s
Buffer: 2 Sequence: 118 Field: None Timestamp: 76.688890s
Buffer: 3 Sequence: 119 Field: None Timestamp: 76.711771s
test MMAP: OK
test USERPTR: OK (Not Supported)
test DMABUF: Cannot test, specify --expbuf-device

Total: 42, Succeeded: 42, Failed: 0, Warnings: 0

Thanks,
--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 v1] lib/genalloc.c: Export devm_gen_pool_create for modules

2014-12-02 Thread Prabhakar Lad
Hi Michal,

On Tue, Dec 2, 2014 at 9:36 AM, Michal Simek  wrote:
> On 12/02/2014 10:31 AM, Prabhakar Lad wrote:
>> Hi Michal,
>>
>> Thanks for the patch.
>>
>> On Mon, Dec 1, 2014 at 1:00 PM, Michal Simek  wrote:
>>> Modules can use this function for creating pool.
>>>
>>> Signed-off-by: Michal Simek 
>>> ---
>>>
>>> I am pushing Zynq OCMC driver which is using this function.
>>>
>>> ---
>>>  lib/genalloc.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>> don’t you need to add an entry in include/linux/genalloc.h ?
>
> Not sure what exactly you mean. declaration is there.
> include/linux/genalloc.h:120:extern struct gen_pool 
> *devm_gen_pool_create(struct device *dev,
>
> And all EXPORT_SYMBOL() are out of headers directly below function
> which you want to export.
>
> Can you please clarify what you did mean by that?
>
Oops missed it, since it was a exported function I was expecting a entry
in a header file, but yes the entry  exists already so,

Acked-by: Lad, Prabhakar 

Thanks,
--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 v1] lib/genalloc.c: Export devm_gen_pool_create for modules

2014-12-02 Thread Prabhakar Lad
Hi Michal,

Thanks for the patch.

On Mon, Dec 1, 2014 at 1:00 PM, Michal Simek  wrote:
> Modules can use this function for creating pool.
>
> Signed-off-by: Michal Simek 
> ---
>
> I am pushing Zynq OCMC driver which is using this function.
>
> ---
>  lib/genalloc.c | 1 +
>  1 file changed, 1 insertion(+)
>
don’t you need to add an entry in include/linux/genalloc.h ?

Thanks,
--Prabhakar Lad

> diff --git a/lib/genalloc.c b/lib/genalloc.c
> index cce4dd68c40d..2e65d206b01c 100644
> --- a/lib/genalloc.c
> +++ b/lib/genalloc.c
> @@ -598,6 +598,7 @@ struct gen_pool *devm_gen_pool_create(struct device *dev, 
> int min_alloc_order,
>
> return pool;
>  }
> +EXPORT_SYMBOL(devm_gen_pool_create);
>
>  /**
>   * dev_get_gen_pool - Obtain the gen_pool (if any) for a device
> --
> 1.8.2.3
>
--
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 v1] lib/genalloc.c: Export devm_gen_pool_create for modules

2014-12-02 Thread Prabhakar Lad
Hi Michal,

Thanks for the patch.

On Mon, Dec 1, 2014 at 1:00 PM, Michal Simek michal.si...@xilinx.com wrote:
 Modules can use this function for creating pool.

 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---

 I am pushing Zynq OCMC driver which is using this function.

 ---
  lib/genalloc.c | 1 +
  1 file changed, 1 insertion(+)

don’t you need to add an entry in include/linux/genalloc.h ?

Thanks,
--Prabhakar Lad

 diff --git a/lib/genalloc.c b/lib/genalloc.c
 index cce4dd68c40d..2e65d206b01c 100644
 --- a/lib/genalloc.c
 +++ b/lib/genalloc.c
 @@ -598,6 +598,7 @@ struct gen_pool *devm_gen_pool_create(struct device *dev, 
 int min_alloc_order,

 return pool;
  }
 +EXPORT_SYMBOL(devm_gen_pool_create);

  /**
   * dev_get_gen_pool - Obtain the gen_pool (if any) for a device
 --
 1.8.2.3

--
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 v1] lib/genalloc.c: Export devm_gen_pool_create for modules

2014-12-02 Thread Prabhakar Lad
Hi Michal,

On Tue, Dec 2, 2014 at 9:36 AM, Michal Simek michal.si...@xilinx.com wrote:
 On 12/02/2014 10:31 AM, Prabhakar Lad wrote:
 Hi Michal,

 Thanks for the patch.

 On Mon, Dec 1, 2014 at 1:00 PM, Michal Simek michal.si...@xilinx.com wrote:
 Modules can use this function for creating pool.

 Signed-off-by: Michal Simek michal.si...@xilinx.com
 ---

 I am pushing Zynq OCMC driver which is using this function.

 ---
  lib/genalloc.c | 1 +
  1 file changed, 1 insertion(+)

 don’t you need to add an entry in include/linux/genalloc.h ?

 Not sure what exactly you mean. declaration is there.
 include/linux/genalloc.h:120:extern struct gen_pool 
 *devm_gen_pool_create(struct device *dev,

 And all EXPORT_SYMBOL() are out of headers directly below function
 which you want to export.

 Can you please clarify what you did mean by that?

Oops missed it, since it was a exported function I was expecting a entry
in a header file, but yes the entry  exists already so,

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--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 v2] media: platform: add VPFE capture driver support for AM437X

2014-12-02 Thread Prabhakar Lad
: 75.796556s
Buffer: 0 Sequence: 80 Field: None Timestamp: 75.819436s
Buffer: 1 Sequence: 81 Field: None Timestamp: 75.842317s
Buffer: 2 Sequence: 82 Field: None Timestamp: 75.865197s
Buffer: 3 Sequence: 83 Field: None Timestamp: 75.888077s
Buffer: 0 Sequence: 84 Field: None Timestamp: 75.910959s
Buffer: 1 Sequence: 85 Field: None Timestamp: 75.933838s
Buffer: 2 Sequence: 86 Field: None Timestamp: 75.956718s
Buffer: 3 Sequence: 87 Field: None Timestamp: 75.979599s
Buffer: 0 Sequence: 88 Field: None Timestamp: 76.002479s
Buffer: 1 Sequence: 89 Field: None Timestamp: 76.025360s
Buffer: 2 Sequence: 90 Field: None Timestamp: 76.048243s
Buffer: 3 Sequence: 91 Field: None Timestamp: 76.071122s
Buffer: 0 Sequence: 92 Field: None Timestamp: 76.094000s
Buffer: 1 Sequence: 93 Field: None Timestamp: 76.116881s
Buffer: 2 Sequence: 94 Field: None Timestamp: 76.139761s
Buffer: 3 Sequence: 95 Field: None Timestamp: 76.162642s
Buffer: 0 Sequence: 96 Field: None Timestamp: 76.185522s
Buffer: 1 Sequence: 97 Field: None Timestamp: 76.208402s
Buffer: 2 Sequence: 98 Field: None Timestamp: 76.231284s
Buffer: 3 Sequence: 99 Field: None Timestamp: 76.254163s
Buffer: 0 Sequence: 100 Field: None Timestamp: 76.277044s
Buffer: 1 Sequence: 101 Field: None Timestamp: 76.299924s
Buffer: 2 Sequence: 102 Field: None Timestamp: 76.322805s
Buffer: 3 Sequence: 103 Field: None Timestamp: 76.345685s
Buffer: 0 Sequence: 104 Field: None Timestamp: 76.368565s
Buffer: 1 Sequence: 105 Field: None Timestamp: 76.391447s
Buffer: 2 Sequence: 106 Field: None Timestamp: 76.414326s
Buffer: 3 Sequence: 107 Field: None Timestamp: 76.437206s
Buffer: 0 Sequence: 108 Field: None Timestamp: 76.460087s
Buffer: 1 Sequence: 109 Field: None Timestamp: 76.482967s
Buffer: 2 Sequence: 110 Field: None Timestamp: 76.505847s
Buffer: 3 Sequence: 111 Field: None Timestamp: 76.528727s
Buffer: 0 Sequence: 112 Field: None Timestamp: 76.551607s
Buffer: 1 Sequence: 113 Field: None Timestamp: 76.574488s
Buffer: 2 Sequence: 114 Field: None Timestamp: 76.597369s
Buffer: 3 Sequence: 115 Field: None Timestamp: 76.620250s
Buffer: 0 Sequence: 116 Field: None Timestamp: 76.643129s
Buffer: 1 Sequence: 117 Field: None Timestamp: 76.666010s
Buffer: 2 Sequence: 118 Field: None Timestamp: 76.688890s
Buffer: 3 Sequence: 119 Field: None Timestamp: 76.711771s
test MMAP: OK
test USERPTR: OK (Not Supported)
test DMABUF: Cannot test, specify --expbuf-device

Total: 42, Succeeded: 42, Failed: 0, Warnings: 0

Thanks,
--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 v8 08/50] virtio: memory access APIs

2014-12-02 Thread Prabhakar Lad
Hi Michael,

Thanks for the patch.

On Mon, Dec 1, 2014 at 4:03 PM, Michael S. Tsirkin m...@redhat.com wrote:
 virtio 1.0 makes all memory structures LE, so
[Snip]
 +/*
 + * Low-level memory accessors for handling virtio in modern little endian 
 and in
 + * compatibility native endian format.
 + */
 +
 +static inline u16 __virtio16_to_cpu(bool little_endian, __virtio16 val)
 +{
 +   if (little_endian)
 +   return le16_to_cpu((__force __le16)val);
 +   else
 +   return (__force u16)val;
 +}
 +
 +static inline __virtio16 __cpu_to_virtio16(bool little_endian, u16 val)
 +{
 +   if (little_endian)
 +   return (__force __virtio16)cpu_to_le16(val);
 +   else
 +   return (__force __virtio16)val;
 +}
 +
 +static inline u32 __virtio32_to_cpu(bool little_endian, __virtio32 val)
 +{
 +   if (little_endian)
 +   return le32_to_cpu((__force __le32)val);
 +   else
 +   return (__force u32)val;
 +}
 +
 +static inline __virtio32 __cpu_to_virtio32(bool little_endian, u32 val)
 +{
 +   if (little_endian)
 +   return (__force __virtio32)cpu_to_le32(val);
 +   else
 +   return (__force __virtio32)val;
 +}
 +
 +static inline u64 __virtio64_to_cpu(bool little_endian, __virtio64 val)
 +{
 +   if (little_endian)
 +   return le64_to_cpu((__force __le64)val);
 +   else
 +   return (__force u64)val;
 +}
 +
 +static inline __virtio64 __cpu_to_virtio64(bool little_endian, u64 val)
 +{
 +   if (little_endian)
 +   return (__force __virtio64)cpu_to_le64(val);
 +   else
 +   return (__force __virtio64)val;
 +}
 +

Nitpicking, could remove the else for the all above functions and
align the return appropriately ?

Apart from that patch looks good.

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Mon, Dec 1, 2014 at 11:26 PM, Laurent Pinchart
 wrote:
> Hi Prabhakar,
>
> On Sunday 30 November 2014 21:30:35 Prabhakar Lad wrote:
>> On Sun, Nov 30, 2014 at 9:16 PM, Laurent Pinchart wrote:
>> > On Sunday 30 November 2014 21:05:50 Prabhakar Lad wrote:
>> >> On Sat, Nov 29, 2014 at 7:12 PM, Laurent Pinchart wrote:
>> >> > Hi Prabhakar,
>> >>
>> >> [Snip]
>> >>
>> >>>>> Sure. That's a better choice than removing the config option
>> >>>>> dependency of the fields struct v4l2_subdev.
>> >>>
>> >>> Decoupling CONFIG_VIDEO_V4L2_SUBDEV_API from the availability of the
>> >>> in-kernel pad format and selection rectangles helpers is definitely a
>> >>> good idea. I was thinking about decoupling the try format and
>> >>> rectangles from v4l2_subdev_fh by creating a kind of configuration store
>> >>> structure to store them, and embedding that structure in v4l2_subdev_fh.
>> >>> The pad-level operations would then take a pointer to the configuration
>> >>> store instead of the v4l2_subdev_fh. Bridge drivers that want to
>> >>> implement TRY_FMT based on pad-level operations would create a
>> >>> configuration store, use the pad-level operations, and destroy the
>> >>> configuration store. The userspace subdev API would use the
>> >>> configuration store from the file handle.
>> >>
>> >> are planning to work/post any time soon ? Or are you OK with suggestion
>> >> from Hans ?
>> >
>> > I have no plan to work on that myself now, I was hoping you could
>> > implement it ;-)
>>
>> OK will implement it.
>>
>> Can you please elaborate a more on this "The userspace subdev API would use
>> the configuration store from the file handle."
>
> Basically,
>
> 1. Create a subdev pad configuration store structure to store the formats and
> selection rectangles for each pad.
>
> 2. Embed an instance of that structure in v4l2_subdev_fh.
>
> 3. Modify the subdev pad ops to take a configuration store pointer instead of
> a file handle pointer.
>
> The userspace API implementation (v4l2-subdev.c) would then pass >store to
> the pad operations instead of fh.
>
> Bridge drivers that need to implement TRY_FMT on top of pad ops would create a
> temporary store (or temporary stores when multiple subsdevs are involved),
> call the pad ops with a pointer to the temporary store to propagate TRY
> formats, destroy the store(s) and return the resulting format.
>
> Is that clear ?
>
Yes thank you, I'll soon shoot a RFC version.

Regards,
--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] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Tue, Dec 2, 2014 at 7:32 AM, Hans Verkuil  wrote:
> On 12/01/2014 11:27 PM, Prabhakar Lad wrote:
>> Hi Hans,
>>
>> On Mon, Dec 1, 2014 at 11:00 AM, Hans Verkuil  wrote:
>>> On 12/01/2014 11:11 AM, Hans Verkuil wrote:
>>>> Hi all,
>>>>
>>>> Thanks for the patch, review comments are below.
>>>>
>>>> For the next version I would appreciate if someone can test this driver 
>>>> with
>>>> the latest v4l2-compliance from the v4l-utils git repo. If possible test 
>>>> streaming
>>>> as well (v4l2-compliance -s), ideally with both a sensor and a STD 
>>>> receiver input.
>>>> But that depends on the available hardware of course.
>>>>
>>>> I'd like to see the v4l2-compliance output. It's always good to have that 
>>>> on
>>>> record.
>>>>
>>>>
>>>> On 11/24/2014 02:10 AM, Lad, Prabhakar wrote:
>>>>> From: Benoit Parrot 
>>>>>
>>>>> This patch adds Video Processing Front End (VPFE) driver for
>>>>> AM437X family of devices
>>>>> Driver supports the following:
>>>>> - V4L2 API using MMAP buffer access based on videobuf2 api
>>>>> - Asynchronous sensor/decoder sub device registration
>>>>> - DT support
>>>>>
>>>>> Signed-off-by: Benoit Parrot 
>>>>> Signed-off-by: Darren Etheridge 
>>>>> Signed-off-by: Lad, Prabhakar 
>>>>> ---
>>>>>  .../devicetree/bindings/media/ti-am437xx-vpfe.txt  |   61 +
>>>>>  MAINTAINERS|9 +
>>>>>  drivers/media/platform/Kconfig |1 +
>>>>>  drivers/media/platform/Makefile|2 +
>>>>>  drivers/media/platform/am437x/Kconfig  |   10 +
>>>>>  drivers/media/platform/am437x/Makefile |2 +
>>>>>  drivers/media/platform/am437x/vpfe.c   | 2764 
>>>>> 
>>>>>  drivers/media/platform/am437x/vpfe.h   |  286 ++
>>>>>  drivers/media/platform/am437x/vpfe_regs.h  |  140 +
>>>>>  include/uapi/linux/Kbuild  |1 +
>>>>>  include/uapi/linux/am437x_vpfe.h   |  122 +
>>>>>  11 files changed, 3398 insertions(+)
>>>
>>> Can the source names be improved? There are too many 'vpfe' sources.
>>> Perhaps prefix all with 'am437x-'?
>>>
>> I did think of it but, dropped it as anyway the source's are present
>> in am437x folder, again naming the files am437x-xxx.x didn't make
>> me feel good. If you still insists I'll do it.
>
> Yes, please do. If you look at most other drivers they all have a prefix.
>
> Another reason is that the media_build system expects unique names.
>
OK makes sense I'll prefix it with 'am437x-'. probably fixing all the review
comments i'll post a v2 end of the day.

Thanks,
--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] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Tue, Dec 2, 2014 at 7:26 AM, Hans Verkuil  wrote:
> On 12/01/2014 11:17 PM, Prabhakar Lad wrote:
>> Hi Hans,
>>
>> Thanks for the review.
>>
>> On Mon, Dec 1, 2014 at 10:11 AM, Hans Verkuil  wrote:
>>> Hi all,
>>>
>>> Thanks for the patch, review comments are below.
>>>
>>> For the next version I would appreciate if someone can test this driver with
>>> the latest v4l2-compliance from the v4l-utils git repo. If possible test 
>>> streaming
>>> as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver 
>>> input.
>>> But that depends on the available hardware of course.
>>>
>>> I'd like to see the v4l2-compliance output. It's always good to have that on
>>> record.
>>>
>> Following is the compliance output, missed it post it along with patch:
>>
>> root@am437x-evm:~# ./v4l2-compliance -s -i 0 -v
>> Driver Info:
>> Driver name   : vpfe
>> Card type :[   70.363908] vpfe 48326000.vpfe:
>> =  START STATUS  =
>>  TI AM437x VPFE
>> Bus info  : platform:vpfe [   70.375576] vpfe
>> 48326000.vpfe: ==  END STATUS  ==
>> 48326000.vpfe
>> Driver version: 3.18.0
>> Capabil[   70.388485] vpfe 48326000.vpfe: invalid input index: 1
>> ities  : 0x8521
>> Video Capture
>> Read/Write
>> Streaming
>> Extended Pix Format
>> Device Capabilities
>> Device Caps   : 0x0521
>> Video Capture
>> Read/Write
>> Streaming
>> Extended Pix Format
>>
>> Compliance test for device /dev/video0 (not using libv4l2):
>>
>> Required ioctls:
>> test VIDIOC_QUERYCAP: OK
>>
>> Allow for multiple opens:
>> test second video open: OK
>> test VIDIOC_QUERYCAP: OK
>> test VIDIOC_G/S_PRIORITY: OK
>>
>> Debug ioctls:
>> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>> test VIDIOC_LOG_STATUS: OK
>>
>> Input ioctls:
>> test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>> test VIDIOC_ENUMAUDIO: OK (Not Supported)
>> test VIDIOC_G/S/ENUMINPUT: OK
>> test VIDIOC_G/S_AUDIO: OK (Not Supported)
>> Inputs: 1 Audio Inputs: 0 Tuners: 0
>>
>> Output ioctls:
>> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>> Outputs: 0 Audio Outputs: 0 Modulators: 0
>>
>> Input/Output configuration ioctls:
>> test VIDIOC_ENUM/G/S/QUERY_STD: OK
>> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>> test VIDIOC_G/S_EDID: OK (Not Supported)
>>
>> Test input 0:
>>
>> Control ioctls:
>> test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
>> test VIDIOC_QUERYCTRL: OK (Not Supported)
>> test VIDIOC_G/S_CTRL: OK (Not Supported)
>> test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>> Standard Controls: 0 Private Controls: 0
>>
>> Format ioctls:
>> info: found 7 framesizes for pixel format 56595559
>> info: found 7 framesizes for pixel format 59565955
>> info: found 7 framesizes for pixel format 52424752
>> info: found 7 framesizes for pixel format 31384142
>> info: found 4 formats for buftype 1
>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>> test VIDIOC_G/S_PARM: OK
>> test VIDIOC_G_FBUF: OK (Not Supported)
>> test VIDIOC_G_FMT: OK
>> test VIDIOC_TRY_FMT: OK
>> info: Global format check succeeded for type 1
>> test VIDIOC_S_FMT: OK
>> test V

Re: [PATCH] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Mon, Dec 1, 2014 at 11:00 AM, Hans Verkuil  wrote:
> On 12/01/2014 11:11 AM, Hans Verkuil wrote:
>> Hi all,
>>
>> Thanks for the patch, review comments are below.
>>
>> For the next version I would appreciate if someone can test this driver with
>> the latest v4l2-compliance from the v4l-utils git repo. If possible test 
>> streaming
>> as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver 
>> input.
>> But that depends on the available hardware of course.
>>
>> I'd like to see the v4l2-compliance output. It's always good to have that on
>> record.
>>
>>
>> On 11/24/2014 02:10 AM, Lad, Prabhakar wrote:
>>> From: Benoit Parrot 
>>>
>>> This patch adds Video Processing Front End (VPFE) driver for
>>> AM437X family of devices
>>> Driver supports the following:
>>> - V4L2 API using MMAP buffer access based on videobuf2 api
>>> - Asynchronous sensor/decoder sub device registration
>>> - DT support
>>>
>>> Signed-off-by: Benoit Parrot 
>>> Signed-off-by: Darren Etheridge 
>>> Signed-off-by: Lad, Prabhakar 
>>> ---
>>>  .../devicetree/bindings/media/ti-am437xx-vpfe.txt  |   61 +
>>>  MAINTAINERS|9 +
>>>  drivers/media/platform/Kconfig |1 +
>>>  drivers/media/platform/Makefile|2 +
>>>  drivers/media/platform/am437x/Kconfig  |   10 +
>>>  drivers/media/platform/am437x/Makefile |2 +
>>>  drivers/media/platform/am437x/vpfe.c   | 2764 
>>> 
>>>  drivers/media/platform/am437x/vpfe.h   |  286 ++
>>>  drivers/media/platform/am437x/vpfe_regs.h  |  140 +
>>>  include/uapi/linux/Kbuild  |1 +
>>>  include/uapi/linux/am437x_vpfe.h   |  122 +
>>>  11 files changed, 3398 insertions(+)
>
> Can the source names be improved? There are too many 'vpfe' sources.
> Perhaps prefix all with 'am437x-'?
>
I did think of it but, dropped it as anyway the source's are present
in am437x folder, again naming the files am437x-xxx.x didn't make
me feel good. If you still insists I'll do it.

> In general I prefer '-' over '_' in a source name: it's looks better
> IMHO.
>
I am almost done with all the review comments, if we take a decision
on this quickly I can post a v2 soon.

> One question, unrelated to this patch series:
>
> Prabhakar, would it make sense to look at the various existing TI sources
> as well and rename them to make it more explicit for which SoCs they are
> meant? Most are pretty vague with variations on vpe, vpif, vpfe, etc. but
> no reference to the actual SoC they are for.
>
vpe - definitely needs to be changed.
vpif - under davinci is common for (Davinci series
dm6467/dm6467t/omapl138/am1808)
vpfe -  under davinci is common for (Davinci series dm36x/dm6446/dm355)

I am falling short of names for renaming this common drivers :)

Thanks,
--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] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

Thanks for the review.

On Mon, Dec 1, 2014 at 10:11 AM, Hans Verkuil  wrote:
> Hi all,
>
> Thanks for the patch, review comments are below.
>
> For the next version I would appreciate if someone can test this driver with
> the latest v4l2-compliance from the v4l-utils git repo. If possible test 
> streaming
> as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver 
> input.
> But that depends on the available hardware of course.
>
> I'd like to see the v4l2-compliance output. It's always good to have that on
> record.
>
Following is the compliance output, missed it post it along with patch:

root@am437x-evm:~# ./v4l2-compliance -s -i 0 -v
Driver Info:
Driver name   : vpfe
Card type :[   70.363908] vpfe 48326000.vpfe:
=  START STATUS  =
 TI AM437x VPFE
Bus info  : platform:vpfe [   70.375576] vpfe
48326000.vpfe: ==  END STATUS  ==
48326000.vpfe
Driver version: 3.18.0
Capabil[   70.388485] vpfe 48326000.vpfe: invalid input index: 1
ities  : 0x8521
Video Capture
Read/Write
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0521
Video Capture
Read/Write
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0

Format ioctls:
info: found 7 framesizes for pixel format 56595559
info: found 7 framesizes for pixel format 59565955
info: found 7 framesizes for pixel format 52424752
info: found 7 framesizes for pixel format 31384142
info: found 4 formats for buftype 1
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
info: Global format check succeeded for type 1
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)

Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
info: test buftype Video Capture
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

Streaming ioctls:
test read/write: OK
Video Capture:
Buffer: 0 Sequence: 0 Field: None Timestamp: 74.956437s
Buffer: 1 Sequence: 0 Field: None Timestamp: 74.979310s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.002191s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.208114s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.230998s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.253877s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.276756s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.299637s
   

Re: [PATCH] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

Thanks for the review.

On Mon, Dec 1, 2014 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi all,

 Thanks for the patch, review comments are below.

 For the next version I would appreciate if someone can test this driver with
 the latest v4l2-compliance from the v4l-utils git repo. If possible test 
 streaming
 as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver 
 input.
 But that depends on the available hardware of course.

 I'd like to see the v4l2-compliance output. It's always good to have that on
 record.

Following is the compliance output, missed it post it along with patch:

root@am437x-evm:~# ./v4l2-compliance -s -i 0 -v
Driver Info:
Driver name   : vpfe
Card type :[   70.363908] vpfe 48326000.vpfe:
=  START STATUS  =
 TI AM437x VPFE
Bus info  : platform:vpfe [   70.375576] vpfe
48326000.vpfe: ==  END STATUS  ==
48326000.vpfe
Driver version: 3.18.0
Capabil[   70.388485] vpfe 48326000.vpfe: invalid input index: 1
ities  : 0x8521
Video Capture
Read/Write
Streaming
Extended Pix Format
Device Capabilities
Device Caps   : 0x0521
Video Capture
Read/Write
Streaming
Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
test second video open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK

Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK

Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0

Format ioctls:
info: found 7 framesizes for pixel format 56595559
info: found 7 framesizes for pixel format 59565955
info: found 7 framesizes for pixel format 52424752
info: found 7 framesizes for pixel format 31384142
info: found 4 formats for buftype 1
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
info: Global format check succeeded for type 1
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)

Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls:
info: test buftype Video Capture
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test VIDIOC_EXPBUF: OK

Streaming ioctls:
test read/write: OK
Video Capture:
Buffer: 0 Sequence: 0 Field: None Timestamp: 74.956437s
Buffer: 1 Sequence: 0 Field: None Timestamp: 74.979310s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.002191s
Buffer: 3 Sequence: 0 Field: None Timestamp: 75.208114s
Buffer: 0 Sequence: 0 Field: None Timestamp: 75.230998s
Buffer: 1 Sequence: 0 Field: None Timestamp: 75.253877s
Buffer: 2 Sequence: 0 Field: None Timestamp: 75.276756s
Buffer: 3 Sequence: 0 Field: None Timestamp: 

Re: [PATCH] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Mon, Dec 1, 2014 at 11:00 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 12/01/2014 11:11 AM, Hans Verkuil wrote:
 Hi all,

 Thanks for the patch, review comments are below.

 For the next version I would appreciate if someone can test this driver with
 the latest v4l2-compliance from the v4l-utils git repo. If possible test 
 streaming
 as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver 
 input.
 But that depends on the available hardware of course.

 I'd like to see the v4l2-compliance output. It's always good to have that on
 record.


 On 11/24/2014 02:10 AM, Lad, Prabhakar wrote:
 From: Benoit Parrot bpar...@ti.com

 This patch adds Video Processing Front End (VPFE) driver for
 AM437X family of devices
 Driver supports the following:
 - V4L2 API using MMAP buffer access based on videobuf2 api
 - Asynchronous sensor/decoder sub device registration
 - DT support

 Signed-off-by: Benoit Parrot bpar...@ti.com
 Signed-off-by: Darren Etheridge detheri...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  .../devicetree/bindings/media/ti-am437xx-vpfe.txt  |   61 +
  MAINTAINERS|9 +
  drivers/media/platform/Kconfig |1 +
  drivers/media/platform/Makefile|2 +
  drivers/media/platform/am437x/Kconfig  |   10 +
  drivers/media/platform/am437x/Makefile |2 +
  drivers/media/platform/am437x/vpfe.c   | 2764 
 
  drivers/media/platform/am437x/vpfe.h   |  286 ++
  drivers/media/platform/am437x/vpfe_regs.h  |  140 +
  include/uapi/linux/Kbuild  |1 +
  include/uapi/linux/am437x_vpfe.h   |  122 +
  11 files changed, 3398 insertions(+)

 Can the source names be improved? There are too many 'vpfe' sources.
 Perhaps prefix all with 'am437x-'?

I did think of it but, dropped it as anyway the source's are present
in am437x folder, again naming the files am437x-xxx.x didn't make
me feel good. If you still insists I'll do it.

 In general I prefer '-' over '_' in a source name: it's looks better
 IMHO.

I am almost done with all the review comments, if we take a decision
on this quickly I can post a v2 soon.

 One question, unrelated to this patch series:

 Prabhakar, would it make sense to look at the various existing TI sources
 as well and rename them to make it more explicit for which SoCs they are
 meant? Most are pretty vague with variations on vpe, vpif, vpfe, etc. but
 no reference to the actual SoC they are for.

vpe - definitely needs to be changed.
vpif - under davinci is common for (Davinci series
dm6467/dm6467t/omapl138/am1808)
vpfe -  under davinci is common for (Davinci series dm36x/dm6446/dm355)

I am falling short of names for renaming this common drivers :)

Thanks,
--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] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Tue, Dec 2, 2014 at 7:26 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 12/01/2014 11:17 PM, Prabhakar Lad wrote:
 Hi Hans,

 Thanks for the review.

 On Mon, Dec 1, 2014 at 10:11 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi all,

 Thanks for the patch, review comments are below.

 For the next version I would appreciate if someone can test this driver with
 the latest v4l2-compliance from the v4l-utils git repo. If possible test 
 streaming
 as well (v4l2-compliance -s), ideally with both a sensor and a STD receiver 
 input.
 But that depends on the available hardware of course.

 I'd like to see the v4l2-compliance output. It's always good to have that on
 record.

 Following is the compliance output, missed it post it along with patch:

 root@am437x-evm:~# ./v4l2-compliance -s -i 0 -v
 Driver Info:
 Driver name   : vpfe
 Card type :[   70.363908] vpfe 48326000.vpfe:
 =  START STATUS  =
  TI AM437x VPFE
 Bus info  : platform:vpfe [   70.375576] vpfe
 48326000.vpfe: ==  END STATUS  ==
 48326000.vpfe
 Driver version: 3.18.0
 Capabil[   70.388485] vpfe 48326000.vpfe: invalid input index: 1
 ities  : 0x8521
 Video Capture
 Read/Write
 Streaming
 Extended Pix Format
 Device Capabilities
 Device Caps   : 0x0521
 Video Capture
 Read/Write
 Streaming
 Extended Pix Format

 Compliance test for device /dev/video0 (not using libv4l2):

 Required ioctls:
 test VIDIOC_QUERYCAP: OK

 Allow for multiple opens:
 test second video open: OK
 test VIDIOC_QUERYCAP: OK
 test VIDIOC_G/S_PRIORITY: OK

 Debug ioctls:
 test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
 test VIDIOC_LOG_STATUS: OK

 Input ioctls:
 test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
 test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
 test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
 test VIDIOC_ENUMAUDIO: OK (Not Supported)
 test VIDIOC_G/S/ENUMINPUT: OK
 test VIDIOC_G/S_AUDIO: OK (Not Supported)
 Inputs: 1 Audio Inputs: 0 Tuners: 0

 Output ioctls:
 test VIDIOC_G/S_MODULATOR: OK (Not Supported)
 test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
 test VIDIOC_ENUMAUDOUT: OK (Not Supported)
 test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
 test VIDIOC_G/S_AUDOUT: OK (Not Supported)
 Outputs: 0 Audio Outputs: 0 Modulators: 0

 Input/Output configuration ioctls:
 test VIDIOC_ENUM/G/S/QUERY_STD: OK
 test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
 test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
 test VIDIOC_G/S_EDID: OK (Not Supported)

 Test input 0:

 Control ioctls:
 test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
 test VIDIOC_QUERYCTRL: OK (Not Supported)
 test VIDIOC_G/S_CTRL: OK (Not Supported)
 test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
 test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
 test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
 Standard Controls: 0 Private Controls: 0

 Format ioctls:
 info: found 7 framesizes for pixel format 56595559
 info: found 7 framesizes for pixel format 59565955
 info: found 7 framesizes for pixel format 52424752
 info: found 7 framesizes for pixel format 31384142
 info: found 4 formats for buftype 1
 test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
 test VIDIOC_G/S_PARM: OK
 test VIDIOC_G_FBUF: OK (Not Supported)
 test VIDIOC_G_FMT: OK
 test VIDIOC_TRY_FMT: OK
 info: Global format check succeeded for type 1
 test VIDIOC_S_FMT: OK
 test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)

 Codec ioctls:
 test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
 test VIDIOC_G_ENC_INDEX: OK (Not Supported)
 test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

 Buffer ioctls:
 info: test buftype Video Capture
 test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
 test VIDIOC_EXPBUF: OK

 Streaming ioctls:
 test read/write: OK
 Video Capture:
 Buffer: 0 Sequence: 0 Field: None Timestamp: 74.956437s
 Buffer: 1 Sequence: 0 Field: None Timestamp: 74.979310s
 Buffer: 2 Sequence: 0 Field: None Timestamp: 75.002191s
 Buffer: 3 Sequence: 0 Field: None Timestamp: 75.208114s
 Buffer: 0 Sequence: 0 Field: None Timestamp

Re: [PATCH] media: platform: add VPFE capture driver support for AM437X

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Tue, Dec 2, 2014 at 7:32 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 12/01/2014 11:27 PM, Prabhakar Lad wrote:
 Hi Hans,

 On Mon, Dec 1, 2014 at 11:00 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 12/01/2014 11:11 AM, Hans Verkuil wrote:
 Hi all,

 Thanks for the patch, review comments are below.

 For the next version I would appreciate if someone can test this driver 
 with
 the latest v4l2-compliance from the v4l-utils git repo. If possible test 
 streaming
 as well (v4l2-compliance -s), ideally with both a sensor and a STD 
 receiver input.
 But that depends on the available hardware of course.

 I'd like to see the v4l2-compliance output. It's always good to have that 
 on
 record.


 On 11/24/2014 02:10 AM, Lad, Prabhakar wrote:
 From: Benoit Parrot bpar...@ti.com

 This patch adds Video Processing Front End (VPFE) driver for
 AM437X family of devices
 Driver supports the following:
 - V4L2 API using MMAP buffer access based on videobuf2 api
 - Asynchronous sensor/decoder sub device registration
 - DT support

 Signed-off-by: Benoit Parrot bpar...@ti.com
 Signed-off-by: Darren Etheridge detheri...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  .../devicetree/bindings/media/ti-am437xx-vpfe.txt  |   61 +
  MAINTAINERS|9 +
  drivers/media/platform/Kconfig |1 +
  drivers/media/platform/Makefile|2 +
  drivers/media/platform/am437x/Kconfig  |   10 +
  drivers/media/platform/am437x/Makefile |2 +
  drivers/media/platform/am437x/vpfe.c   | 2764 
 
  drivers/media/platform/am437x/vpfe.h   |  286 ++
  drivers/media/platform/am437x/vpfe_regs.h  |  140 +
  include/uapi/linux/Kbuild  |1 +
  include/uapi/linux/am437x_vpfe.h   |  122 +
  11 files changed, 3398 insertions(+)

 Can the source names be improved? There are too many 'vpfe' sources.
 Perhaps prefix all with 'am437x-'?

 I did think of it but, dropped it as anyway the source's are present
 in am437x folder, again naming the files am437x-xxx.x didn't make
 me feel good. If you still insists I'll do it.

 Yes, please do. If you look at most other drivers they all have a prefix.

 Another reason is that the media_build system expects unique names.

OK makes sense I'll prefix it with 'am437x-'. probably fixing all the review
comments i'll post a v2 end of the day.

Thanks,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-12-01 Thread Prabhakar Lad
Hi Hans,

On Mon, Dec 1, 2014 at 11:26 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

 On Sunday 30 November 2014 21:30:35 Prabhakar Lad wrote:
 On Sun, Nov 30, 2014 at 9:16 PM, Laurent Pinchart wrote:
  On Sunday 30 November 2014 21:05:50 Prabhakar Lad wrote:
  On Sat, Nov 29, 2014 at 7:12 PM, Laurent Pinchart wrote:
   Hi Prabhakar,
 
  [Snip]
 
  Sure. That's a better choice than removing the config option
  dependency of the fields struct v4l2_subdev.
 
  Decoupling CONFIG_VIDEO_V4L2_SUBDEV_API from the availability of the
  in-kernel pad format and selection rectangles helpers is definitely a
  good idea. I was thinking about decoupling the try format and
  rectangles from v4l2_subdev_fh by creating a kind of configuration store
  structure to store them, and embedding that structure in v4l2_subdev_fh.
  The pad-level operations would then take a pointer to the configuration
  store instead of the v4l2_subdev_fh. Bridge drivers that want to
  implement TRY_FMT based on pad-level operations would create a
  configuration store, use the pad-level operations, and destroy the
  configuration store. The userspace subdev API would use the
  configuration store from the file handle.
 
  are planning to work/post any time soon ? Or are you OK with suggestion
  from Hans ?
 
  I have no plan to work on that myself now, I was hoping you could
  implement it ;-)

 OK will implement it.

 Can you please elaborate a more on this The userspace subdev API would use
 the configuration store from the file handle.

 Basically,

 1. Create a subdev pad configuration store structure to store the formats and
 selection rectangles for each pad.

 2. Embed an instance of that structure in v4l2_subdev_fh.

 3. Modify the subdev pad ops to take a configuration store pointer instead of
 a file handle pointer.

 The userspace API implementation (v4l2-subdev.c) would then pass fh-store to
 the pad operations instead of fh.

 Bridge drivers that need to implement TRY_FMT on top of pad ops would create a
 temporary store (or temporary stores when multiple subsdevs are involved),
 call the pad ops with a pointer to the temporary store to propagate TRY
 formats, destroy the store(s) and return the resulting format.

 Is that clear ?

Yes thank you, I'll soon shoot a RFC version.

Regards,
--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 v7 44/46] virtio_scsi: export to userspace

2014-11-30 Thread Prabhakar Lad
Hi Michael,

Thanks for the patch.

On Sun, Nov 30, 2014 at 3:12 PM, Michael S. Tsirkin  wrote:
> Replace uXX by __uXX and _packed by __attribute((packed))
> as seems to be the norm for userspace headers.
>
> Signed-off-by: Michael S. Tsirkin 
> Acked-by: Paolo Bonzini 

> ---
>  include/uapi/linux/virtio_scsi.h | 74 
> 
>  include/uapi/linux/Kbuild|  1 +

Probably worth making a separate patch for this ? as it doesnt
match with patch description aswel.

Apart from that patch looks good.

Acked-by: Lad, Prabhakar 

Regards,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-30 Thread Prabhakar Lad
Hi Laurent,

On Sun, Nov 30, 2014 at 9:16 PM, Laurent Pinchart
 wrote:
> On Sunday 30 November 2014 21:05:50 Prabhakar Lad wrote:
>> On Sat, Nov 29, 2014 at 7:12 PM, Laurent Pinchart wrote:
>> > Hi Prabhakar,
>>
>> [Snip]
>>
>> >> > Sure. That's a better choice than removing the config option dependency
>> >> > of the fields struct v4l2_subdev.
>> >
>> > Decoupling CONFIG_VIDEO_V4L2_SUBDEV_API from the availability of the
>> > in-kernel pad format and selection rectangles helpers is definitely a
>> > good idea. I was thinking about decoupling the try format and rectangles
>> > from v4l2_subdev_fh by creating a kind of configuration store structure
>> > to store them, and embedding that structure in v4l2_subdev_fh. The
>> > pad-level operations would then take a pointer to the configuration store
>> > instead of the v4l2_subdev_fh. Bridge drivers that want to implement
>> > TRY_FMT based on pad-level operations would create a configuration store,
>> > use the pad-level operations, and destroy the configuration store. The
>> > userspace subdev API would use the configuration store from the file
>> > handle.
>>
>> are planning to work/post any time soon ? Or are you OK with suggestion from
>> Hans ?
>
> I have no plan to work on that myself now, I was hoping you could implement it
> ;-)
>
OK will implement it.

Can you please elaborate a more on this "The userspace subdev API would use
the configuration store from the file  handle."

Thanks,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-30 Thread Prabhakar Lad
Hi Laurent,

On Sat, Nov 29, 2014 at 7:12 PM, Laurent Pinchart
 wrote:
> Hi Prabhakar,
>
[Snip]
>> > Sure. That's a better choice than removing the config option dependency of
>> > the fields struct v4l2_subdev.
>
> Decoupling CONFIG_VIDEO_V4L2_SUBDEV_API from the availability of the in-kernel
> pad format and selection rectangles helpers is definitely a good idea. I was
> thinking about decoupling the try format and rectangles from v4l2_subdev_fh by
> creating a kind of configuration store structure to store them, and embedding
> that structure in v4l2_subdev_fh. The pad-level operations would then take a
> pointer to the configuration store instead of the v4l2_subdev_fh. Bridge
> drivers that want to implement TRY_FMT based on pad-level operations would
> create a configuration store, use the pad-level operations, and destroy the
> configuration store. The userspace subdev API would use the configuration
> store from the file handle.
>
are planning to work/post any time soon ? Or are you OK with suggestion from
Hans ?

Thanks,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-30 Thread Prabhakar Lad
Hi Laurent,

On Sat, Nov 29, 2014 at 7:12 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

[Snip]
  Sure. That's a better choice than removing the config option dependency of
  the fields struct v4l2_subdev.

 Decoupling CONFIG_VIDEO_V4L2_SUBDEV_API from the availability of the in-kernel
 pad format and selection rectangles helpers is definitely a good idea. I was
 thinking about decoupling the try format and rectangles from v4l2_subdev_fh by
 creating a kind of configuration store structure to store them, and embedding
 that structure in v4l2_subdev_fh. The pad-level operations would then take a
 pointer to the configuration store instead of the v4l2_subdev_fh. Bridge
 drivers that want to implement TRY_FMT based on pad-level operations would
 create a configuration store, use the pad-level operations, and destroy the
 configuration store. The userspace subdev API would use the configuration
 store from the file handle.

are planning to work/post any time soon ? Or are you OK with suggestion from
Hans ?

Thanks,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-30 Thread Prabhakar Lad
Hi Laurent,

On Sun, Nov 30, 2014 at 9:16 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 On Sunday 30 November 2014 21:05:50 Prabhakar Lad wrote:
 On Sat, Nov 29, 2014 at 7:12 PM, Laurent Pinchart wrote:
  Hi Prabhakar,

 [Snip]

   Sure. That's a better choice than removing the config option dependency
   of the fields struct v4l2_subdev.
 
  Decoupling CONFIG_VIDEO_V4L2_SUBDEV_API from the availability of the
  in-kernel pad format and selection rectangles helpers is definitely a
  good idea. I was thinking about decoupling the try format and rectangles
  from v4l2_subdev_fh by creating a kind of configuration store structure
  to store them, and embedding that structure in v4l2_subdev_fh. The
  pad-level operations would then take a pointer to the configuration store
  instead of the v4l2_subdev_fh. Bridge drivers that want to implement
  TRY_FMT based on pad-level operations would create a configuration store,
  use the pad-level operations, and destroy the configuration store. The
  userspace subdev API would use the configuration store from the file
  handle.

 are planning to work/post any time soon ? Or are you OK with suggestion from
 Hans ?

 I have no plan to work on that myself now, I was hoping you could implement it
 ;-)

OK will implement it.

Can you please elaborate a more on this The userspace subdev API would use
the configuration store from the file  handle.

Thanks,
--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 v7 44/46] virtio_scsi: export to userspace

2014-11-30 Thread Prabhakar Lad
Hi Michael,

Thanks for the patch.

On Sun, Nov 30, 2014 at 3:12 PM, Michael S. Tsirkin m...@redhat.com wrote:
 Replace uXX by __uXX and _packed by __attribute((packed))
 as seems to be the norm for userspace headers.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 Acked-by: Paolo Bonzini pbonz...@redhat.com

 ---
  include/uapi/linux/virtio_scsi.h | 74 
 
  include/uapi/linux/Kbuild|  1 +

Probably worth making a separate patch for this ? as it doesnt
match with patch description aswel.

Apart from that patch looks good.

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Regards,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-29 Thread Prabhakar Lad
Hi Laurent,

On Tue, Nov 18, 2014 at 6:07 PM, Sakari Ailus  wrote:
> Hi Hans and Prabhakar,
>
> On Tue, Nov 18, 2014 at 10:39:24AM +0100, Hans Verkuil wrote:
>> On 11/17/14 11:41, Lad, Prabhakar wrote:
>> > this patch removes the guard CONFIG_VIDEO_V4L2_SUBDEV_API
>> > for v4l2_subdev_get_try_*() functions.
>> > In cases where a subdev using v4l2_subdev_get_try_*() calls
>> > internally and the bridge using subdev pad ops which is
>> > not MC aware forces to select MEDIA_CONTROLLER, as
>> > VIDEO_V4L2_SUBDEV_API is dependent on it.
>> >
>> > Signed-off-by: Lad, Prabhakar 
>> > ---
>> >  include/media/v4l2-subdev.h | 2 --
>> >  1 file changed, 2 deletions(-)
>> >
>> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> > index 5860292..076ca11 100644
>> > --- a/include/media/v4l2-subdev.h
>> > +++ b/include/media/v4l2-subdev.h
>> > @@ -642,7 +642,6 @@ struct v4l2_subdev_fh {
>> >  #define to_v4l2_subdev_fh(fh)  \
>> > container_of(fh, struct v4l2_subdev_fh, vfh)
>> >
>> > -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>> >  #define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) 
>> >  \
>> > static inline struct rtype *\
>> > v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh,   \
>> > @@ -656,7 +655,6 @@ struct v4l2_subdev_fh {
>> >  __V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
>> >  __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_crop)
>> >  __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
>> > -#endif
>> >
>> >  extern const struct v4l2_file_operations v4l2_subdev_fops;
>> >
>> >
>>
>> The problem is that v4l2_subdev_get_try_*() needs a v4l2_subdev_fh which
>> you don't have if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined. So I don't
>> see how removing the guards help with that.
>>
>> What can be done is that if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined,
>> then these functions return NULL.
>
> Sure. That's a better choice than removing the config option dependency of
> the fields struct v4l2_subdev.
>
>> BTW, one patch I will very happily accept is one where the 
>> __V4L2_SUBDEV_MK_GET_TRY
>> is removed and these three try functions are just written as proper
>> static inlines. I find it very obfuscated code.
>
> I originally wrote them like that in order to avoid writing essentially the
> same code three times over. If there will be more targets, the same repeats
> further, should one write those functions open for all different macro
> arguments. That's why it was a macro to begin with.
>
>> In addition, because it is a macro you won't find the function definitions
>> if you grep on the function name.
>
> True as well. You could simply change the macro to include the full function
> name. This was not suggested in review back then AFAIR.
>
>> But any functional changes here need to be Acked by Laurent first.
>

How do you want me to proceed on this ?

Thanks,
--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 v3] media: usb: uvc: use vb2_ops_wait_prepare/finish helper

2014-11-29 Thread Prabhakar Lad
Hi Laurent,

On Sat, Nov 29, 2014 at 6:11 PM, Laurent Pinchart
 wrote:
> Hi Prabhakar,
>
> On Friday 28 November 2014 17:06:52 Prabhakar Lad wrote:
>> On Thu, Nov 27, 2014 at 9:32 PM, Laurent Pinchart wrote:
>> > Hi Prabhakar,
>>
>> [Snip]
>>
>> >> + queue->queue.lock = >mutex;
>> >
>> > I'm a bit concerned that this would introduce future breakages. Setting
>> > the queue.lock pointer enables locking in all vb2_fop_* and vb2_ops_wait_*
>> > functions. The uvcvideo driver isn't ready for that, but doesn't use the
>> > vb2_fop_* functions yet, so that's not an issue. However, in the future,
>> > videobuf2 might use the lock in more places, including functions used by
>> > the uvcvideo driver. This could then cause breakages.
>>
>> Even if in future if videobuf2 uses this lock it would be in helpers mostly,
>> so any way it doesn’t harm :)
>
> My concern is about vb2 starting using the lock in existing helpers used by
> the uvcvideo driver. I suppose we can deal with it later.
>
>> > It would be better to completely convert the uvcvideo driver to the
>> > vb2_fop_* functions if we want to use vb2_ops_*. I'm not sure how complex
>> > that would be though, and whether it would be possible while still
>> > keeping the fine-grained locking implemented by the uvcvideo driver. Do
>> > you think it should be attempted ?
>>
>> mmap & poll should be fairly simple, looks like open & release cannot be
>> dropped as it does some usb_autopm_get/put_interface() calls which I am not
>> aware of.
>
> I've looked at that, and there's a race condition in vb2_fop_poll() (for which
> I've already sent a patch) and possible in vb2_mmap() (raised the issue on
> #v4l today) as well that need to be fixed first.
>
> Anyway, for this patch
>
> Acked-by: Laurent Pinchart 
>
> Have you tested it by the way ?
>
I have just compile tested it.

> Should I take it in my tree or will you send a pull request for the whole
> series ?
>
Probably you can pick this up via your tree.

Thanks,
--Prabhakar Lad

>> >>   ret = vb2_queue_init(>queue);
>> >>   if (ret)
>> >>   return ret;
>
> --
> 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 v3] media: usb: uvc: use vb2_ops_wait_prepare/finish helper

2014-11-29 Thread Prabhakar Lad
Hi Laurent,

On Sat, Nov 29, 2014 at 6:11 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

 On Friday 28 November 2014 17:06:52 Prabhakar Lad wrote:
 On Thu, Nov 27, 2014 at 9:32 PM, Laurent Pinchart wrote:
  Hi Prabhakar,

 [Snip]

  + queue-queue.lock = queue-mutex;
 
  I'm a bit concerned that this would introduce future breakages. Setting
  the queue.lock pointer enables locking in all vb2_fop_* and vb2_ops_wait_*
  functions. The uvcvideo driver isn't ready for that, but doesn't use the
  vb2_fop_* functions yet, so that's not an issue. However, in the future,
  videobuf2 might use the lock in more places, including functions used by
  the uvcvideo driver. This could then cause breakages.

 Even if in future if videobuf2 uses this lock it would be in helpers mostly,
 so any way it doesn’t harm :)

 My concern is about vb2 starting using the lock in existing helpers used by
 the uvcvideo driver. I suppose we can deal with it later.

  It would be better to completely convert the uvcvideo driver to the
  vb2_fop_* functions if we want to use vb2_ops_*. I'm not sure how complex
  that would be though, and whether it would be possible while still
  keeping the fine-grained locking implemented by the uvcvideo driver. Do
  you think it should be attempted ?

 mmap  poll should be fairly simple, looks like open  release cannot be
 dropped as it does some usb_autopm_get/put_interface() calls which I am not
 aware of.

 I've looked at that, and there's a race condition in vb2_fop_poll() (for which
 I've already sent a patch) and possible in vb2_mmap() (raised the issue on
 #v4l today) as well that need to be fixed first.

 Anyway, for this patch

 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

 Have you tested it by the way ?

I have just compile tested it.

 Should I take it in my tree or will you send a pull request for the whole
 series ?

Probably you can pick this up via your tree.

Thanks,
--Prabhakar Lad

ret = vb2_queue_init(queue-queue);
if (ret)
return ret;

 --
 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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-29 Thread Prabhakar Lad
Hi Laurent,

On Tue, Nov 18, 2014 at 6:07 PM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Hi Hans and Prabhakar,

 On Tue, Nov 18, 2014 at 10:39:24AM +0100, Hans Verkuil wrote:
 On 11/17/14 11:41, Lad, Prabhakar wrote:
  this patch removes the guard CONFIG_VIDEO_V4L2_SUBDEV_API
  for v4l2_subdev_get_try_*() functions.
  In cases where a subdev using v4l2_subdev_get_try_*() calls
  internally and the bridge using subdev pad ops which is
  not MC aware forces to select MEDIA_CONTROLLER, as
  VIDEO_V4L2_SUBDEV_API is dependent on it.
 
  Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
  ---
   include/media/v4l2-subdev.h | 2 --
   1 file changed, 2 deletions(-)
 
  diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
  index 5860292..076ca11 100644
  --- a/include/media/v4l2-subdev.h
  +++ b/include/media/v4l2-subdev.h
  @@ -642,7 +642,6 @@ struct v4l2_subdev_fh {
   #define to_v4l2_subdev_fh(fh)  \
  container_of(fh, struct v4l2_subdev_fh, vfh)
 
  -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
   #define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) 
   \
  static inline struct rtype *\
  v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh,   \
  @@ -656,7 +655,6 @@ struct v4l2_subdev_fh {
   __V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
   __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_crop)
   __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
  -#endif
 
   extern const struct v4l2_file_operations v4l2_subdev_fops;
 
 

 The problem is that v4l2_subdev_get_try_*() needs a v4l2_subdev_fh which
 you don't have if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined. So I don't
 see how removing the guards help with that.

 What can be done is that if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined,
 then these functions return NULL.

 Sure. That's a better choice than removing the config option dependency of
 the fields struct v4l2_subdev.

 BTW, one patch I will very happily accept is one where the 
 __V4L2_SUBDEV_MK_GET_TRY
 is removed and these three try functions are just written as proper
 static inlines. I find it very obfuscated code.

 I originally wrote them like that in order to avoid writing essentially the
 same code three times over. If there will be more targets, the same repeats
 further, should one write those functions open for all different macro
 arguments. That's why it was a macro to begin with.

 In addition, because it is a macro you won't find the function definitions
 if you grep on the function name.

 True as well. You could simply change the macro to include the full function
 name. This was not suggested in review back then AFAIR.

 But any functional changes here need to be Acked by Laurent first.


How do you want me to proceed on this ?

Thanks,
--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 v3] media: usb: uvc: use vb2_ops_wait_prepare/finish helper

2014-11-28 Thread Prabhakar Lad
Hi Laurent,

Thanks for the review.

On Thu, Nov 27, 2014 at 9:32 PM, Laurent Pinchart
 wrote:
> Hi Prabhakar,
[Snip]
>>
>> + queue->queue.lock = >mutex;
>
> I'm a bit concerned that this would introduce future breakages. Setting the
> queue.lock pointer enables locking in all vb2_fop_* and vb2_ops_wait_*
> functions. The uvcvideo driver isn't ready for that, but doesn't use the
> vb2_fop_* functions yet, so that's not an issue. However, in the future,
> videobuf2 might use the lock in more places, including functions used by the
> uvcvideo driver. This could then cause breakages.
>
Even if in future if videobuf2 uses this lock it would be in helpers mostly,
so any way it doesn’t harm :)

> It would be better to completely convert the uvcvideo driver to the vb2_fop_*
> functions if we want to use vb2_ops_*. I'm not sure how complex that would be
> though, and whether it would be possible while still keeping the fine-grained
> locking implemented by the uvcvideo driver. Do you think it should be
> attempted ?
>
mmap & poll should be fairly simple, looks like open & release cannot be dropped
as it does some usb_autopm_get/put_interface() calls which I am not aware of.

Thanks,
--Prabhakar Lad

>>   ret = vb2_queue_init(>queue);
>>   if (ret)
>>   return ret;
>
> --
> 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 v3] media: usb: uvc: use vb2_ops_wait_prepare/finish helper

2014-11-28 Thread Prabhakar Lad
Hi Laurent,

Thanks for the review.

On Thu, Nov 27, 2014 at 9:32 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,
[Snip]

 + queue-queue.lock = queue-mutex;

 I'm a bit concerned that this would introduce future breakages. Setting the
 queue.lock pointer enables locking in all vb2_fop_* and vb2_ops_wait_*
 functions. The uvcvideo driver isn't ready for that, but doesn't use the
 vb2_fop_* functions yet, so that's not an issue. However, in the future,
 videobuf2 might use the lock in more places, including functions used by the
 uvcvideo driver. This could then cause breakages.

Even if in future if videobuf2 uses this lock it would be in helpers mostly,
so any way it doesn’t harm :)

 It would be better to completely convert the uvcvideo driver to the vb2_fop_*
 functions if we want to use vb2_ops_*. I'm not sure how complex that would be
 though, and whether it would be possible while still keeping the fine-grained
 locking implemented by the uvcvideo driver. Do you think it should be
 attempted ?

mmap  poll should be fairly simple, looks like open  release cannot be dropped
as it does some usb_autopm_get/put_interface() calls which I am not aware of.

Thanks,
--Prabhakar Lad

   ret = vb2_queue_init(queue-queue);
   if (ret)
   return ret;

 --
 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 v2 11/11] media: usb: uvc: use vb2_ops_wait_prepare/finish helper

2014-11-26 Thread Prabhakar Lad
Hi Laurent,

Thanks for the review.

On Wed, Nov 26, 2014 at 10:59 PM, Laurent Pinchart
 wrote:
> Hi Prabhakar,
>
> Thank you for the patch.
>
> On Wednesday 26 November 2014 22:42:34 Lad, Prabhakar wrote:
>> This patch drops driver specific wait_prepare() and
>> wait_finish() callbacks from vb2_ops and instead uses
>> the the helpers vb2_ops_wait_prepare/finish() provided
>> by the vb2 core, the lock member of the queue needs
>> to be initalized to a mutex so that vb2 helpers
>> vb2_ops_wait_prepare/finish() can make use of it.
>
> The queue lock field isn't initialized by the uvcvideo driver, so you can't
> use vb2_ops_wait_prepare|finish().
>
Oops not sure what happened here I just took the patch from [1]
and added commit message. anyway will post a single patch v3.

[1] https://patchwork.kernel.org/patch/5327451/

Thanks,
--Prabhakar Lad

>> Signed-off-by: Lad, Prabhakar 
>> Cc: Laurent Pinchart 
>> ---
>>  drivers/media/usb/uvc/uvc_queue.c | 18 ++
>>  1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/media/usb/uvc/uvc_queue.c
>> b/drivers/media/usb/uvc/uvc_queue.c index cc96072..64147b5 100644
>> --- a/drivers/media/usb/uvc/uvc_queue.c
>> +++ b/drivers/media/usb/uvc/uvc_queue.c
>> @@ -143,20 +143,6 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
>>   uvc_video_clock_update(stream, >v4l2_buf, buf);
>>  }
>>
>> -static void uvc_wait_prepare(struct vb2_queue *vq)
>> -{
>> - struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>> -
>> - mutex_unlock(>mutex);
>> -}
>> -
>> -static void uvc_wait_finish(struct vb2_queue *vq)
>> -{
>> - struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>> -
>> - mutex_lock(>mutex);
>> -}
>> -
>>  static int uvc_start_streaming(struct vb2_queue *vq, unsigned int count)
>>  {
>>   struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
>> @@ -195,8 +181,8 @@ static struct vb2_ops uvc_queue_qops = {
>>   .buf_prepare = uvc_buffer_prepare,
>>   .buf_queue = uvc_buffer_queue,
>>   .buf_finish = uvc_buffer_finish,
>> - .wait_prepare = uvc_wait_prepare,
>> - .wait_finish = uvc_wait_finish,
>> + .wait_prepare = vb2_ops_wait_prepare,
>> + .wait_finish = vb2_ops_wait_finish,
>>   .start_streaming = uvc_start_streaming,
>>   .stop_streaming = uvc_stop_streaming,
>>  };
>
> --
> 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 v2 11/11] media: usb: uvc: use vb2_ops_wait_prepare/finish helper

2014-11-26 Thread Prabhakar Lad
Hi Laurent,

Thanks for the review.

On Wed, Nov 26, 2014 at 10:59 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

 Thank you for the patch.

 On Wednesday 26 November 2014 22:42:34 Lad, Prabhakar wrote:
 This patch drops driver specific wait_prepare() and
 wait_finish() callbacks from vb2_ops and instead uses
 the the helpers vb2_ops_wait_prepare/finish() provided
 by the vb2 core, the lock member of the queue needs
 to be initalized to a mutex so that vb2 helpers
 vb2_ops_wait_prepare/finish() can make use of it.

 The queue lock field isn't initialized by the uvcvideo driver, so you can't
 use vb2_ops_wait_prepare|finish().

Oops not sure what happened here I just took the patch from [1]
and added commit message. anyway will post a single patch v3.

[1] https://patchwork.kernel.org/patch/5327451/

Thanks,
--Prabhakar Lad

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 Cc: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  drivers/media/usb/uvc/uvc_queue.c | 18 ++
  1 file changed, 2 insertions(+), 16 deletions(-)

 diff --git a/drivers/media/usb/uvc/uvc_queue.c
 b/drivers/media/usb/uvc/uvc_queue.c index cc96072..64147b5 100644
 --- a/drivers/media/usb/uvc/uvc_queue.c
 +++ b/drivers/media/usb/uvc/uvc_queue.c
 @@ -143,20 +143,6 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
   uvc_video_clock_update(stream, vb-v4l2_buf, buf);
  }

 -static void uvc_wait_prepare(struct vb2_queue *vq)
 -{
 - struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
 -
 - mutex_unlock(queue-mutex);
 -}
 -
 -static void uvc_wait_finish(struct vb2_queue *vq)
 -{
 - struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
 -
 - mutex_lock(queue-mutex);
 -}
 -
  static int uvc_start_streaming(struct vb2_queue *vq, unsigned int count)
  {
   struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
 @@ -195,8 +181,8 @@ static struct vb2_ops uvc_queue_qops = {
   .buf_prepare = uvc_buffer_prepare,
   .buf_queue = uvc_buffer_queue,
   .buf_finish = uvc_buffer_finish,
 - .wait_prepare = uvc_wait_prepare,
 - .wait_finish = uvc_wait_finish,
 + .wait_prepare = vb2_ops_wait_prepare,
 + .wait_finish = vb2_ops_wait_finish,
   .start_streaming = uvc_start_streaming,
   .stop_streaming = uvc_stop_streaming,
  };

 --
 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] media: exynos-gsc: fix build warning

2014-11-25 Thread Prabhakar Lad
Hi,

On Tue, Nov 25, 2014 at 3:18 PM, Prabhakar Lad
 wrote:
> Hi Mauro,
>
> On Tue, Nov 25, 2014 at 3:04 PM, Mauro Carvalho Chehab
>  wrote:
>> Em Tue, 18 Nov 2014 10:57:48 +
> [Snip]
>>
>> -static u32 get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index)
>> +static int get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index, u32 
>> *ret_addr)
>>  {
>> if (frm->addr.y == addr) {
>> *index = 0;
>> -   return frm->addr.y;
>> +   *ret_addr = frm->addr.y;
>> } else if (frm->addr.cb == addr) {
>> *index = 1;
>> -   return frm->addr.cb;
>> +   *ret_addr = frm->addr.cb;
>> } else if (frm->addr.cr == addr) {
>> *index = 2;
>> -   return frm->addr.cr;
>> +   *ret_addr = frm->addr.cr;
>> } else {
>> pr_err("Plane address is wrong");
>> return -EINVAL;
>>     }
>> +   return 0;
> the control wont reach here! may be you can remove the complete else
> part outside ?
>
Ah my bad :(, I missread 'ret_addr' to return.

Thanks,
--Prabhakar Lad

> with that change,
>
> Reported-by: Lad, Prabhakar 
> Acked-by: Lad, Prabhakar 
>
> Thanks,
> --Prabhakar Lad
>
>>  }
>>
>>  void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
>> @@ -352,9 +353,11 @@ void gsc_set_prefbuf(struct gsc_dev *gsc, struct 
>> gsc_frame *frm)
>> u32 t_min, t_max;
>>
>> t_min = min3(frm->addr.y, frm->addr.cb, frm->addr.cr);
>> -   low_addr = get_plane_info(frm, t_min, _plane);
>> +   if (get_plane_info(frm, t_min, _plane, _addr))
>> +   return;
>> t_max = max3(frm->addr.y, frm->addr.cb, frm->addr.cr);
>> -   high_addr = get_plane_info(frm, t_max, _plane);
>> +   if (get_plane_info(frm, t_max, _plane, _addr))
>> +   return;
>>
>> mid_plane = 3 - (low_plane + high_plane);
>> if (mid_plane == 0)
--
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] media: exynos-gsc: fix build warning

2014-11-25 Thread Prabhakar Lad
Hi Mauro,

On Tue, Nov 25, 2014 at 3:04 PM, Mauro Carvalho Chehab
 wrote:
> Em Tue, 18 Nov 2014 10:57:48 +
[Snip]
>
> -static u32 get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index)
> +static int get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index, u32 
> *ret_addr)
>  {
> if (frm->addr.y == addr) {
> *index = 0;
> -   return frm->addr.y;
> +   *ret_addr = frm->addr.y;
> } else if (frm->addr.cb == addr) {
> *index = 1;
> -   return frm->addr.cb;
> +   *ret_addr = frm->addr.cb;
> } else if (frm->addr.cr == addr) {
> *index = 2;
> -   return frm->addr.cr;
> +   *ret_addr = frm->addr.cr;
> } else {
> pr_err("Plane address is wrong");
> return -EINVAL;
> }
> +   return 0;
the control wont reach here! may be you can remove the complete else
part outside ?

with that change,

Reported-by: Lad, Prabhakar 
Acked-by: Lad, Prabhakar 

Thanks,
--Prabhakar Lad

>  }
>
>  void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
> @@ -352,9 +353,11 @@ void gsc_set_prefbuf(struct gsc_dev *gsc, struct 
> gsc_frame *frm)
> u32 t_min, t_max;
>
> t_min = min3(frm->addr.y, frm->addr.cb, frm->addr.cr);
> -   low_addr = get_plane_info(frm, t_min, _plane);
> +   if (get_plane_info(frm, t_min, _plane, _addr))
> +   return;
> t_max = max3(frm->addr.y, frm->addr.cb, frm->addr.cr);
> -   high_addr = get_plane_info(frm, t_max, _plane);
> +   if (get_plane_info(frm, t_max, _plane, _addr))
> +   return;
>
> mid_plane = 3 - (low_plane + high_plane);
> if (mid_plane == 0)
--
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] media: exynos-gsc: fix build warning

2014-11-25 Thread Prabhakar Lad
Hi Mauro,

On Tue, Nov 25, 2014 at 3:04 PM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 Em Tue, 18 Nov 2014 10:57:48 +
[Snip]

 -static u32 get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index)
 +static int get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index, u32 
 *ret_addr)
  {
 if (frm-addr.y == addr) {
 *index = 0;
 -   return frm-addr.y;
 +   *ret_addr = frm-addr.y;
 } else if (frm-addr.cb == addr) {
 *index = 1;
 -   return frm-addr.cb;
 +   *ret_addr = frm-addr.cb;
 } else if (frm-addr.cr == addr) {
 *index = 2;
 -   return frm-addr.cr;
 +   *ret_addr = frm-addr.cr;
 } else {
 pr_err(Plane address is wrong);
 return -EINVAL;
 }
 +   return 0;
the control wont reach here! may be you can remove the complete else
part outside ?

with that change,

Reported-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--Prabhakar Lad

  }

  void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
 @@ -352,9 +353,11 @@ void gsc_set_prefbuf(struct gsc_dev *gsc, struct 
 gsc_frame *frm)
 u32 t_min, t_max;

 t_min = min3(frm-addr.y, frm-addr.cb, frm-addr.cr);
 -   low_addr = get_plane_info(frm, t_min, low_plane);
 +   if (get_plane_info(frm, t_min, low_plane, low_addr))
 +   return;
 t_max = max3(frm-addr.y, frm-addr.cb, frm-addr.cr);
 -   high_addr = get_plane_info(frm, t_max, high_plane);
 +   if (get_plane_info(frm, t_max, high_plane, high_addr))
 +   return;

 mid_plane = 3 - (low_plane + high_plane);
 if (mid_plane == 0)
--
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] media: exynos-gsc: fix build warning

2014-11-25 Thread Prabhakar Lad
Hi,

On Tue, Nov 25, 2014 at 3:18 PM, Prabhakar Lad
prabhakar.cse...@gmail.com wrote:
 Hi Mauro,

 On Tue, Nov 25, 2014 at 3:04 PM, Mauro Carvalho Chehab
 mche...@osg.samsung.com wrote:
 Em Tue, 18 Nov 2014 10:57:48 +
 [Snip]

 -static u32 get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index)
 +static int get_plane_info(struct gsc_frame *frm, u32 addr, u32 *index, u32 
 *ret_addr)
  {
 if (frm-addr.y == addr) {
 *index = 0;
 -   return frm-addr.y;
 +   *ret_addr = frm-addr.y;
 } else if (frm-addr.cb == addr) {
 *index = 1;
 -   return frm-addr.cb;
 +   *ret_addr = frm-addr.cb;
 } else if (frm-addr.cr == addr) {
 *index = 2;
 -   return frm-addr.cr;
 +   *ret_addr = frm-addr.cr;
 } else {
 pr_err(Plane address is wrong);
 return -EINVAL;
 }
 +   return 0;
 the control wont reach here! may be you can remove the complete else
 part outside ?

Ah my bad :(, I missread 'ret_addr' to return.

Thanks,
--Prabhakar Lad

 with that change,

 Reported-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

 Thanks,
 --Prabhakar Lad

  }

  void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
 @@ -352,9 +353,11 @@ void gsc_set_prefbuf(struct gsc_dev *gsc, struct 
 gsc_frame *frm)
 u32 t_min, t_max;

 t_min = min3(frm-addr.y, frm-addr.cb, frm-addr.cr);
 -   low_addr = get_plane_info(frm, t_min, low_plane);
 +   if (get_plane_info(frm, t_min, low_plane, low_addr))
 +   return;
 t_max = max3(frm-addr.y, frm-addr.cb, frm-addr.cr);
 -   high_addr = get_plane_info(frm, t_max, high_plane);
 +   if (get_plane_info(frm, t_max, high_plane, high_addr))
 +   return;

 mid_plane = 3 - (low_plane + high_plane);
 if (mid_plane == 0)
--
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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-18 Thread Prabhakar Lad
Hi Hans,

On Tue, Nov 18, 2014 at 9:39 AM, Hans Verkuil  wrote:
> On 11/17/14 11:41, Lad, Prabhakar wrote:
>> this patch removes the guard CONFIG_VIDEO_V4L2_SUBDEV_API
>> for v4l2_subdev_get_try_*() functions.
>> In cases where a subdev using v4l2_subdev_get_try_*() calls
>> internally and the bridge using subdev pad ops which is
>> not MC aware forces to select MEDIA_CONTROLLER, as
>> VIDEO_V4L2_SUBDEV_API is dependent on it.
>>
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>  include/media/v4l2-subdev.h | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index 5860292..076ca11 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -642,7 +642,6 @@ struct v4l2_subdev_fh {
>>  #define to_v4l2_subdev_fh(fh)\
>>   container_of(fh, struct v4l2_subdev_fh, vfh)
>>
>> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>>  #define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name)   
>>  \
>>   static inline struct rtype *\
>>   v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh,   \
>> @@ -656,7 +655,6 @@ struct v4l2_subdev_fh {
>>  __V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
>>  __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_crop)
>>  __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
>> -#endif
>>
>>  extern const struct v4l2_file_operations v4l2_subdev_fops;
>>
>>
>
> The problem is that v4l2_subdev_get_try_*() needs a v4l2_subdev_fh which
> you don't have if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined. So I don't
> see how removing the guards help with that.
>
Yes

> What can be done is that if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined,
> then these functions return NULL.
>
exactly.

> BTW, one patch I will very happily accept is one where the 
> __V4L2_SUBDEV_MK_GET_TRY
> is removed and these three try functions are just written as proper
> static inlines. I find it very obfuscated code.
>
the functions were initially inline itself which were changes into
macro's later.

> In addition, because it is a macro you won't find the function definitions
> if you grep on the function name.
>
> But any functional changes here need to be Acked by Laurent first.
>
Yes, ill probably wait until Laurent is back from his holidays.

Thanks,
--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 06/12] media: marvell-ccic: use vb2_ops_wait_prepare/finish helper

2014-11-18 Thread Prabhakar Lad
Hi Jonathan,

On Tue, Nov 18, 2014 at 1:03 PM, Jonathan Corbet  wrote:
> On Tue, 18 Nov 2014 11:23:35 +
> "Lad, Prabhakar"  wrote:
>
>>  drivers/media/platform/marvell-ccic/mcam-core.c | 29 
>> +
>>  1 file changed, 5 insertions(+), 24 deletions(-)
>
> So I'm not convinced that this patch improves things; it moves a tiny bit
> of code into another file where anybody reading the driver will have to
> go look to see what's going on.  But I guess it doesn't really make
> things worse either; I won't try to stand in its way.  It would be nice
> to see a real changelog on the patch, though.
>
Sorry there is no movement of code to other file.  And I dont see any
reason why anybody reading will go haywire its a standard v4l2 thing.
The subject explains it all, If you still want me to elaborate I can
post a v2.

Thanks,
--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 06/12] media: marvell-ccic: use vb2_ops_wait_prepare/finish helper

2014-11-18 Thread Prabhakar Lad
Hi Jonathan,

On Tue, Nov 18, 2014 at 1:03 PM, Jonathan Corbet cor...@lwn.net wrote:
 On Tue, 18 Nov 2014 11:23:35 +
 Lad, Prabhakar prabhakar.cse...@gmail.com wrote:

  drivers/media/platform/marvell-ccic/mcam-core.c | 29 
 +
  1 file changed, 5 insertions(+), 24 deletions(-)

 So I'm not convinced that this patch improves things; it moves a tiny bit
 of code into another file where anybody reading the driver will have to
 go look to see what's going on.  But I guess it doesn't really make
 things worse either; I won't try to stand in its way.  It would be nice
 to see a real changelog on the patch, though.

Sorry there is no movement of code to other file.  And I dont see any
reason why anybody reading will go haywire its a standard v4l2 thing.
The subject explains it all, If you still want me to elaborate I can
post a v2.

Thanks,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-18 Thread Prabhakar Lad
Hi Hans,

On Tue, Nov 18, 2014 at 9:39 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 11/17/14 11:41, Lad, Prabhakar wrote:
 this patch removes the guard CONFIG_VIDEO_V4L2_SUBDEV_API
 for v4l2_subdev_get_try_*() functions.
 In cases where a subdev using v4l2_subdev_get_try_*() calls
 internally and the bridge using subdev pad ops which is
 not MC aware forces to select MEDIA_CONTROLLER, as
 VIDEO_V4L2_SUBDEV_API is dependent on it.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  include/media/v4l2-subdev.h | 2 --
  1 file changed, 2 deletions(-)

 diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
 index 5860292..076ca11 100644
 --- a/include/media/v4l2-subdev.h
 +++ b/include/media/v4l2-subdev.h
 @@ -642,7 +642,6 @@ struct v4l2_subdev_fh {
  #define to_v4l2_subdev_fh(fh)\
   container_of(fh, struct v4l2_subdev_fh, vfh)

 -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
  #define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name)   
  \
   static inline struct rtype *\
   v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh,   \
 @@ -656,7 +655,6 @@ struct v4l2_subdev_fh {
  __V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
  __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_crop)
  __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
 -#endif

  extern const struct v4l2_file_operations v4l2_subdev_fops;



 The problem is that v4l2_subdev_get_try_*() needs a v4l2_subdev_fh which
 you don't have if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined. So I don't
 see how removing the guards help with that.

Yes

 What can be done is that if CONFIG_VIDEO_V4L2_SUBDEV_API is not defined,
 then these functions return NULL.

exactly.

 BTW, one patch I will very happily accept is one where the 
 __V4L2_SUBDEV_MK_GET_TRY
 is removed and these three try functions are just written as proper
 static inlines. I find it very obfuscated code.

the functions were initially inline itself which were changes into
macro's later.

 In addition, because it is a macro you won't find the function definitions
 if you grep on the function name.

 But any functional changes here need to be Acked by Laurent first.

Yes, ill probably wait until Laurent is back from his holidays.

Thanks,
--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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-17 Thread Prabhakar Lad
Hi Sakari,

On Mon, Nov 17, 2014 at 10:53 AM, Sakari Ailus
 wrote:
> Hi Prabhakar,
>
> Thank you for the patch.
>
> Lad, Prabhakar wrote:
>>
>> this patch removes the guard CONFIG_VIDEO_V4L2_SUBDEV_API
>> for v4l2_subdev_get_try_*() functions.
>> In cases where a subdev using v4l2_subdev_get_try_*() calls
>> internally and the bridge using subdev pad ops which is
>> not MC aware forces to select MEDIA_CONTROLLER, as
>> VIDEO_V4L2_SUBDEV_API is dependent on it.
>>
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>   include/media/v4l2-subdev.h | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
>> index 5860292..076ca11 100644
>> --- a/include/media/v4l2-subdev.h
>> +++ b/include/media/v4l2-subdev.h
>> @@ -642,7 +642,6 @@ struct v4l2_subdev_fh {
>>   #define to_v4l2_subdev_fh(fh) \
>> container_of(fh, struct v4l2_subdev_fh, vfh)
>>
>> -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>
>
> Wouldn't you need to drop these from struct v4l2_subdev_fh as well? The code
> won't compile if the fields aren't there.
>
Ah missed it, thanks for the catch!

Thanks,
--Prabhakar Lad

>>   #define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) \
>> static inline struct rtype *\
>> v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh,   \
>> @@ -656,7 +655,6 @@ struct v4l2_subdev_fh {
>>   __V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
>>   __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_crop)
>>   __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
>> -#endif
>>
>>   extern const struct v4l2_file_operations v4l2_subdev_fops;
>>
>>
>
> --
> Kind regards,
>
> Sakari Ailus
> sakari.ai...@linux.intel.com
--
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] media: v4l2-subdev.h: drop the guard CONFIG_VIDEO_V4L2_SUBDEV_API for v4l2_subdev_get_try_*()

2014-11-17 Thread Prabhakar Lad
Hi Sakari,

On Mon, Nov 17, 2014 at 10:53 AM, Sakari Ailus
sakari.ai...@linux.intel.com wrote:
 Hi Prabhakar,

 Thank you for the patch.

 Lad, Prabhakar wrote:

 this patch removes the guard CONFIG_VIDEO_V4L2_SUBDEV_API
 for v4l2_subdev_get_try_*() functions.
 In cases where a subdev using v4l2_subdev_get_try_*() calls
 internally and the bridge using subdev pad ops which is
 not MC aware forces to select MEDIA_CONTROLLER, as
 VIDEO_V4L2_SUBDEV_API is dependent on it.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
   include/media/v4l2-subdev.h | 2 --
   1 file changed, 2 deletions(-)

 diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
 index 5860292..076ca11 100644
 --- a/include/media/v4l2-subdev.h
 +++ b/include/media/v4l2-subdev.h
 @@ -642,7 +642,6 @@ struct v4l2_subdev_fh {
   #define to_v4l2_subdev_fh(fh) \
 container_of(fh, struct v4l2_subdev_fh, vfh)

 -#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)


 Wouldn't you need to drop these from struct v4l2_subdev_fh as well? The code
 won't compile if the fields aren't there.

Ah missed it, thanks for the catch!

Thanks,
--Prabhakar Lad

   #define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) \
 static inline struct rtype *\
 v4l2_subdev_get_try_##fun_name(struct v4l2_subdev_fh *fh,   \
 @@ -656,7 +655,6 @@ struct v4l2_subdev_fh {
   __V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, format, try_fmt)
   __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, crop, try_crop)
   __V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, compose, try_compose)
 -#endif

   extern const struct v4l2_file_operations v4l2_subdev_fops;



 --
 Kind regards,

 Sakari Ailus
 sakari.ai...@linux.intel.com
--
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 v3 04/10] [media] i2c: Make use of media_bus_format enum

2014-11-08 Thread Prabhakar Lad
On Fri, Nov 7, 2014 at 2:07 PM, Boris Brezillon
 wrote:
> In order to have subsytem agnostic media bus format definitions we've
> moved media bus definitions to include/uapi/linux/media-bus-format.h and
> prefixed values with MEDIA_BUS_FMT instead of V4L2_MBUS_FMT.
>
> Replace all references to the old definitions in i2c drivers.
>
> Signed-off-by: Boris Brezillon 
> ---
[Snip]
>  drivers/media/i2c/mt9p031.c   |  8 ++--
[Snip]
>  drivers/media/i2c/tvp514x.c   | 12 +++---
>  drivers/media/i2c/tvp7002.c   | 10 ++---

For all the above,

Acked-by: Lad, Prabhakar 

Thanks,
--Prabhakar Lad

>  drivers/media/i2c/vs6624.c| 18 
>  46 files changed, 382 insertions(+), 382 deletions(-)
>
> diff --git a/drivers/media/i2c/adv7170.c b/drivers/media/i2c/adv7170.c
> index 04bb297..40a1a95 100644
> --- a/drivers/media/i2c/adv7170.c
> +++ b/drivers/media/i2c/adv7170.c
> @@ -63,9 +63,9 @@ static inline struct adv7170 *to_adv7170(struct v4l2_subdev 
> *sd)
>
>  static char *inputs[] = { "pass_through", "play_back" };
>
> -static enum v4l2_mbus_pixelcode adv7170_codes[] = {
> -   V4L2_MBUS_FMT_UYVY8_2X8,
> -   V4L2_MBUS_FMT_UYVY8_1X16,
> +static u32 adv7170_codes[] = {
> +   MEDIA_BUS_FMT_UYVY8_2X8,
> +   MEDIA_BUS_FMT_UYVY8_1X16,
>  };
>
>  /* --- */
> @@ -263,7 +263,7 @@ static int adv7170_s_routing(struct v4l2_subdev *sd,
>  }
>
>  static int adv7170_enum_fmt(struct v4l2_subdev *sd, unsigned int index,
> -   enum v4l2_mbus_pixelcode *code)
> +   u32 *code)
>  {
> if (index >= ARRAY_SIZE(adv7170_codes))
> return -EINVAL;
> @@ -278,9 +278,9 @@ static int adv7170_g_fmt(struct v4l2_subdev *sd,
> u8 val = adv7170_read(sd, 0x7);
>
> if ((val & 0x40) == (1 << 6))
> -   mf->code = V4L2_MBUS_FMT_UYVY8_1X16;
> +   mf->code = MEDIA_BUS_FMT_UYVY8_1X16;
> else
> -   mf->code = V4L2_MBUS_FMT_UYVY8_2X8;
> +   mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
>
> mf->colorspace  = V4L2_COLORSPACE_SMPTE170M;
> mf->width   = 0;
> @@ -297,11 +297,11 @@ static int adv7170_s_fmt(struct v4l2_subdev *sd,
> int ret;
>
> switch (mf->code) {
> -   case V4L2_MBUS_FMT_UYVY8_2X8:
> +   case MEDIA_BUS_FMT_UYVY8_2X8:
> val &= ~0x40;
> break;
>
> -   case V4L2_MBUS_FMT_UYVY8_1X16:
> +   case MEDIA_BUS_FMT_UYVY8_1X16:
> val |= 0x40;
> break;
>
> diff --git a/drivers/media/i2c/adv7175.c b/drivers/media/i2c/adv7175.c
> index b88f3b3..d220af5 100644
> --- a/drivers/media/i2c/adv7175.c
> +++ b/drivers/media/i2c/adv7175.c
> @@ -60,9 +60,9 @@ static inline struct adv7175 *to_adv7175(struct v4l2_subdev 
> *sd)
>
>  static char *inputs[] = { "pass_through", "play_back", "color_bar" };
>
> -static enum v4l2_mbus_pixelcode adv7175_codes[] = {
> -   V4L2_MBUS_FMT_UYVY8_2X8,
> -   V4L2_MBUS_FMT_UYVY8_1X16,
> +static u32 adv7175_codes[] = {
> +   MEDIA_BUS_FMT_UYVY8_2X8,
> +   MEDIA_BUS_FMT_UYVY8_1X16,
>  };
>
>  /* --- */
> @@ -301,7 +301,7 @@ static int adv7175_s_routing(struct v4l2_subdev *sd,
>  }
>
>  static int adv7175_enum_fmt(struct v4l2_subdev *sd, unsigned int index,
> -   enum v4l2_mbus_pixelcode *code)
> +   u32 *code)
>  {
> if (index >= ARRAY_SIZE(adv7175_codes))
> return -EINVAL;
> @@ -316,9 +316,9 @@ static int adv7175_g_fmt(struct v4l2_subdev *sd,
> u8 val = adv7175_read(sd, 0x7);
>
> if ((val & 0x40) == (1 << 6))
> -   mf->code = V4L2_MBUS_FMT_UYVY8_1X16;
> +   mf->code = MEDIA_BUS_FMT_UYVY8_1X16;
> else
> -   mf->code = V4L2_MBUS_FMT_UYVY8_2X8;
> +   mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
>
> mf->colorspace  = V4L2_COLORSPACE_SMPTE170M;
> mf->width   = 0;
> @@ -335,11 +335,11 @@ static int adv7175_s_fmt(struct v4l2_subdev *sd,
> int ret;
>
> switch (mf->code) {
> -   case V4L2_MBUS_FMT_UYVY8_2X8:
> +   case MEDIA_BUS_FMT_UYVY8_2X8:
> val &= ~0x40;
> break;
>
> -   case V4L2_MBUS_FMT_UYVY8_1X16:
> +   case MEDIA_BUS_FMT_UYVY8_1X16:
> val |= 0x40;
>

Re: [PATCH v3 06/10] [media] platform: Make use of media_bus_format enum

2014-11-08 Thread Prabhakar Lad
Hi,

Thanks for the patch,

On Fri, Nov 7, 2014 at 2:07 PM, Boris Brezillon
 wrote:
> In order to have subsytem agnostic media bus format definitions we've
> moved media bus definition to include/uapi/linux/media-bus-format.h and
> prefixed values with MEDIA_BUS_FMT instead of V4L2_MBUS_FMT.
>
> Reference new definitions in all platform drivers.
>
> Signed-off-by: Boris Brezillon 
> ---
>  arch/arm/mach-davinci/board-dm355-evm.c|   2 +-
>  arch/arm/mach-davinci/board-dm365-evm.c|   4 +-
>  arch/arm/mach-davinci/dm355.c  |   7 +-
>  arch/arm/mach-davinci/dm365.c  |   7 +-

@Sekhar can you ack for the machine changes for davinci ?

[Snip]
>  drivers/media/platform/davinci/vpbe.c  |   2 +-
>  drivers/media/platform/davinci/vpfe_capture.c  |   4 +-
[snip]
>  include/media/davinci/vpbe.h   |   2 +-
>  include/media/davinci/vpbe_venc.h  |   5 +-

For all the above.

Acked-by: Lad, Prabhakar 

Thanks,
--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 v3 08/10] staging: media: Make use of MEDIA_BUS_FMT_ definitions

2014-11-08 Thread Prabhakar Lad
On Fri, Nov 7, 2014 at 2:07 PM, Boris Brezillon
 wrote:
> In order to have subsytem agnostic media bus format definitions we've
> moved media bus definition to include/uapi/linux/media-bus-format.h and
> prefixed values with MEDIA_BUS_FMT instead of V4L2_MBUS_FMT.
>
> Reference new definitions in all media drivers residing in staging.
>
> Signed-off-by: Boris Brezillon 
> ---
>  drivers/staging/media/davinci_vpfe/dm365_ipipe.c   |  18 ++--
>  .../staging/media/davinci_vpfe/dm365_ipipe_hw.c|  26 +++---
>  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 100 
> ++---
>  drivers/staging/media/davinci_vpfe/dm365_isif.c|  90 +--
>  drivers/staging/media/davinci_vpfe/dm365_resizer.c |  98 ++--
>  .../staging/media/davinci_vpfe/vpfe_mc_capture.c   |  18 ++--

For all the above

Acked-by: Lad, Prabhakar 

Thanks,
--Prabhakar Lad

>  drivers/staging/media/omap4iss/iss_csi2.c  |  62 ++---
>  drivers/staging/media/omap4iss/iss_ipipe.c |  16 ++--
>  drivers/staging/media/omap4iss/iss_ipipeif.c   |  28 +++---
>  drivers/staging/media/omap4iss/iss_resizer.c   |  26 +++---
>  drivers/staging/media/omap4iss/iss_video.c |  78 
>  drivers/staging/media/omap4iss/iss_video.h |  10 +--
>  12 files changed, 285 insertions(+), 285 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> index bdc7f00..704fa20 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> @@ -37,15 +37,15 @@
>
>  /* ipipe input format's */
>  static const unsigned int ipipe_input_fmts[] = {
> -   V4L2_MBUS_FMT_UYVY8_2X8,
> -   V4L2_MBUS_FMT_SGRBG12_1X12,
> -   V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
> -   V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8,
> +   MEDIA_BUS_FMT_UYVY8_2X8,
> +   MEDIA_BUS_FMT_SGRBG12_1X12,
> +   MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
> +   MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8,
>  };
>
>  /* ipipe output format's */
>  static const unsigned int ipipe_output_fmts[] = {
> -   V4L2_MBUS_FMT_UYVY8_2X8,
> +   MEDIA_BUS_FMT_UYVY8_2X8,
>  };
>
>  static int ipipe_validate_lutdpc_params(struct vpfe_ipipe_lutdpc *lutdpc)
> @@ -1457,7 +1457,7 @@ ipipe_try_format(struct vpfe_ipipe_device *ipipe,
>
> /* If not found, use SBGGR10 as default */
> if (i >= ARRAY_SIZE(ipipe_input_fmts))
> -   fmt->code = V4L2_MBUS_FMT_SGRBG12_1X12;
> +   fmt->code = MEDIA_BUS_FMT_SGRBG12_1X12;
> } else if (pad == IPIPE_PAD_SOURCE) {
> for (i = 0; i < ARRAY_SIZE(ipipe_output_fmts); i++)
> if (fmt->code == ipipe_output_fmts[i])
> @@ -1465,7 +1465,7 @@ ipipe_try_format(struct vpfe_ipipe_device *ipipe,
>
> /* If not found, use UYVY as default */
> if (i >= ARRAY_SIZE(ipipe_output_fmts))
> -   fmt->code = V4L2_MBUS_FMT_UYVY8_2X8;
> +   fmt->code = MEDIA_BUS_FMT_UYVY8_2X8;
> }
>
> fmt->width = clamp_t(u32, fmt->width, MIN_OUT_HEIGHT, max_out_width);
> @@ -1642,7 +1642,7 @@ ipipe_init_formats(struct v4l2_subdev *sd, struct 
> v4l2_subdev_fh *fh)
> memset(, 0, sizeof(format));
> format.pad = IPIPE_PAD_SINK;
> format.which = fh ? V4L2_SUBDEV_FORMAT_TRY : 
> V4L2_SUBDEV_FORMAT_ACTIVE;
> -   format.format.code = V4L2_MBUS_FMT_SGRBG12_1X12;
> +   format.format.code = MEDIA_BUS_FMT_SGRBG12_1X12;
> format.format.width = IPIPE_MAX_OUTPUT_WIDTH_A;
> format.format.height = IPIPE_MAX_OUTPUT_HEIGHT_A;
> ipipe_set_format(sd, fh, );
> @@ -1650,7 +1650,7 @@ ipipe_init_formats(struct v4l2_subdev *sd, struct 
> v4l2_subdev_fh *fh)
> memset(, 0, sizeof(format));
> format.pad = IPIPE_PAD_SOURCE;
> format.which = fh ? V4L2_SUBDEV_FORMAT_TRY : 
> V4L2_SUBDEV_FORMAT_ACTIVE;
> -   format.format.code = V4L2_MBUS_FMT_UYVY8_2X8;
> +   format.format.code = MEDIA_BUS_FMT_UYVY8_2X8;
> format.format.width = IPIPE_MAX_OUTPUT_WIDTH_A;
> format.format.height = IPIPE_MAX_OUTPUT_HEIGHT_A;
> ipipe_set_format(sd, fh, );
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c 
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> index b2daf5e..6461de1 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
> @@ -196,12 +196,12 @@ ipipe_setup_resizer(void *__iomem rsz_base, struct 
> resizer_params *param

Re: [PATCH v3 08/10] staging: media: Make use of MEDIA_BUS_FMT_ definitions

2014-11-08 Thread Prabhakar Lad
On Fri, Nov 7, 2014 at 2:07 PM, Boris Brezillon
boris.brezil...@free-electrons.com wrote:
 In order to have subsytem agnostic media bus format definitions we've
 moved media bus definition to include/uapi/linux/media-bus-format.h and
 prefixed values with MEDIA_BUS_FMT instead of V4L2_MBUS_FMT.

 Reference new definitions in all media drivers residing in staging.

 Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com
 ---
  drivers/staging/media/davinci_vpfe/dm365_ipipe.c   |  18 ++--
  .../staging/media/davinci_vpfe/dm365_ipipe_hw.c|  26 +++---
  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 100 
 ++---
  drivers/staging/media/davinci_vpfe/dm365_isif.c|  90 +--
  drivers/staging/media/davinci_vpfe/dm365_resizer.c |  98 ++--
  .../staging/media/davinci_vpfe/vpfe_mc_capture.c   |  18 ++--

For all the above

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--Prabhakar Lad

  drivers/staging/media/omap4iss/iss_csi2.c  |  62 ++---
  drivers/staging/media/omap4iss/iss_ipipe.c |  16 ++--
  drivers/staging/media/omap4iss/iss_ipipeif.c   |  28 +++---
  drivers/staging/media/omap4iss/iss_resizer.c   |  26 +++---
  drivers/staging/media/omap4iss/iss_video.c |  78 
  drivers/staging/media/omap4iss/iss_video.h |  10 +--
  12 files changed, 285 insertions(+), 285 deletions(-)

 diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
 b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
 index bdc7f00..704fa20 100644
 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
 +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
 @@ -37,15 +37,15 @@

  /* ipipe input format's */
  static const unsigned int ipipe_input_fmts[] = {
 -   V4L2_MBUS_FMT_UYVY8_2X8,
 -   V4L2_MBUS_FMT_SGRBG12_1X12,
 -   V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
 -   V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8,
 +   MEDIA_BUS_FMT_UYVY8_2X8,
 +   MEDIA_BUS_FMT_SGRBG12_1X12,
 +   MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
 +   MEDIA_BUS_FMT_SGRBG10_ALAW8_1X8,
  };

  /* ipipe output format's */
  static const unsigned int ipipe_output_fmts[] = {
 -   V4L2_MBUS_FMT_UYVY8_2X8,
 +   MEDIA_BUS_FMT_UYVY8_2X8,
  };

  static int ipipe_validate_lutdpc_params(struct vpfe_ipipe_lutdpc *lutdpc)
 @@ -1457,7 +1457,7 @@ ipipe_try_format(struct vpfe_ipipe_device *ipipe,

 /* If not found, use SBGGR10 as default */
 if (i = ARRAY_SIZE(ipipe_input_fmts))
 -   fmt-code = V4L2_MBUS_FMT_SGRBG12_1X12;
 +   fmt-code = MEDIA_BUS_FMT_SGRBG12_1X12;
 } else if (pad == IPIPE_PAD_SOURCE) {
 for (i = 0; i  ARRAY_SIZE(ipipe_output_fmts); i++)
 if (fmt-code == ipipe_output_fmts[i])
 @@ -1465,7 +1465,7 @@ ipipe_try_format(struct vpfe_ipipe_device *ipipe,

 /* If not found, use UYVY as default */
 if (i = ARRAY_SIZE(ipipe_output_fmts))
 -   fmt-code = V4L2_MBUS_FMT_UYVY8_2X8;
 +   fmt-code = MEDIA_BUS_FMT_UYVY8_2X8;
 }

 fmt-width = clamp_t(u32, fmt-width, MIN_OUT_HEIGHT, max_out_width);
 @@ -1642,7 +1642,7 @@ ipipe_init_formats(struct v4l2_subdev *sd, struct 
 v4l2_subdev_fh *fh)
 memset(format, 0, sizeof(format));
 format.pad = IPIPE_PAD_SINK;
 format.which = fh ? V4L2_SUBDEV_FORMAT_TRY : 
 V4L2_SUBDEV_FORMAT_ACTIVE;
 -   format.format.code = V4L2_MBUS_FMT_SGRBG12_1X12;
 +   format.format.code = MEDIA_BUS_FMT_SGRBG12_1X12;
 format.format.width = IPIPE_MAX_OUTPUT_WIDTH_A;
 format.format.height = IPIPE_MAX_OUTPUT_HEIGHT_A;
 ipipe_set_format(sd, fh, format);
 @@ -1650,7 +1650,7 @@ ipipe_init_formats(struct v4l2_subdev *sd, struct 
 v4l2_subdev_fh *fh)
 memset(format, 0, sizeof(format));
 format.pad = IPIPE_PAD_SOURCE;
 format.which = fh ? V4L2_SUBDEV_FORMAT_TRY : 
 V4L2_SUBDEV_FORMAT_ACTIVE;
 -   format.format.code = V4L2_MBUS_FMT_UYVY8_2X8;
 +   format.format.code = MEDIA_BUS_FMT_UYVY8_2X8;
 format.format.width = IPIPE_MAX_OUTPUT_WIDTH_A;
 format.format.height = IPIPE_MAX_OUTPUT_HEIGHT_A;
 ipipe_set_format(sd, fh, format);
 diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c 
 b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
 index b2daf5e..6461de1 100644
 --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
 +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
 @@ -196,12 +196,12 @@ ipipe_setup_resizer(void *__iomem rsz_base, struct 
 resizer_params *params)
 rsz_set_rsz_regs(rsz_base, RSZ_B, params);
  }

 -static u32 ipipe_get_color_pat(enum v4l2_mbus_pixelcode pix)
 +static u32 ipipe_get_color_pat(u32 pix)
  {
 switch (pix) {
 -   case V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8:
 -   case V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8:
 -   case

Re: [PATCH v3 06/10] [media] platform: Make use of media_bus_format enum

2014-11-08 Thread Prabhakar Lad
Hi,

Thanks for the patch,

On Fri, Nov 7, 2014 at 2:07 PM, Boris Brezillon
boris.brezil...@free-electrons.com wrote:
 In order to have subsytem agnostic media bus format definitions we've
 moved media bus definition to include/uapi/linux/media-bus-format.h and
 prefixed values with MEDIA_BUS_FMT instead of V4L2_MBUS_FMT.

 Reference new definitions in all platform drivers.

 Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com
 ---
  arch/arm/mach-davinci/board-dm355-evm.c|   2 +-
  arch/arm/mach-davinci/board-dm365-evm.c|   4 +-
  arch/arm/mach-davinci/dm355.c  |   7 +-
  arch/arm/mach-davinci/dm365.c  |   7 +-

@Sekhar can you ack for the machine changes for davinci ?

[Snip]
  drivers/media/platform/davinci/vpbe.c  |   2 +-
  drivers/media/platform/davinci/vpfe_capture.c  |   4 +-
[snip]
  include/media/davinci/vpbe.h   |   2 +-
  include/media/davinci/vpbe_venc.h  |   5 +-

For all the above.

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--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 v3 04/10] [media] i2c: Make use of media_bus_format enum

2014-11-08 Thread Prabhakar Lad
On Fri, Nov 7, 2014 at 2:07 PM, Boris Brezillon
boris.brezil...@free-electrons.com wrote:
 In order to have subsytem agnostic media bus format definitions we've
 moved media bus definitions to include/uapi/linux/media-bus-format.h and
 prefixed values with MEDIA_BUS_FMT instead of V4L2_MBUS_FMT.

 Replace all references to the old definitions in i2c drivers.

 Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com
 ---
[Snip]
  drivers/media/i2c/mt9p031.c   |  8 ++--
[Snip]
  drivers/media/i2c/tvp514x.c   | 12 +++---
  drivers/media/i2c/tvp7002.c   | 10 ++---

For all the above,

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--Prabhakar Lad

  drivers/media/i2c/vs6624.c| 18 
  46 files changed, 382 insertions(+), 382 deletions(-)

 diff --git a/drivers/media/i2c/adv7170.c b/drivers/media/i2c/adv7170.c
 index 04bb297..40a1a95 100644
 --- a/drivers/media/i2c/adv7170.c
 +++ b/drivers/media/i2c/adv7170.c
 @@ -63,9 +63,9 @@ static inline struct adv7170 *to_adv7170(struct v4l2_subdev 
 *sd)

  static char *inputs[] = { pass_through, play_back };

 -static enum v4l2_mbus_pixelcode adv7170_codes[] = {
 -   V4L2_MBUS_FMT_UYVY8_2X8,
 -   V4L2_MBUS_FMT_UYVY8_1X16,
 +static u32 adv7170_codes[] = {
 +   MEDIA_BUS_FMT_UYVY8_2X8,
 +   MEDIA_BUS_FMT_UYVY8_1X16,
  };

  /* --- */
 @@ -263,7 +263,7 @@ static int adv7170_s_routing(struct v4l2_subdev *sd,
  }

  static int adv7170_enum_fmt(struct v4l2_subdev *sd, unsigned int index,
 -   enum v4l2_mbus_pixelcode *code)
 +   u32 *code)
  {
 if (index = ARRAY_SIZE(adv7170_codes))
 return -EINVAL;
 @@ -278,9 +278,9 @@ static int adv7170_g_fmt(struct v4l2_subdev *sd,
 u8 val = adv7170_read(sd, 0x7);

 if ((val  0x40) == (1  6))
 -   mf-code = V4L2_MBUS_FMT_UYVY8_1X16;
 +   mf-code = MEDIA_BUS_FMT_UYVY8_1X16;
 else
 -   mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
 +   mf-code = MEDIA_BUS_FMT_UYVY8_2X8;

 mf-colorspace  = V4L2_COLORSPACE_SMPTE170M;
 mf-width   = 0;
 @@ -297,11 +297,11 @@ static int adv7170_s_fmt(struct v4l2_subdev *sd,
 int ret;

 switch (mf-code) {
 -   case V4L2_MBUS_FMT_UYVY8_2X8:
 +   case MEDIA_BUS_FMT_UYVY8_2X8:
 val = ~0x40;
 break;

 -   case V4L2_MBUS_FMT_UYVY8_1X16:
 +   case MEDIA_BUS_FMT_UYVY8_1X16:
 val |= 0x40;
 break;

 diff --git a/drivers/media/i2c/adv7175.c b/drivers/media/i2c/adv7175.c
 index b88f3b3..d220af5 100644
 --- a/drivers/media/i2c/adv7175.c
 +++ b/drivers/media/i2c/adv7175.c
 @@ -60,9 +60,9 @@ static inline struct adv7175 *to_adv7175(struct v4l2_subdev 
 *sd)

  static char *inputs[] = { pass_through, play_back, color_bar };

 -static enum v4l2_mbus_pixelcode adv7175_codes[] = {
 -   V4L2_MBUS_FMT_UYVY8_2X8,
 -   V4L2_MBUS_FMT_UYVY8_1X16,
 +static u32 adv7175_codes[] = {
 +   MEDIA_BUS_FMT_UYVY8_2X8,
 +   MEDIA_BUS_FMT_UYVY8_1X16,
  };

  /* --- */
 @@ -301,7 +301,7 @@ static int adv7175_s_routing(struct v4l2_subdev *sd,
  }

  static int adv7175_enum_fmt(struct v4l2_subdev *sd, unsigned int index,
 -   enum v4l2_mbus_pixelcode *code)
 +   u32 *code)
  {
 if (index = ARRAY_SIZE(adv7175_codes))
 return -EINVAL;
 @@ -316,9 +316,9 @@ static int adv7175_g_fmt(struct v4l2_subdev *sd,
 u8 val = adv7175_read(sd, 0x7);

 if ((val  0x40) == (1  6))
 -   mf-code = V4L2_MBUS_FMT_UYVY8_1X16;
 +   mf-code = MEDIA_BUS_FMT_UYVY8_1X16;
 else
 -   mf-code = V4L2_MBUS_FMT_UYVY8_2X8;
 +   mf-code = MEDIA_BUS_FMT_UYVY8_2X8;

 mf-colorspace  = V4L2_COLORSPACE_SMPTE170M;
 mf-width   = 0;
 @@ -335,11 +335,11 @@ static int adv7175_s_fmt(struct v4l2_subdev *sd,
 int ret;

 switch (mf-code) {
 -   case V4L2_MBUS_FMT_UYVY8_2X8:
 +   case MEDIA_BUS_FMT_UYVY8_2X8:
 val = ~0x40;
 break;

 -   case V4L2_MBUS_FMT_UYVY8_1X16:
 +   case MEDIA_BUS_FMT_UYVY8_1X16:
 val |= 0x40;
 break;

 diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
 index 821178d..bffe6eb 100644
 --- a/drivers/media/i2c/adv7180.c
 +++ b/drivers/media/i2c/adv7180.c
 @@ -422,12 +422,12 @@ static void adv7180_exit_controls(struct adv7180_state 
 *state)
  }

  static int adv7180_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned int index,
 -enum v4l2_mbus_pixelcode *code)
 +u32 *code)
  {
 if (index  0)
 return -EINVAL

Re: [PATCH] media: davinci: vpbe: missing clk_put

2014-11-07 Thread Prabhakar Lad
Hi,

Thanks for the patch!

On Thu, Nov 6, 2014 at 1:04 PM, Sudip Mukherjee
 wrote:
> we are getting struct clk using clk_get before calling
> clk_prepare_enable. but if clk_prepare_enable fails, then we are
> jumping to fail_mutex_unlock where we are just unlocking the mutex,
> but we are not freeing the clock source.
> this patch just adds a call to clk_put before jumping to
> fail_mutex_unlock.
>
> Signed-off-by: Sudip Mukherjee 

Acked-by: Lad, Prabhakar 

Thanks,
--Prabhakar

> ---
>  drivers/media/platform/davinci/vpbe.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/platform/davinci/vpbe.c 
> b/drivers/media/platform/davinci/vpbe.c
> index 49d2de0..e5df991 100644
> --- a/drivers/media/platform/davinci/vpbe.c
> +++ b/drivers/media/platform/davinci/vpbe.c
> @@ -625,6 +625,7 @@ static int vpbe_initialize(struct device *dev, struct 
> vpbe_device *vpbe_dev)
> }
> if (clk_prepare_enable(vpbe_dev->dac_clk)) {
> ret =  -ENODEV;
> +   clk_put(vpbe_dev->dac_clk);
> goto fail_mutex_unlock;
> }
> }
> --
> 1.8.1.2
>
--
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] media: davinci: vpbe: missing clk_put

2014-11-07 Thread Prabhakar Lad
Hi,

Thanks for the patch!

On Thu, Nov 6, 2014 at 1:04 PM, Sudip Mukherjee
sudipm.mukher...@gmail.com wrote:
 we are getting struct clk using clk_get before calling
 clk_prepare_enable. but if clk_prepare_enable fails, then we are
 jumping to fail_mutex_unlock where we are just unlocking the mutex,
 but we are not freeing the clock source.
 this patch just adds a call to clk_put before jumping to
 fail_mutex_unlock.

 Signed-off-by: Sudip Mukherjee su...@vectorindia.org

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--Prabhakar

 ---
  drivers/media/platform/davinci/vpbe.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/drivers/media/platform/davinci/vpbe.c 
 b/drivers/media/platform/davinci/vpbe.c
 index 49d2de0..e5df991 100644
 --- a/drivers/media/platform/davinci/vpbe.c
 +++ b/drivers/media/platform/davinci/vpbe.c
 @@ -625,6 +625,7 @@ static int vpbe_initialize(struct device *dev, struct 
 vpbe_device *vpbe_dev)
 }
 if (clk_prepare_enable(vpbe_dev-dac_clk)) {
 ret =  -ENODEV;
 +   clk_put(vpbe_dev-dac_clk);
 goto fail_mutex_unlock;
 }
 }
 --
 1.8.1.2

--
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 10/15] media: davinci: vpbe: add support for VIDIOC_CREATE_BUFS

2014-10-22 Thread Prabhakar Lad
Hi Hans,

On Wed, Oct 22, 2014 at 12:26 PM, Hans Verkuil  wrote:
> Hi Prabhakar,
>
> This patch series looks good, except for this one.
>
> If you add create_bufs support, then you should also update queue_setup.
>
> If the fmt argument to queue_setup is non-NULL, then check that the
> fmt.pix.sizeimage field is >= the current format's sizeimage. If not,
> return -EINVAL.
>
> This prevents userspace from creating additional buffers that are smaller
> than
> the minimum required size.
>
> I'm just skipping this patch and queuing all the others for 3.19. Just post
> an
> updated version for this one and I'll pick it up later.
>
I fixed it and posted the patch. To avoid conflicts I have rebased the patch on
for-v3.19a branch of your tree.

Thanks,
--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 10/15] media: davinci: vpbe: add support for VIDIOC_CREATE_BUFS

2014-10-22 Thread Prabhakar Lad
Hi Hans,

On Wed, Oct 22, 2014 at 12:26 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi Prabhakar,

 This patch series looks good, except for this one.

 If you add create_bufs support, then you should also update queue_setup.

 If the fmt argument to queue_setup is non-NULL, then check that the
 fmt.pix.sizeimage field is = the current format's sizeimage. If not,
 return -EINVAL.

 This prevents userspace from creating additional buffers that are smaller
 than
 the minimum required size.

 I'm just skipping this patch and queuing all the others for 3.19. Just post
 an
 updated version for this one and I'll pick it up later.

I fixed it and posted the patch. To avoid conflicts I have rebased the patch on
for-v3.19a branch of your tree.

Thanks,
--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 3/3] [media] vpif: Fix compilation with allmodconfig

2014-09-09 Thread Prabhakar Lad
On Tue, Sep 9, 2014 at 3:38 PM, Mauro Carvalho Chehab
 wrote:
> When vpif is compiled as module, those errors happen:
>
> ERROR: "vpif_lock" [drivers/media/platform/davinci/vpif_display.ko] undefined!
> ERROR: "vpif_lock" [drivers/media/platform/davinci/vpif_capture.ko] undefined!
>
> That's because vpif_lock symbol is not exported.
>

Acked-by: Lad, Prabhakar 

Regards,
--Prabhakar Lad

> Reported-by: Stephen Rothwell 
> Signed-off-by: Mauro Carvalho Chehab 
>
> diff --git a/drivers/media/platform/davinci/vpif.c 
> b/drivers/media/platform/davinci/vpif.c
> index cd08e5248387..3dad5bd7fe0a 100644
> --- a/drivers/media/platform/davinci/vpif.c
> +++ b/drivers/media/platform/davinci/vpif.c
> @@ -38,6 +38,7 @@ MODULE_LICENSE("GPL");
>  #define VPIF_CH3_MAX_MODES 2
>
>  spinlock_t vpif_lock;
> +EXPORT_SYMBOL_GPL(vpif_lock);
>
>  void __iomem *vpif_base;
>  EXPORT_SYMBOL_GPL(vpif_base);
> --
> 1.9.3
>
--
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 3/3] [media] vpif: Fix compilation with allmodconfig

2014-09-09 Thread Prabhakar Lad
On Tue, Sep 9, 2014 at 3:38 PM, Mauro Carvalho Chehab
m.che...@samsung.com wrote:
 When vpif is compiled as module, those errors happen:

 ERROR: vpif_lock [drivers/media/platform/davinci/vpif_display.ko] undefined!
 ERROR: vpif_lock [drivers/media/platform/davinci/vpif_capture.ko] undefined!

 That's because vpif_lock symbol is not exported.


Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Regards,
--Prabhakar Lad

 Reported-by: Stephen Rothwell s...@canb.auug.org.au
 Signed-off-by: Mauro Carvalho Chehab m.che...@samsung.com

 diff --git a/drivers/media/platform/davinci/vpif.c 
 b/drivers/media/platform/davinci/vpif.c
 index cd08e5248387..3dad5bd7fe0a 100644
 --- a/drivers/media/platform/davinci/vpif.c
 +++ b/drivers/media/platform/davinci/vpif.c
 @@ -38,6 +38,7 @@ MODULE_LICENSE(GPL);
  #define VPIF_CH3_MAX_MODES 2

  spinlock_t vpif_lock;
 +EXPORT_SYMBOL_GPL(vpif_lock);

  void __iomem *vpif_base;
  EXPORT_SYMBOL_GPL(vpif_base);
 --
 1.9.3

--
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/


[GIT PULL FOR v3.17] Davinci Media

2014-07-16 Thread Prabhakar Lad
Hi Mauro,

Please pull the following patches for davinci media which include
trivial cleanups.

Thanks,
--Prabhakar Lad

The following changes since commit 3c0d394ea7022bb9666d9df97a5776c4bcc3045c:

  [media] dib8000: improve the message that reports per-layer locks
(2014-07-07 09:59:01 -0300)

are available in the git repository at:

  git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git davinci_media

for you to fetch changes up to c219eb5c0fa71e11793cdc1456da8fe04f76b5ff:

  staging: media: davinci_vpfe: fix checkpatch warning (2014-07-16
20:48:18 +0100)


Andrey Utkin (1):
  davinci-vpfe: Fix retcode check

Dan Carpenter (1):
  davinci: vpfe: dm365: remove duplicate RSZ_LPF_INT_MASK

Lad, Prabhakar (2):
  media: davinci_vpfe: dm365_resizer: fix sparse warning
  staging: media: davinci_vpfe: fix checkpatch warning

 drivers/staging/media/davinci_vpfe/dm365_ipipe.c| 2 ++
 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h | 1 -
 drivers/staging/media/davinci_vpfe/dm365_resizer.c  | 4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)
--
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 v2 2/2] net: davinci_mdio: allow to create phys from dt

2014-07-16 Thread Prabhakar Lad
On Wed, Jul 16, 2014 at 1:13 PM, Grygorii Strashko
 wrote:
> This patch allows to create PHYs from DT in case
> if they are explicitly defined. The of_mdiobus_register() is
> used for such purposes.
>
> For backward compatibility, call  of_mdiobus_register() only in case
> if at least one PHY's child is defined in DT, otherwise rollback to
> mdiobus_register().
>
> Reviewed-by: Santosh Shilimkar 
> Acked-by: Mugunthan V N 
> Signed-off-by: Grygorii Strashko 

Reviewed-by: Lad, Prabhakar 

Thanks,
--Prabhakar Lad

> ---
> Hi Santosh,
>
> I haven't changed style for long-comments, because Networking subsystem
> requires first line of comment not to be empty:
> WARNING:NETWORKING_BLOCK_COMMENT_STYLE: networking block comments don't use 
> an empty /* line, use /* Comment...
>
>  drivers/net/ethernet/ti/davinci_mdio.c |   21 +++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c 
> b/drivers/net/ethernet/ti/davinci_mdio.c
> index 735dc53..2791f6f 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -38,6 +38,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  /*
> @@ -95,6 +96,10 @@ struct davinci_mdio_data {
> struct mii_bus  *bus;
> boolsuspended;
> unsigned long   access_time; /* jiffies */
> +   /* Indicates that driver shouldn't modify phy_mask in case
> +* if MDIO bus is registered from DT.
> +*/
> +   boolskip_scan;
>  };
>
>  static void __davinci_mdio_reset(struct davinci_mdio_data *data)
> @@ -144,6 +149,9 @@ static int davinci_mdio_reset(struct mii_bus *bus)
> dev_info(data->dev, "davinci mdio revision %d.%d\n",
>  (ver >> 8) & 0xff, ver & 0xff);
>
> +   if (data->skip_scan)
> +   return 0;
> +
> /* get phy mask from the alive register */
> phy_mask = __raw_readl(>regs->alive);
> if (phy_mask) {
> @@ -369,8 +377,17 @@ static int davinci_mdio_probe(struct platform_device 
> *pdev)
> goto bail_out;
> }
>
> -   /* register the mii bus */
> -   ret = mdiobus_register(data->bus);
> +   /* register the mii bus
> +* Create PHYs from DT only in case if PHY child nodes are explicitly
> +* defined to support backward compatibility with DTs which assume 
> that
> +* Davinci MDIO will always scan the bus for PHYs detection.
> +*/
> +   if (dev->of_node && of_get_child_count(dev->of_node)) {
> +   data->skip_scan = true;
> +   ret = of_mdiobus_register(data->bus, dev->of_node);
> +   } else {
> +   ret = mdiobus_register(data->bus);
> +   }
> if (ret)
> goto bail_out;
>
> --
> 1.7.9.5
>
--
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 v2 1/2] net: davinci_mdio: reuse for keystone2 arch

2014-07-16 Thread Prabhakar Lad
On Wed, Jul 16, 2014 at 1:13 PM, Grygorii Strashko
 wrote:
> The similar MDIO HW blocks is used by keystone 2 SoCs as
> in Davinci SoCs:
> - one in Gigabit Ethernet (GbE) Switch Subsystem
>   See http://www.ti.com/lit/ug/sprugv9d/sprugv9d.pdf
> - one in 10 Gigabit Ethernet Subsystem
>   See http://www.ti.com/lit/ug/spruhj5/spruhj5.pdf
>
> Hence, reuse Davinci MDIO driver for Keystone 2 and
> enable TI networking for Keystone 2 devices
>
> Reviewed-by: Santosh Shilimkar 
> Acked-by: Mugunthan V N 
> Signed-off-by: Grygorii Strashko 

Reviewed-by: Lad, Prabhakar 

Thanks,
--Prabhakar Lad

> ---
>  .../devicetree/bindings/net/davinci-mdio.txt   |8 
>  drivers/net/ethernet/ti/Kconfig|4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt 
> b/Documentation/devicetree/bindings/net/davinci-mdio.txt
> index 72efaaf..0369e25 100644
> --- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
> +++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
> @@ -1,8 +1,8 @@
> -TI SoC Davinci MDIO Controller Device Tree Bindings
> +TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
>  ---
>
>  Required properties:
> -- compatible   : Should be "ti,davinci_mdio"
> +- compatible   : Should be "ti,davinci_mdio" or "ti,keystone_mdio"
>  - reg  : physical base address and size of the davinci mdio
>   registers map
>  - bus_freq : Mdio Bus frequency
> @@ -19,7 +19,7 @@ file.
>  Examples:
>
> mdio: davinci_mdio@4A101000 {
> -   compatible = "ti,cpsw";
> +   compatible = "ti,davinci_mdio";
> reg = <0x4A101000 0x1000>;
> bus_freq = <100>;
> };
> @@ -27,7 +27,7 @@ Examples:
>  (or)
>
> mdio: davinci_mdio@4A101000 {
> -   compatible = "ti,cpsw";
> +   compatible = "ti,davinci_mdio";
> ti,hwmods = "davinci_mdio";
> bus_freq = <100>;
> };
> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> index 53150c2..1769700 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -5,7 +5,7 @@
>  config NET_VENDOR_TI
> bool "Texas Instruments (TI) devices"
> default y
> -   depends on PCI || EISA || AR7 || (ARM && (ARCH_DAVINCI || ARCH_OMAP3 
> || SOC_AM33XX))
> +   depends on PCI || EISA || AR7 || (ARM && (ARCH_DAVINCI || ARCH_OMAP3 
> || SOC_AM33XX || ARCH_KEYSTONE))
> ---help---
>   If you have a network (Ethernet) card belonging to this class, say Y
>   and read the Ethernet-HOWTO, available from
> @@ -32,7 +32,7 @@ config TI_DAVINCI_EMAC
>
>  config TI_DAVINCI_MDIO
> tristate "TI DaVinci MDIO Support"
> -   depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
> +   depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX || 
> ARCH_KEYSTONE )
> select PHYLIB
> ---help---
>   This driver supports TI's DaVinci MDIO module.
> --
> 1.7.9.5
>
--
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 v2 1/2] net: davinci_mdio: reuse for keystone2 arch

2014-07-16 Thread Prabhakar Lad
On Wed, Jul 16, 2014 at 1:13 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
 The similar MDIO HW blocks is used by keystone 2 SoCs as
 in Davinci SoCs:
 - one in Gigabit Ethernet (GbE) Switch Subsystem
   See http://www.ti.com/lit/ug/sprugv9d/sprugv9d.pdf
 - one in 10 Gigabit Ethernet Subsystem
   See http://www.ti.com/lit/ug/spruhj5/spruhj5.pdf

 Hence, reuse Davinci MDIO driver for Keystone 2 and
 enable TI networking for Keystone 2 devices

 Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com
 Acked-by: Mugunthan V N mugunthan...@ti.com
 Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com

Reviewed-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--Prabhakar Lad

 ---
  .../devicetree/bindings/net/davinci-mdio.txt   |8 
  drivers/net/ethernet/ti/Kconfig|4 ++--
  2 files changed, 6 insertions(+), 6 deletions(-)

 diff --git a/Documentation/devicetree/bindings/net/davinci-mdio.txt 
 b/Documentation/devicetree/bindings/net/davinci-mdio.txt
 index 72efaaf..0369e25 100644
 --- a/Documentation/devicetree/bindings/net/davinci-mdio.txt
 +++ b/Documentation/devicetree/bindings/net/davinci-mdio.txt
 @@ -1,8 +1,8 @@
 -TI SoC Davinci MDIO Controller Device Tree Bindings
 +TI SoC Davinci/Keystone2 MDIO Controller Device Tree Bindings
  ---

  Required properties:
 -- compatible   : Should be ti,davinci_mdio
 +- compatible   : Should be ti,davinci_mdio or ti,keystone_mdio
  - reg  : physical base address and size of the davinci mdio
   registers map
  - bus_freq : Mdio Bus frequency
 @@ -19,7 +19,7 @@ file.
  Examples:

 mdio: davinci_mdio@4A101000 {
 -   compatible = ti,cpsw;
 +   compatible = ti,davinci_mdio;
 reg = 0x4A101000 0x1000;
 bus_freq = 100;
 };
 @@ -27,7 +27,7 @@ Examples:
  (or)

 mdio: davinci_mdio@4A101000 {
 -   compatible = ti,cpsw;
 +   compatible = ti,davinci_mdio;
 ti,hwmods = davinci_mdio;
 bus_freq = 100;
 };
 diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
 index 53150c2..1769700 100644
 --- a/drivers/net/ethernet/ti/Kconfig
 +++ b/drivers/net/ethernet/ti/Kconfig
 @@ -5,7 +5,7 @@
  config NET_VENDOR_TI
 bool Texas Instruments (TI) devices
 default y
 -   depends on PCI || EISA || AR7 || (ARM  (ARCH_DAVINCI || ARCH_OMAP3 
 || SOC_AM33XX))
 +   depends on PCI || EISA || AR7 || (ARM  (ARCH_DAVINCI || ARCH_OMAP3 
 || SOC_AM33XX || ARCH_KEYSTONE))
 ---help---
   If you have a network (Ethernet) card belonging to this class, say Y
   and read the Ethernet-HOWTO, available from
 @@ -32,7 +32,7 @@ config TI_DAVINCI_EMAC

  config TI_DAVINCI_MDIO
 tristate TI DaVinci MDIO Support
 -   depends on ARM  ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX )
 +   depends on ARM  ( ARCH_DAVINCI || ARCH_OMAP3 || SOC_AM33XX || 
 ARCH_KEYSTONE )
 select PHYLIB
 ---help---
   This driver supports TI's DaVinci MDIO module.
 --
 1.7.9.5

--
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 v2 2/2] net: davinci_mdio: allow to create phys from dt

2014-07-16 Thread Prabhakar Lad
On Wed, Jul 16, 2014 at 1:13 PM, Grygorii Strashko
grygorii.stras...@ti.com wrote:
 This patch allows to create PHYs from DT in case
 if they are explicitly defined. The of_mdiobus_register() is
 used for such purposes.

 For backward compatibility, call  of_mdiobus_register() only in case
 if at least one PHY's child is defined in DT, otherwise rollback to
 mdiobus_register().

 Reviewed-by: Santosh Shilimkar santosh.shilim...@ti.com
 Acked-by: Mugunthan V N mugunthan...@ti.com
 Signed-off-by: Grygorii Strashko grygorii.stras...@ti.com

Reviewed-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--Prabhakar Lad

 ---
 Hi Santosh,

 I haven't changed style for long-comments, because Networking subsystem
 requires first line of comment not to be empty:
 WARNING:NETWORKING_BLOCK_COMMENT_STYLE: networking block comments don't use 
 an empty /* line, use /* Comment...

  drivers/net/ethernet/ti/davinci_mdio.c |   21 +++--
  1 file changed, 19 insertions(+), 2 deletions(-)

 diff --git a/drivers/net/ethernet/ti/davinci_mdio.c 
 b/drivers/net/ethernet/ti/davinci_mdio.c
 index 735dc53..2791f6f 100644
 --- a/drivers/net/ethernet/ti/davinci_mdio.c
 +++ b/drivers/net/ethernet/ti/davinci_mdio.c
 @@ -38,6 +38,7 @@
  #include linux/davinci_emac.h
  #include linux/of.h
  #include linux/of_device.h
 +#include linux/of_mdio.h
  #include linux/pinctrl/consumer.h

  /*
 @@ -95,6 +96,10 @@ struct davinci_mdio_data {
 struct mii_bus  *bus;
 boolsuspended;
 unsigned long   access_time; /* jiffies */
 +   /* Indicates that driver shouldn't modify phy_mask in case
 +* if MDIO bus is registered from DT.
 +*/
 +   boolskip_scan;
  };

  static void __davinci_mdio_reset(struct davinci_mdio_data *data)
 @@ -144,6 +149,9 @@ static int davinci_mdio_reset(struct mii_bus *bus)
 dev_info(data-dev, davinci mdio revision %d.%d\n,
  (ver  8)  0xff, ver  0xff);

 +   if (data-skip_scan)
 +   return 0;
 +
 /* get phy mask from the alive register */
 phy_mask = __raw_readl(data-regs-alive);
 if (phy_mask) {
 @@ -369,8 +377,17 @@ static int davinci_mdio_probe(struct platform_device 
 *pdev)
 goto bail_out;
 }

 -   /* register the mii bus */
 -   ret = mdiobus_register(data-bus);
 +   /* register the mii bus
 +* Create PHYs from DT only in case if PHY child nodes are explicitly
 +* defined to support backward compatibility with DTs which assume 
 that
 +* Davinci MDIO will always scan the bus for PHYs detection.
 +*/
 +   if (dev-of_node  of_get_child_count(dev-of_node)) {
 +   data-skip_scan = true;
 +   ret = of_mdiobus_register(data-bus, dev-of_node);
 +   } else {
 +   ret = mdiobus_register(data-bus);
 +   }
 if (ret)
 goto bail_out;

 --
 1.7.9.5

--
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/


[GIT PULL FOR v3.17] Davinci Media

2014-07-16 Thread Prabhakar Lad
Hi Mauro,

Please pull the following patches for davinci media which include
trivial cleanups.

Thanks,
--Prabhakar Lad

The following changes since commit 3c0d394ea7022bb9666d9df97a5776c4bcc3045c:

  [media] dib8000: improve the message that reports per-layer locks
(2014-07-07 09:59:01 -0300)

are available in the git repository at:

  git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git davinci_media

for you to fetch changes up to c219eb5c0fa71e11793cdc1456da8fe04f76b5ff:

  staging: media: davinci_vpfe: fix checkpatch warning (2014-07-16
20:48:18 +0100)


Andrey Utkin (1):
  davinci-vpfe: Fix retcode check

Dan Carpenter (1):
  davinci: vpfe: dm365: remove duplicate RSZ_LPF_INT_MASK

Lad, Prabhakar (2):
  media: davinci_vpfe: dm365_resizer: fix sparse warning
  staging: media: davinci_vpfe: fix checkpatch warning

 drivers/staging/media/davinci_vpfe/dm365_ipipe.c| 2 ++
 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h | 1 -
 drivers/staging/media/davinci_vpfe/dm365_resizer.c  | 4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)
--
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] [media] davinci-vpfe: Fix retcode check

2014-07-08 Thread Prabhakar Lad
Hi Andrey,

Thanks for the patch!

On Tue, Jul 8, 2014 at 3:58 PM, Andrey Utkin
 wrote:
> 2014-07-08 17:32 GMT+03:00 Levente Kurusa :
>> Hmm, while it is true that get_ipipe_mode returns an int, but
>> the consequent call to regw_ip takes an u32 as its second
>> argument. Did it cause a build warning for you? (Can't really
>> check since I don't have ARM cross compilers close-by)
>> If not, then:
>
> Cannot say for sure would compiler complain.
> I also haven't really checked it, and unfortunately even haven't
> succeeded to make a config that would build that code. But i believe
> that warning is still better than misbehaviour.
>
It wont cause any compile warning.

Applied for v3.17

Thanks,
--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] cpufreq: Makefile: fix compilation for davinci platform

2014-07-08 Thread Prabhakar Lad
On Tue, Jul 8, 2014 at 4:29 PM, Viresh Kumar  wrote:
> On 8 July 2014 20:55, Lad, Prabhakar  wrote:
>> From patch with commt-id:8a7b1227e303d7e927214ee0f260041aef44bb58
>> "cpufreq: davinci: move cpufreq driver to drivers/cpufreq"
>> this added dependancy only for CONFIG_ARCH_DAVINCI_DA850 where as
>> davinci_cpufreq_init() call is used by all davinci platform.
>>
>> This patch fixes following build error:
>>
>> arch/arm/mach-davinci/built-in.o: In function `davinci_init_late':
>> :(.init.text+0x928): undefined reference to `davinci_cpufreq_init'
>> make: *** [vmlinux] Error 1
>>
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>  drivers/cpufreq/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
>> index 738c8b7..db6d9a2 100644
>> --- a/drivers/cpufreq/Makefile
>> +++ b/drivers/cpufreq/Makefile
>> @@ -49,7 +49,7 @@ obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ)  += arm_big_little.o
>>  # LITTLE drivers, so that it is probed last.
>>  obj-$(CONFIG_ARM_DT_BL_CPUFREQ)+= arm_big_little_dt.o
>>
>> -obj-$(CONFIG_ARCH_DAVINCI_DA850)   += davinci-cpufreq.o
>> +obj-$(CONFIG_ARCH_DAVINCI) += davinci-cpufreq.o
>>  obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
>>  obj-$(CONFIG_ARM_EXYNOS_CPUFREQ)   += exynos-cpufreq.o
>>  obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ)   += exynos4210-cpufreq.o
>
> It took one year and 5 kernel releases to catch this again?
>
For a long time I was been working on DA850 itself, so couldn’t
notice it :(.

> Acked-by: Viresh Kumar 

Thanks!

Regards,
--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] cpufreq: Makefile: fix compilation for davinci platform

2014-07-08 Thread Prabhakar Lad
On Tue, Jul 8, 2014 at 4:29 PM, Viresh Kumar viresh.ku...@linaro.org wrote:
 On 8 July 2014 20:55, Lad, Prabhakar prabhakar.cse...@gmail.com wrote:
 From patch with commt-id:8a7b1227e303d7e927214ee0f260041aef44bb58
 cpufreq: davinci: move cpufreq driver to drivers/cpufreq
 this added dependancy only for CONFIG_ARCH_DAVINCI_DA850 where as
 davinci_cpufreq_init() call is used by all davinci platform.

 This patch fixes following build error:

 arch/arm/mach-davinci/built-in.o: In function `davinci_init_late':
 :(.init.text+0x928): undefined reference to `davinci_cpufreq_init'
 make: *** [vmlinux] Error 1

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  drivers/cpufreq/Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
 index 738c8b7..db6d9a2 100644
 --- a/drivers/cpufreq/Makefile
 +++ b/drivers/cpufreq/Makefile
 @@ -49,7 +49,7 @@ obj-$(CONFIG_ARM_BIG_LITTLE_CPUFREQ)  += arm_big_little.o
  # LITTLE drivers, so that it is probed last.
  obj-$(CONFIG_ARM_DT_BL_CPUFREQ)+= arm_big_little_dt.o

 -obj-$(CONFIG_ARCH_DAVINCI_DA850)   += davinci-cpufreq.o
 +obj-$(CONFIG_ARCH_DAVINCI) += davinci-cpufreq.o
  obj-$(CONFIG_UX500_SOC_DB8500) += dbx500-cpufreq.o
  obj-$(CONFIG_ARM_EXYNOS_CPUFREQ)   += exynos-cpufreq.o
  obj-$(CONFIG_ARM_EXYNOS4210_CPUFREQ)   += exynos4210-cpufreq.o

 It took one year and 5 kernel releases to catch this again?

For a long time I was been working on DA850 itself, so couldn’t
notice it :(.

 Acked-by: Viresh Kumar viresh.ku...@linaro.org

Thanks!

Regards,
--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] [media] davinci-vpfe: Fix retcode check

2014-07-08 Thread Prabhakar Lad
Hi Andrey,

Thanks for the patch!

On Tue, Jul 8, 2014 at 3:58 PM, Andrey Utkin
andrey.krieger.ut...@gmail.com wrote:
 2014-07-08 17:32 GMT+03:00 Levente Kurusa lkur...@redhat.com:
 Hmm, while it is true that get_ipipe_mode returns an int, but
 the consequent call to regw_ip takes an u32 as its second
 argument. Did it cause a build warning for you? (Can't really
 check since I don't have ARM cross compilers close-by)
 If not, then:

 Cannot say for sure would compiler complain.
 I also haven't really checked it, and unfortunately even haven't
 succeeded to make a config that would build that code. But i believe
 that warning is still better than misbehaviour.

It wont cause any compile warning.

Applied for v3.17

Thanks,
--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] can: c_can: convert to use devm * api

2014-06-22 Thread Prabhakar Lad
On Sun, Jun 22, 2014 at 11:56 AM, Marc Kleine-Budde  wrote:
> On 06/20/2014 01:59 PM, Lad, Prabhakar wrote:
>> From: "Lad, Prabhakar" 
>>
>> this patch uses devm_* APIs as they are device managed
>> and make code simpler.
>>
>> Signed-off-by: Lad, Prabhakar 
>
> Thanks for the patch. Applied to can-next with minor change (see inline).
>
>> ---
>>  Note: This patch is compile tested only.
>>
>>  drivers/net/can/c_can/c_can_platform.c | 41 
>> --
>>  1 file changed, 9 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/net/can/c_can/c_can_platform.c 
>> b/drivers/net/can/c_can/c_can_platform.c
>> index 1df0b32..e0dcbcf 100644
>> --- a/drivers/net/can/c_can/c_can_platform.c
>> +++ b/drivers/net/can/c_can/c_can_platform.c
>> @@ -157,40 +157,31 @@ static int c_can_plat_probe(struct platform_device 
>> *pdev)
>>   }
>>
>>   /* get the appropriate clk */
>> - clk = clk_get(>dev, NULL);
>> + clk = devm_clk_get(>dev, NULL);
>>   if (IS_ERR(clk)) {
>> - dev_err(>dev, "no clock defined\n");
>> - ret = -ENODEV;
>> + ret = PTR_ERR(clk);
>>   goto exit;
>>   }
>>
>>   /* get the platform data */
>>   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>   irq = platform_get_irq(pdev, 0);
>> - if (!mem || irq <= 0) {
>> - ret = -ENODEV;
>> - goto exit_free_clk;
>> - }
>> -
>> - if (!request_mem_region(mem->start, resource_size(mem),
>> - KBUILD_MODNAME)) {
>> - dev_err(>dev, "resource unavailable\n");
>> + if (irq <= 0) {
>>   ret = -ENODEV;
>> - goto exit_free_clk;
>> + goto exit;
>>   }
>>
>> - addr = ioremap(mem->start, resource_size(mem));
>> - if (!addr) {
>> - dev_err(>dev, "failed to map can port\n");
>> - ret = -ENOMEM;
>> - goto exit_release_mem;
>
> I've moved the mem = platform_get_resource() for slightly better
> readability here
>
Thanks makes sense.

Regards,
--Prabhakar Lad

>> + addr = devm_ioremap_resource(>dev, mem);
>> + if (IS_ERR(addr)) {
>> + ret =  PTR_ERR(addr);
>> + goto exit;
>>   }
>
> Thanks,
> Marc
>
> --
> Pengutronix e.K.  | Marc Kleine-Budde   |
> Industrial Linux Solutions| Phone: +49-231-2826-924 |
> Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
>
--
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] can: c_can: convert to use devm * api

2014-06-22 Thread Prabhakar Lad
On Sun, Jun 22, 2014 at 11:56 AM, Marc Kleine-Budde m...@pengutronix.de wrote:
 On 06/20/2014 01:59 PM, Lad, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 this patch uses devm_* APIs as they are device managed
 and make code simpler.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com

 Thanks for the patch. Applied to can-next with minor change (see inline).

 ---
  Note: This patch is compile tested only.

  drivers/net/can/c_can/c_can_platform.c | 41 
 --
  1 file changed, 9 insertions(+), 32 deletions(-)

 diff --git a/drivers/net/can/c_can/c_can_platform.c 
 b/drivers/net/can/c_can/c_can_platform.c
 index 1df0b32..e0dcbcf 100644
 --- a/drivers/net/can/c_can/c_can_platform.c
 +++ b/drivers/net/can/c_can/c_can_platform.c
 @@ -157,40 +157,31 @@ static int c_can_plat_probe(struct platform_device 
 *pdev)
   }

   /* get the appropriate clk */
 - clk = clk_get(pdev-dev, NULL);
 + clk = devm_clk_get(pdev-dev, NULL);
   if (IS_ERR(clk)) {
 - dev_err(pdev-dev, no clock defined\n);
 - ret = -ENODEV;
 + ret = PTR_ERR(clk);
   goto exit;
   }

   /* get the platform data */
   mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   irq = platform_get_irq(pdev, 0);
 - if (!mem || irq = 0) {
 - ret = -ENODEV;
 - goto exit_free_clk;
 - }
 -
 - if (!request_mem_region(mem-start, resource_size(mem),
 - KBUILD_MODNAME)) {
 - dev_err(pdev-dev, resource unavailable\n);
 + if (irq = 0) {
   ret = -ENODEV;
 - goto exit_free_clk;
 + goto exit;
   }

 - addr = ioremap(mem-start, resource_size(mem));
 - if (!addr) {
 - dev_err(pdev-dev, failed to map can port\n);
 - ret = -ENOMEM;
 - goto exit_release_mem;

 I've moved the mem = platform_get_resource() for slightly better
 readability here

Thanks makes sense.

Regards,
--Prabhakar Lad

 + addr = devm_ioremap_resource(pdev-dev, mem);
 + if (IS_ERR(addr)) {
 + ret =  PTR_ERR(addr);
 + goto exit;
   }

 Thanks,
 Marc

 --
 Pengutronix e.K.  | Marc Kleine-Budde   |
 Industrial Linux Solutions| Phone: +49-231-2826-924 |
 Vertretung West/Dortmund  | Fax:   +49-5121-206917- |
 Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

--
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] media: v4l2-core: v4l2-dv-timings.c: Cleaning up code that putting values to the same variable twice

2014-06-10 Thread Prabhakar Lad
Hi Rickard,

Thanks for the patch.

On Sat, Jun 7, 2014 at 1:31 AM, Rickard Strandqvist
 wrote:
> Instead of putting the same variable twice,
> was rather intended to set this value to two different variable.
>
> This was partly found using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist 

The commit message and header needs to be improved, apart from that rest of the
patch looks good.

Acked-by: Lad, Prabhakar 

Regards,
--Prabhakar Lad

> ---
>  drivers/media/v4l2-core/v4l2-dv-timings.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c 
> b/drivers/media/v4l2-core/v4l2-dv-timings.c
> index 48b20df..eb3850c 100644
> --- a/drivers/media/v4l2-core/v4l2-dv-timings.c
> +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
> @@ -599,10 +599,10 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 
> hor_landscape, u8 vert_portrait)
> aspect.denominator = 9;
> } else if (ratio == 34) {
> aspect.numerator = 4;
> -   aspect.numerator = 3;
> +   aspect.denominator = 3;
> } else if (ratio == 68) {
> aspect.numerator = 15;
> -   aspect.numerator = 9;
> +   aspect.denominator = 9;
> } else {
> aspect.numerator = hor_landscape + 99;
> aspect.denominator = 100;
> --
> 1.7.10.4
>
--
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] media: v4l2-core: v4l2-dv-timings.c: Cleaning up code that putting values to the same variable twice

2014-06-10 Thread Prabhakar Lad
Hi Rickard,

Thanks for the patch.

On Sat, Jun 7, 2014 at 1:31 AM, Rickard Strandqvist
rickard_strandqv...@spectrumdigital.se wrote:
 Instead of putting the same variable twice,
 was rather intended to set this value to two different variable.

 This was partly found using a static code analysis program called cppcheck.

 Signed-off-by: Rickard Strandqvist rickard_strandqv...@spectrumdigital.se

The commit message and header needs to be improved, apart from that rest of the
patch looks good.

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Regards,
--Prabhakar Lad

 ---
  drivers/media/v4l2-core/v4l2-dv-timings.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/media/v4l2-core/v4l2-dv-timings.c 
 b/drivers/media/v4l2-core/v4l2-dv-timings.c
 index 48b20df..eb3850c 100644
 --- a/drivers/media/v4l2-core/v4l2-dv-timings.c
 +++ b/drivers/media/v4l2-core/v4l2-dv-timings.c
 @@ -599,10 +599,10 @@ struct v4l2_fract v4l2_calc_aspect_ratio(u8 
 hor_landscape, u8 vert_portrait)
 aspect.denominator = 9;
 } else if (ratio == 34) {
 aspect.numerator = 4;
 -   aspect.numerator = 3;
 +   aspect.denominator = 3;
 } else if (ratio == 68) {
 aspect.numerator = 15;
 -   aspect.numerator = 9;
 +   aspect.denominator = 9;
 } else {
 aspect.numerator = hor_landscape + 99;
 aspect.denominator = 100;
 --
 1.7.10.4

--
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] [media] dm644x_ccdc: remove check for CONFIG_DM644X_VIDEO_PORT_ENABLE

2014-06-09 Thread Prabhakar Lad
Hi Paul,

Thanks for the patch.

On Thu, May 22, 2014 at 8:42 PM, Paul Bolle  wrote:
> A check for CONFIG_DM644X_VIDEO_PORT_ENABLE was added in v2.6.32. The
> related Kconfig symbol was never added so this check has always
> evaluated to false. Remove that check.
>
Applied.

Thanks,
--Prabhakar Lad

> Signed-off-by: Paul Bolle 
> ---
> Untested.
>
> Related, trivial, cleanup: make ccdc_enable_vport() a oneliner.
>
>  drivers/media/platform/davinci/dm644x_ccdc.c | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c 
> b/drivers/media/platform/davinci/dm644x_ccdc.c
> index 30fa08405d61..07e98df3d867 100644
> --- a/drivers/media/platform/davinci/dm644x_ccdc.c
> +++ b/drivers/media/platform/davinci/dm644x_ccdc.c
> @@ -581,13 +581,8 @@ void ccdc_config_raw(void)
>  config_params->alaw.enable)
> syn_mode |= CCDC_DATA_PACK_ENABLE;
>
> -#ifdef CONFIG_DM644X_VIDEO_PORT_ENABLE
> -   /* enable video port */
> -   val = CCDC_ENABLE_VIDEO_PORT;
> -#else
> /* disable video port */
> val = CCDC_DISABLE_VIDEO_PORT;
> -#endif
>
> if (config_params->data_sz == CCDC_DATA_8BITS)
> val |= (CCDC_DATA_10BITS & CCDC_FMTCFG_VPIN_MASK)
> --
> 1.9.0
>
--
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] [media] dm644x_ccdc: remove check for CONFIG_DM644X_VIDEO_PORT_ENABLE

2014-06-09 Thread Prabhakar Lad
Hi Paul,

Thanks for the patch.

On Thu, May 22, 2014 at 8:42 PM, Paul Bolle pebo...@tiscali.nl wrote:
 A check for CONFIG_DM644X_VIDEO_PORT_ENABLE was added in v2.6.32. The
 related Kconfig symbol was never added so this check has always
 evaluated to false. Remove that check.

Applied.

Thanks,
--Prabhakar Lad

 Signed-off-by: Paul Bolle pebo...@tiscali.nl
 ---
 Untested.

 Related, trivial, cleanup: make ccdc_enable_vport() a oneliner.

  drivers/media/platform/davinci/dm644x_ccdc.c | 5 -
  1 file changed, 5 deletions(-)

 diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c 
 b/drivers/media/platform/davinci/dm644x_ccdc.c
 index 30fa08405d61..07e98df3d867 100644
 --- a/drivers/media/platform/davinci/dm644x_ccdc.c
 +++ b/drivers/media/platform/davinci/dm644x_ccdc.c
 @@ -581,13 +581,8 @@ void ccdc_config_raw(void)
  config_params-alaw.enable)
 syn_mode |= CCDC_DATA_PACK_ENABLE;

 -#ifdef CONFIG_DM644X_VIDEO_PORT_ENABLE
 -   /* enable video port */
 -   val = CCDC_ENABLE_VIDEO_PORT;
 -#else
 /* disable video port */
 val = CCDC_DISABLE_VIDEO_PORT;
 -#endif

 if (config_params-data_sz == CCDC_DATA_8BITS)
 val |= (CCDC_DATA_10BITS  CCDC_FMTCFG_VPIN_MASK)
 --
 1.9.0

--
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 v5 41/49] media: davinci: vpif_capture: drop unneeded module params

2014-05-23 Thread Prabhakar Lad
Hi Mauro,

On Sat, May 24, 2014 at 4:08 AM, Mauro Carvalho Chehab
 wrote:
> Em Fri, 16 May 2014 19:03:47 +0530
> "Lad, Prabhakar"  escreveu:
>
>> From: "Lad, Prabhakar" 
>>
>> Signed-off-by: Lad, Prabhakar 
>
> -ENOPATCHDESCRIPTION
>
> Why to remove those parameters? Please _ALWAYS_ describe your patches.
>
My bad! will take care next time.

Thanks,
--Prabhakar Lad

> My crystal ball is malfunctioning today, so I was unable to scry the
> reasons for this patch.
>
> Thanks,
> Mauro
>
>> ---
>>  drivers/media/platform/davinci/vpif_capture.c |   54 
>> +
>>  drivers/media/platform/davinci/vpif_capture.h |   11 -
>>  2 files changed, 1 insertion(+), 64 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>> b/drivers/media/platform/davinci/vpif_capture.c
>> index d452eaf..e967cf7 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -38,32 +38,10 @@ MODULE_VERSION(VPIF_CAPTURE_VERSION);
>>   v4l2_dbg(level, debug, _obj.v4l2_dev, fmt, ## arg)
>>
>>  static int debug = 1;
>> -static u32 ch0_numbuffers = 3;
>> -static u32 ch1_numbuffers = 3;
>> -static u32 ch0_bufsize = 1920 * 1080 * 2;
>> -static u32 ch1_bufsize = 720 * 576 * 2;
>>
>>  module_param(debug, int, 0644);
>> -module_param(ch0_numbuffers, uint, S_IRUGO);
>> -module_param(ch1_numbuffers, uint, S_IRUGO);
>> -module_param(ch0_bufsize, uint, S_IRUGO);
>> -module_param(ch1_bufsize, uint, S_IRUGO);
>>
>>  MODULE_PARM_DESC(debug, "Debug level 0-1");
>> -MODULE_PARM_DESC(ch2_numbuffers, "Channel0 buffer count (default:3)");
>> -MODULE_PARM_DESC(ch3_numbuffers, "Channel1 buffer count (default:3)");
>> -MODULE_PARM_DESC(ch2_bufsize, "Channel0 buffer size (default:1920 x 1080 x 
>> 2)");
>> -MODULE_PARM_DESC(ch3_bufsize, "Channel1 buffer size (default:720 x 576 x 
>> 2)");
>> -
>> -static struct vpif_config_params config_params = {
>> - .min_numbuffers = 3,
>> - .numbuffers[0] = 3,
>> - .numbuffers[1] = 3,
>> - .min_bufsize[0] = 720 * 480 * 2,
>> - .min_bufsize[1] = 720 * 480 * 2,
>> - .channel_bufsize[0] = 1920 * 1080 * 2,
>> - .channel_bufsize[1] = 720 * 576 * 2,
>> -};
>>
>>  #define VPIF_DRIVER_NAME "vpif_capture"
>>
>> @@ -609,9 +587,6 @@ static void vpif_config_format(struct channel_obj *ch)
>>   vpif_dbg(2, debug, "vpif_config_format\n");
>>
>>   common->fmt.fmt.pix.field = V4L2_FIELD_ANY;
>> - common->fmt.fmt.pix.sizeimage
>> - = config_params.channel_bufsize[ch->channel_id];
>> -
>>   if (ch->vpifparams.iface.if_type == VPIF_IF_RAW_BAYER)
>>   common->fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_SBGGR8;
>>   else
>> @@ -1358,36 +1333,9 @@ static struct v4l2_file_operations vpif_fops = {
>>   */
>>  static int initialize_vpif(void)
>>  {
>> - int err = 0, i, j;
>> + int err, i, j;
>>   int free_channel_objects_index;
>>
>> - /* Default number of buffers should be 3 */
>> - if ((ch0_numbuffers > 0) &&
>> - (ch0_numbuffers < config_params.min_numbuffers))
>> - ch0_numbuffers = config_params.min_numbuffers;
>> - if ((ch1_numbuffers > 0) &&
>> - (ch1_numbuffers < config_params.min_numbuffers))
>> - ch1_numbuffers = config_params.min_numbuffers;
>> -
>> - /* Set buffer size to min buffers size if it is invalid */
>> - if (ch0_bufsize < config_params.min_bufsize[VPIF_CHANNEL0_VIDEO])
>> - ch0_bufsize =
>> - config_params.min_bufsize[VPIF_CHANNEL0_VIDEO];
>> - if (ch1_bufsize < config_params.min_bufsize[VPIF_CHANNEL1_VIDEO])
>> - ch1_bufsize =
>> - config_params.min_bufsize[VPIF_CHANNEL1_VIDEO];
>> -
>> - config_params.numbuffers[VPIF_CHANNEL0_VIDEO] = ch0_numbuffers;
>> - config_params.numbuffers[VPIF_CHANNEL1_VIDEO] = ch1_numbuffers;
>> - if (ch0_numbuffers) {
>> - config_params.channel_bufsize[VPIF_CHANNEL0_VIDEO]
>> - = ch0_bufsize;
>> - }
>> - if (ch1_numbuffers) {
>> - config_params.channel_bufsize[VPIF_CHANNEL1_VIDEO]
>> - = ch1_bufsize;
>> - }
>> -
>>   /* Allocate memory for six channel objects 

Re: [PATCH v5 00/49] DaVinci: vpif: upgrade with v4l helpers and v4l compliance fixes

2014-05-23 Thread Prabhakar Lad
Hi Hans,

Thanks for the review from 2 patches to 50 :)

On Fri, May 23, 2014 at 2:00 PM, Hans Verkuil  wrote:
> Hi Prabhakar,
>
> Thanks for this patch series, it looks good to me and I'll make a pull
> request for this.
>
Thanks.

> I did find a few issues, but they are all pre-existing problems, so they
> can be fixed in follow-up patches.
>
> I'll comment on those in the relevant patches. Since display and capture are
> so similar I will only comment on the display patches, but it's valid for
> both.
>
Ok will fix them up soon I am relocating next week so will take me at-least
2-3 weeks for me to get back the boards and get in action.

Regards,
--Prabhakar Lad

> Regards,
>
> Hans
>
> On 05/16/2014 03:33 PM, Lad, Prabhakar wrote:
>> From: "Lad, Prabhakar" 
>>
>> Hi,
>>
>> This patch series upgrades the vpif capture & display
>> driver with the all the helpers provided by v4l, this makes
>> the driver much simpler and cleaner. This also includes few
>> checkpatch issues.
>>
>> Changes for v2:
>> a> Added a copyright.
>> b> Dropped buf_init() callback from vb2_ops.
>> c> Fixed enabling & disabling of interrupts in case of HD formats.
>>
>> Changes for v3:
>> a> Fixed review comments pointed by Hans.
>>
>> Changes for v4: Rebased the patches on media tree.
>>
>> Changes for v5: Split up the patches
>>
>> Following is the output of v4l-compliance for capture:
>> --
>>
>> ./v4l2-compliance -d /dev/video0 -i 0 -s -v --expbuf-device=2
>>
>> Driver Info:
>> Driver name   : vpif_capture
>> Card type : DA850/OMAP-L138 Video Capture
>> Bus info  : platform:vpif_capture
>> Driver version: 3.15.0
>> Capabilities  : 0x8401
>> Video Capture
>> Streaming
>> Device Capabilities
>> Device Caps   : 0x0401
>> Video Capture
>> Streaming
>>
>> Compliance test for device /dev/video0 (not using libv4l2):
>>
>> Required ioctls:
>> test VIDIOC_QUERYCAP: OK
>>
>> Allow for multiple opens:
>> test second video open: OK
>> test VIDIOC_QUERYCAP: OK
>> test VIDIOC_G/S_PRIORITY: OK
>>
>> Debug ioctls:
>> test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>> test VIDIOC_LOG_STATUS: OK
>>
>> Input ioctls:
>> test VIDIOC_G/S_TUNER: OK (Not Supported)
>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>> test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>> test VIDIOC_ENUMAUDIO: OK (Not Supported)
>> test VIDIOC_G/S/ENUMINPUT: OK
>> test VIDIOC_G/S_AUDIO: OK (Not Supported)
>> Inputs: 1 Audio Inputs: 0 Tuners: 0
>>
>> Output ioctls:
>> test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>> test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>> test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>> test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>> test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>> Outputs: 0 Audio Outputs: 0 Modulators: 0
>>
>> Input/Output configuration ioctls:
>> test VIDIOC_ENUM/G/S/QUERY_STD: OK
>> test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>> test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>> test VIDIOC_G/S_EDID: OK (Not Supported)
>>
>> Test input 0:
>>
>> Control ioctls:
>> test VIDIOC_QUERYCTRL/MENU: OK (Not Supported)
>> test VIDIOC_G/S_CTRL: OK (Not Supported)
>> test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
>> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
>> test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>> Standard Controls: 0 Private Controls: 0
>>
>> Format ioctls:
>> info: found 1 formats for buftype 1
>> test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>> fail: v4l2-test-formats.cpp(1003): cap->readbuffers
>> test VIDIOC_G/S_PARM: FAIL
>> test VIDIOC_G_FBUF: OK (Not Supported)
>> test VIDIOC_G_FMT: OK
>> test VIDIOC_TRY_FMT: OK
>> test VIDIOC_S_FMT: OK
>> test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>>
>>

Re: [PATCH v5 04/49] media: davinci: vpif_display: release buffers in case start_streaming() call back fails

2014-05-23 Thread Prabhakar Lad
Hi Hans,

Thanks for the review.

On Fri, May 23, 2014 at 2:05 PM, Hans Verkuil  wrote:
> On 05/16/2014 03:33 PM, Lad, Prabhakar wrote:
>> From: "Lad, Prabhakar" 
>>
>> this patch adds support to release the buffer by calling
>> vb2_buffer_done(), with state marked as VB2_BUF_STATE_QUEUED
>> if start_streaming() call back fails.
>>
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>  drivers/media/platform/davinci/vpif_display.c |   42 
>> +++--
>>  1 file changed, 26 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif_display.c 
>> b/drivers/media/platform/davinci/vpif_display.c
>> index 8bb9f02..1a17a45 100644
>> --- a/drivers/media/platform/davinci/vpif_display.c
>> +++ b/drivers/media/platform/davinci/vpif_display.c
>> @@ -196,26 +196,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
>> unsigned int count)
>>   struct channel_obj *ch = vb2_get_drv_priv(vq);
>>   struct common_obj *common = >common[VPIF_VIDEO_INDEX];
>>   struct vpif_params *vpif = >vpifparams;
>> - unsigned long addr = 0;
>> - unsigned long flags;
>> + struct vpif_disp_buffer *buf, *tmp;
>> + unsigned long addr, flags;
>>   int ret;
>>
>>   spin_lock_irqsave(>irqlock, flags);
>>
>> - /* Get the next frame from the buffer queue */
>> - common->next_frm = common->cur_frm =
>> - list_entry(common->dma_queue.next,
>> -struct vpif_disp_buffer, list);
>> -
>> - list_del(>cur_frm->list);
>> - spin_unlock_irqrestore(>irqlock, flags);
>> - /* Mark state of the current frame to active */
>> - common->cur_frm->vb.state = VB2_BUF_STATE_ACTIVE;
>> -
>>   /* Initialize field_id and started member */
>>   ch->field_id = 0;
>>   common->started = 1;
>> - addr = vb2_dma_contig_plane_dma_addr(>cur_frm->vb, 0);
>> +
>>   /* Calculate the offset for Y and C data  in the buffer */
>>   vpif_calculate_offsets(ch);
>>
>> @@ -225,7 +215,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
>> unsigned int count)
>>   || (!ch->vpifparams.std_info.frm_fmt
>>   && (common->fmt.fmt.pix.field == V4L2_FIELD_NONE))) {
>>   vpif_err("conflict in field format and std format\n");
>> - return -EINVAL;
>> + ret = -EINVAL;
>> + goto err;
>>   }
>>
>>   /* clock settings */
>> @@ -234,17 +225,28 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
>> unsigned int count)
>>   ycmux_mode, ch->vpifparams.std_info.hd_sd);
>>   if (ret < 0) {
>>   vpif_err("can't set clock\n");
>> - return ret;
>> + goto err;
>>   }
>>   }
>>
>>   /* set the parameters and addresses */
>>   ret = vpif_set_video_params(vpif, ch->channel_id + 2);
>>   if (ret < 0)
>> - return ret;
>> + goto err;
>>
>>   common->started = ret;
>>   vpif_config_addr(ch, ret);
>> + /* Get the next frame from the buffer queue */
>> + common->next_frm = common->cur_frm =
>> + list_entry(common->dma_queue.next,
>> +    struct vpif_disp_buffer, list);
>> +
>> + list_del(>cur_frm->list);
>> + spin_unlock_irqrestore(>irqlock, flags);
>> + /* Mark state of the current frame to active */
>> + common->cur_frm->vb.state = VB2_BUF_STATE_ACTIVE;
>
> There is no need to set this, all buffers queued to the driver are always in 
> state
> ACTIVE. The vb2 core sets that for you. In general drivers never need to 
> change the
> state manually.
>
> It happens twice in this driver and in both cases the assignment can be 
> removed.
>
OK, will drop this.

Regards,
--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 v5 04/49] media: davinci: vpif_display: release buffers in case start_streaming() call back fails

2014-05-23 Thread Prabhakar Lad
Hi Hans,

Thanks for the review.

On Fri, May 23, 2014 at 2:05 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 On 05/16/2014 03:33 PM, Lad, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 this patch adds support to release the buffer by calling
 vb2_buffer_done(), with state marked as VB2_BUF_STATE_QUEUED
 if start_streaming() call back fails.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  drivers/media/platform/davinci/vpif_display.c |   42 
 +++--
  1 file changed, 26 insertions(+), 16 deletions(-)

 diff --git a/drivers/media/platform/davinci/vpif_display.c 
 b/drivers/media/platform/davinci/vpif_display.c
 index 8bb9f02..1a17a45 100644
 --- a/drivers/media/platform/davinci/vpif_display.c
 +++ b/drivers/media/platform/davinci/vpif_display.c
 @@ -196,26 +196,16 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   struct channel_obj *ch = vb2_get_drv_priv(vq);
   struct common_obj *common = ch-common[VPIF_VIDEO_INDEX];
   struct vpif_params *vpif = ch-vpifparams;
 - unsigned long addr = 0;
 - unsigned long flags;
 + struct vpif_disp_buffer *buf, *tmp;
 + unsigned long addr, flags;
   int ret;

   spin_lock_irqsave(common-irqlock, flags);

 - /* Get the next frame from the buffer queue */
 - common-next_frm = common-cur_frm =
 - list_entry(common-dma_queue.next,
 -struct vpif_disp_buffer, list);
 -
 - list_del(common-cur_frm-list);
 - spin_unlock_irqrestore(common-irqlock, flags);
 - /* Mark state of the current frame to active */
 - common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;
 -
   /* Initialize field_id and started member */
   ch-field_id = 0;
   common-started = 1;
 - addr = vb2_dma_contig_plane_dma_addr(common-cur_frm-vb, 0);
 +
   /* Calculate the offset for Y and C data  in the buffer */
   vpif_calculate_offsets(ch);

 @@ -225,7 +215,8 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   || (!ch-vpifparams.std_info.frm_fmt
(common-fmt.fmt.pix.field == V4L2_FIELD_NONE))) {
   vpif_err(conflict in field format and std format\n);
 - return -EINVAL;
 + ret = -EINVAL;
 + goto err;
   }

   /* clock settings */
 @@ -234,17 +225,28 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   ycmux_mode, ch-vpifparams.std_info.hd_sd);
   if (ret  0) {
   vpif_err(can't set clock\n);
 - return ret;
 + goto err;
   }
   }

   /* set the parameters and addresses */
   ret = vpif_set_video_params(vpif, ch-channel_id + 2);
   if (ret  0)
 - return ret;
 + goto err;

   common-started = ret;
   vpif_config_addr(ch, ret);
 + /* Get the next frame from the buffer queue */
 + common-next_frm = common-cur_frm =
 + list_entry(common-dma_queue.next,
 +struct vpif_disp_buffer, list);
 +
 + list_del(common-cur_frm-list);
 + spin_unlock_irqrestore(common-irqlock, flags);
 + /* Mark state of the current frame to active */
 + common-cur_frm-vb.state = VB2_BUF_STATE_ACTIVE;

 There is no need to set this, all buffers queued to the driver are always in 
 state
 ACTIVE. The vb2 core sets that for you. In general drivers never need to 
 change the
 state manually.

 It happens twice in this driver and in both cases the assignment can be 
 removed.

OK, will drop this.

Regards,
--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 v5 00/49] DaVinci: vpif: upgrade with v4l helpers and v4l compliance fixes

2014-05-23 Thread Prabhakar Lad
Hi Hans,

Thanks for the review from 2 patches to 50 :)

On Fri, May 23, 2014 at 2:00 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi Prabhakar,

 Thanks for this patch series, it looks good to me and I'll make a pull
 request for this.

Thanks.

 I did find a few issues, but they are all pre-existing problems, so they
 can be fixed in follow-up patches.

 I'll comment on those in the relevant patches. Since display and capture are
 so similar I will only comment on the display patches, but it's valid for
 both.

Ok will fix them up soon I am relocating next week so will take me at-least
2-3 weeks for me to get back the boards and get in action.

Regards,
--Prabhakar Lad

 Regards,

 Hans

 On 05/16/2014 03:33 PM, Lad, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 Hi,

 This patch series upgrades the vpif capture  display
 driver with the all the helpers provided by v4l, this makes
 the driver much simpler and cleaner. This also includes few
 checkpatch issues.

 Changes for v2:
 a Added a copyright.
 b Dropped buf_init() callback from vb2_ops.
 c Fixed enabling  disabling of interrupts in case of HD formats.

 Changes for v3:
 a Fixed review comments pointed by Hans.

 Changes for v4: Rebased the patches on media tree.

 Changes for v5: Split up the patches

 Following is the output of v4l-compliance for capture:
 --

 ./v4l2-compliance -d /dev/video0 -i 0 -s -v --expbuf-device=2

 Driver Info:
 Driver name   : vpif_capture
 Card type : DA850/OMAP-L138 Video Capture
 Bus info  : platform:vpif_capture
 Driver version: 3.15.0
 Capabilities  : 0x8401
 Video Capture
 Streaming
 Device Capabilities
 Device Caps   : 0x0401
 Video Capture
 Streaming

 Compliance test for device /dev/video0 (not using libv4l2):

 Required ioctls:
 test VIDIOC_QUERYCAP: OK

 Allow for multiple opens:
 test second video open: OK
 test VIDIOC_QUERYCAP: OK
 test VIDIOC_G/S_PRIORITY: OK

 Debug ioctls:
 test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
 test VIDIOC_LOG_STATUS: OK

 Input ioctls:
 test VIDIOC_G/S_TUNER: OK (Not Supported)
 test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
 test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
 test VIDIOC_ENUMAUDIO: OK (Not Supported)
 test VIDIOC_G/S/ENUMINPUT: OK
 test VIDIOC_G/S_AUDIO: OK (Not Supported)
 Inputs: 1 Audio Inputs: 0 Tuners: 0

 Output ioctls:
 test VIDIOC_G/S_MODULATOR: OK (Not Supported)
 test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
 test VIDIOC_ENUMAUDOUT: OK (Not Supported)
 test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
 test VIDIOC_G/S_AUDOUT: OK (Not Supported)
 Outputs: 0 Audio Outputs: 0 Modulators: 0

 Input/Output configuration ioctls:
 test VIDIOC_ENUM/G/S/QUERY_STD: OK
 test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
 test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
 test VIDIOC_G/S_EDID: OK (Not Supported)

 Test input 0:

 Control ioctls:
 test VIDIOC_QUERYCTRL/MENU: OK (Not Supported)
 test VIDIOC_G/S_CTRL: OK (Not Supported)
 test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
 test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
 test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
 Standard Controls: 0 Private Controls: 0

 Format ioctls:
 info: found 1 formats for buftype 1
 test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
 fail: v4l2-test-formats.cpp(1003): cap-readbuffers
 test VIDIOC_G/S_PARM: FAIL
 test VIDIOC_G_FBUF: OK (Not Supported)
 test VIDIOC_G_FMT: OK
 test VIDIOC_TRY_FMT: OK
 test VIDIOC_S_FMT: OK
 test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)

 Codec ioctls:
 test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
 test VIDIOC_G_ENC_INDEX: OK (Not Supported)
 test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

 Buffer ioctls:
 info: test buftype Video Capture
 test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
 test VIDIOC_EXPBUF: OK
 test read/write: OK (Not Supported)
 Video Capture:
 Buffer: 0 Sequence: 0 Field: Interlaced Timestamp: 
 145.509130s
 Buffer: 1 Sequence: 0 Field: Interlaced Timestamp: 
 145.549125s
 Buffer: 2 Sequence: 0 Field: Interlaced Timestamp: 
 145.589148s
 Buffer: 3 Sequence: 0 Field: Interlaced Timestamp: 
 145.629106s
 Buffer: 0 Sequence: 0 Field: Interlaced Timestamp: 
 145.669110s

Re: [PATCH v5 41/49] media: davinci: vpif_capture: drop unneeded module params

2014-05-23 Thread Prabhakar Lad
Hi Mauro,

On Sat, May 24, 2014 at 4:08 AM, Mauro Carvalho Chehab
m.che...@samsung.com wrote:
 Em Fri, 16 May 2014 19:03:47 +0530
 Lad, Prabhakar prabhakar.cse...@gmail.com escreveu:

 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com

 -ENOPATCHDESCRIPTION

 Why to remove those parameters? Please _ALWAYS_ describe your patches.

My bad! will take care next time.

Thanks,
--Prabhakar Lad

 My crystal ball is malfunctioning today, so I was unable to scry the
 reasons for this patch.

 Thanks,
 Mauro

 ---
  drivers/media/platform/davinci/vpif_capture.c |   54 
 +
  drivers/media/platform/davinci/vpif_capture.h |   11 -
  2 files changed, 1 insertion(+), 64 deletions(-)

 diff --git a/drivers/media/platform/davinci/vpif_capture.c 
 b/drivers/media/platform/davinci/vpif_capture.c
 index d452eaf..e967cf7 100644
 --- a/drivers/media/platform/davinci/vpif_capture.c
 +++ b/drivers/media/platform/davinci/vpif_capture.c
 @@ -38,32 +38,10 @@ MODULE_VERSION(VPIF_CAPTURE_VERSION);
   v4l2_dbg(level, debug, vpif_obj.v4l2_dev, fmt, ## arg)

  static int debug = 1;
 -static u32 ch0_numbuffers = 3;
 -static u32 ch1_numbuffers = 3;
 -static u32 ch0_bufsize = 1920 * 1080 * 2;
 -static u32 ch1_bufsize = 720 * 576 * 2;

  module_param(debug, int, 0644);
 -module_param(ch0_numbuffers, uint, S_IRUGO);
 -module_param(ch1_numbuffers, uint, S_IRUGO);
 -module_param(ch0_bufsize, uint, S_IRUGO);
 -module_param(ch1_bufsize, uint, S_IRUGO);

  MODULE_PARM_DESC(debug, Debug level 0-1);
 -MODULE_PARM_DESC(ch2_numbuffers, Channel0 buffer count (default:3));
 -MODULE_PARM_DESC(ch3_numbuffers, Channel1 buffer count (default:3));
 -MODULE_PARM_DESC(ch2_bufsize, Channel0 buffer size (default:1920 x 1080 x 
 2));
 -MODULE_PARM_DESC(ch3_bufsize, Channel1 buffer size (default:720 x 576 x 
 2));
 -
 -static struct vpif_config_params config_params = {
 - .min_numbuffers = 3,
 - .numbuffers[0] = 3,
 - .numbuffers[1] = 3,
 - .min_bufsize[0] = 720 * 480 * 2,
 - .min_bufsize[1] = 720 * 480 * 2,
 - .channel_bufsize[0] = 1920 * 1080 * 2,
 - .channel_bufsize[1] = 720 * 576 * 2,
 -};

  #define VPIF_DRIVER_NAME vpif_capture

 @@ -609,9 +587,6 @@ static void vpif_config_format(struct channel_obj *ch)
   vpif_dbg(2, debug, vpif_config_format\n);

   common-fmt.fmt.pix.field = V4L2_FIELD_ANY;
 - common-fmt.fmt.pix.sizeimage
 - = config_params.channel_bufsize[ch-channel_id];
 -
   if (ch-vpifparams.iface.if_type == VPIF_IF_RAW_BAYER)
   common-fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_SBGGR8;
   else
 @@ -1358,36 +1333,9 @@ static struct v4l2_file_operations vpif_fops = {
   */
  static int initialize_vpif(void)
  {
 - int err = 0, i, j;
 + int err, i, j;
   int free_channel_objects_index;

 - /* Default number of buffers should be 3 */
 - if ((ch0_numbuffers  0) 
 - (ch0_numbuffers  config_params.min_numbuffers))
 - ch0_numbuffers = config_params.min_numbuffers;
 - if ((ch1_numbuffers  0) 
 - (ch1_numbuffers  config_params.min_numbuffers))
 - ch1_numbuffers = config_params.min_numbuffers;
 -
 - /* Set buffer size to min buffers size if it is invalid */
 - if (ch0_bufsize  config_params.min_bufsize[VPIF_CHANNEL0_VIDEO])
 - ch0_bufsize =
 - config_params.min_bufsize[VPIF_CHANNEL0_VIDEO];
 - if (ch1_bufsize  config_params.min_bufsize[VPIF_CHANNEL1_VIDEO])
 - ch1_bufsize =
 - config_params.min_bufsize[VPIF_CHANNEL1_VIDEO];
 -
 - config_params.numbuffers[VPIF_CHANNEL0_VIDEO] = ch0_numbuffers;
 - config_params.numbuffers[VPIF_CHANNEL1_VIDEO] = ch1_numbuffers;
 - if (ch0_numbuffers) {
 - config_params.channel_bufsize[VPIF_CHANNEL0_VIDEO]
 - = ch0_bufsize;
 - }
 - if (ch1_numbuffers) {
 - config_params.channel_bufsize[VPIF_CHANNEL1_VIDEO]
 - = ch1_bufsize;
 - }
 -
   /* Allocate memory for six channel objects */
   for (i = 0; i  VPIF_CAPTURE_MAX_DEVICES; i++) {
   vpif_obj.dev[i] =
 diff --git a/drivers/media/platform/davinci/vpif_capture.h 
 b/drivers/media/platform/davinci/vpif_capture.h
 index 4960504..537076a 100644
 --- a/drivers/media/platform/davinci/vpif_capture.h
 +++ b/drivers/media/platform/davinci/vpif_capture.h
 @@ -125,16 +125,5 @@ struct vpif_device {
   struct vpif_capture_config *config;
  };

 -struct vpif_config_params {
 - u8 min_numbuffers;
 - u8 numbuffers[VPIF_CAPTURE_NUM_CHANNELS];
 - s8 device_type;
 - u32 min_bufsize[VPIF_CAPTURE_NUM_CHANNELS];
 - u32 channel_bufsize[VPIF_CAPTURE_NUM_CHANNELS];
 - u8 default_device[VPIF_CAPTURE_NUM_CHANNELS];
 - u32 video_limit[VPIF_CAPTURE_NUM_CHANNELS];
 - u8 max_device_type;
 -};
 -
  #endif   /* End of __KERNEL__ */
  #endif

Re: [PATCH] mm: non-atomically mark page accessed during page cache allocation where possible -fix

2014-05-21 Thread Prabhakar Lad
On Wed, May 21, 2014 at 1:04 AM, Andrew Morton
 wrote:
> On Tue, 20 May 2014 16:49:00 +0100 Mel Gorman  wrote:
>
>> Prabhakar Lad reported the following problem
>>
>>   I see following issue on DA850 evm,
>>   git bisect points me to
>>   commit id: 975c3a671f11279441006a29a19f55ccc15fb320
>>   ( mm: non-atomically mark page accessed during page cache allocation
>>   where possible)
>>
>>   Unable to handle kernel paging request at virtual address 30e03501
>>   pgd = c68cc000
>>   [30e03501] *pgd=
>>   Internal error: Oops: 1 [#1] PREEMPT ARM
>>   Modules linked in:
>>   CPU: 0 PID: 1015 Comm: network.sh Not tainted 3.15.0-rc5-00323-g975c3a6 #9
>>   task: c70c4e00 ti: c73d task.ti: c73d
>>   PC is at init_page_accessed+0xc/0x24
>>   LR is at shmem_write_begin+0x54/0x60
>>   pc : []lr : []psr: 2013
>>   sp : c73d1d90  ip : c73d1da0  fp : c73d1d9c
>>   r10: c73d1dec  r9 :   r8 : 
>>   r7 : c73d1e6c  r6 : c694d7bc  r5 : ffe4  r4 : c73d1dec
>>   r3 : c73d  r2 : 0001  r1 :   r0 : 30e03501
>>   Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
>>   Control: 0005317f  Table: c68cc000  DAC: 0015
>>   Process network.sh (pid: 1015, stack limit = 0xc73d01c0)
>>
>> pagep is set but not pointing to anywhere valid as it's an uninitialised
>> stack variable. This patch is a fix to
>> mm-non-atomically-mark-page-accessed-during-page-cache-allocation-where-possible.patch
>>
>> ...
>>
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -2459,7 +2459,7 @@ ssize_t generic_perform_write(struct file *file,
>>   flags |= AOP_FLAG_UNINTERRUPTIBLE;
>>
>>   do {
>> - struct page *page;
>> + struct page *page = NULL;
>>   unsigned long offset;   /* Offset into pagecache page */
>>   unsigned long bytes;/* Bytes to write to page */
>>   size_t copied;  /* Bytes copied from user */
>
> Well not really.  generic_perform_write() only touches *page if
> ->write_begin() returned "success", which is reasonable behavior.
>
> I'd say you mucked up shmem_write_begin() - it runs
> init_page_accessed() even if shmem_getpage() returned an error.  It
> shouldn't be doing that.
>
> This?
>
> From: Andrew Morton 
> Subject: mm/shmem.c: don't run init_page_accessed() against an uninitialised 
> pointer
>
> If shmem_getpage() returned an error then it didn't necessarily initialise
> *pagep.  So shmem_write_begin() shouldn't be playing with *pagep in this
> situation.
>
> Fixes an oops when "mm: non-atomically mark page accessed during page
> cache allocation where possible" (quite reasonably) left *pagep
> uninitialized.
>
> Reported-by: Prabhakar Lad 
> Cc: Johannes Weiner 
> Cc: Vlastimil Babka 
> Cc: Jan Kara 
> Cc: Michal Hocko 
> Cc: Hugh Dickins 
> Cc: Peter Zijlstra 
> Cc: Dave Hansen 
> Cc: Mel Gorman 
> Signed-off-by: Andrew Morton 
> ---
>
>  mm/shmem.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -puN 
> mm/shmem.c~mm-non-atomically-mark-page-accessed-during-page-cache-allocation-where-possiblefix-2
>  mm/shmem.c
> --- 
> a/mm/shmem.c~mm-non-atomically-mark-page-accessed-during-page-cache-allocation-where-possiblefix-2
> +++ a/mm/shmem.c
> @@ -1376,7 +1376,7 @@ shmem_write_begin(struct file *file, str
> struct inode *inode = mapping->host;
> pgoff_t index = pos >> PAGE_CACHE_SHIFT;
> ret = shmem_getpage(inode, index, pagep, SGP_WRITE, NULL);
> -   if (*pagep)
> +   if (ret == 0 && *pagep)
> init_page_accessed(*pagep);
> return ret;
>  }

Reported-and-Tested-by: Lad, Prabhakar 

Regards,
--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] mm: non-atomically mark page accessed during page cache allocation where possible -fix

2014-05-21 Thread Prabhakar Lad
On Wed, May 21, 2014 at 1:04 AM, Andrew Morton
a...@linux-foundation.org wrote:
 On Tue, 20 May 2014 16:49:00 +0100 Mel Gorman mgor...@suse.de wrote:

 Prabhakar Lad reported the following problem

   I see following issue on DA850 evm,
   git bisect points me to
   commit id: 975c3a671f11279441006a29a19f55ccc15fb320
   ( mm: non-atomically mark page accessed during page cache allocation
   where possible)

   Unable to handle kernel paging request at virtual address 30e03501
   pgd = c68cc000
   [30e03501] *pgd=
   Internal error: Oops: 1 [#1] PREEMPT ARM
   Modules linked in:
   CPU: 0 PID: 1015 Comm: network.sh Not tainted 3.15.0-rc5-00323-g975c3a6 #9
   task: c70c4e00 ti: c73d task.ti: c73d
   PC is at init_page_accessed+0xc/0x24
   LR is at shmem_write_begin+0x54/0x60
   pc : [c0088aa0]lr : [c00923e8]psr: 2013
   sp : c73d1d90  ip : c73d1da0  fp : c73d1d9c
   r10: c73d1dec  r9 :   r8 : 
   r7 : c73d1e6c  r6 : c694d7bc  r5 : ffe4  r4 : c73d1dec
   r3 : c73d  r2 : 0001  r1 :   r0 : 30e03501
   Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
   Control: 0005317f  Table: c68cc000  DAC: 0015
   Process network.sh (pid: 1015, stack limit = 0xc73d01c0)

 pagep is set but not pointing to anywhere valid as it's an uninitialised
 stack variable. This patch is a fix to
 mm-non-atomically-mark-page-accessed-during-page-cache-allocation-where-possible.patch

 ...

 --- a/mm/filemap.c
 +++ b/mm/filemap.c
 @@ -2459,7 +2459,7 @@ ssize_t generic_perform_write(struct file *file,
   flags |= AOP_FLAG_UNINTERRUPTIBLE;

   do {
 - struct page *page;
 + struct page *page = NULL;
   unsigned long offset;   /* Offset into pagecache page */
   unsigned long bytes;/* Bytes to write to page */
   size_t copied;  /* Bytes copied from user */

 Well not really.  generic_perform_write() only touches *page if
 -write_begin() returned success, which is reasonable behavior.

 I'd say you mucked up shmem_write_begin() - it runs
 init_page_accessed() even if shmem_getpage() returned an error.  It
 shouldn't be doing that.

 This?

 From: Andrew Morton a...@linux-foundation.org
 Subject: mm/shmem.c: don't run init_page_accessed() against an uninitialised 
 pointer

 If shmem_getpage() returned an error then it didn't necessarily initialise
 *pagep.  So shmem_write_begin() shouldn't be playing with *pagep in this
 situation.

 Fixes an oops when mm: non-atomically mark page accessed during page
 cache allocation where possible (quite reasonably) left *pagep
 uninitialized.

 Reported-by: Prabhakar Lad prabhakar.cse...@gmail.com
 Cc: Johannes Weiner han...@cmpxchg.org
 Cc: Vlastimil Babka vba...@suse.cz
 Cc: Jan Kara j...@suse.cz
 Cc: Michal Hocko mho...@suse.cz
 Cc: Hugh Dickins hu...@google.com
 Cc: Peter Zijlstra pet...@infradead.org
 Cc: Dave Hansen dave.han...@intel.com
 Cc: Mel Gorman mgor...@suse.de
 Signed-off-by: Andrew Morton a...@linux-foundation.org
 ---

  mm/shmem.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff -puN 
 mm/shmem.c~mm-non-atomically-mark-page-accessed-during-page-cache-allocation-where-possiblefix-2
  mm/shmem.c
 --- 
 a/mm/shmem.c~mm-non-atomically-mark-page-accessed-during-page-cache-allocation-where-possiblefix-2
 +++ a/mm/shmem.c
 @@ -1376,7 +1376,7 @@ shmem_write_begin(struct file *file, str
 struct inode *inode = mapping-host;
 pgoff_t index = pos  PAGE_CACHE_SHIFT;
 ret = shmem_getpage(inode, index, pagep, SGP_WRITE, NULL);
 -   if (*pagep)
 +   if (ret == 0  *pagep)
 init_page_accessed(*pagep);
 return ret;
  }

Reported-and-Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Regards,
--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 1/2] media: davinci: vpif capture: upgrade the driver with v4l offerings

2014-05-14 Thread Prabhakar Lad
   test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>> test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>>
>> Buffer ioctls:
>> test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>> fail: v4l2-test-buffers.cpp(506): q.has_expbuf()
>
> This is weird. I'm not sure why this happens since you seem to have 
> VB2_DMABUF set
> and also vb2_ioctl_expbuf.
>
>> test VIDIOC_EXPBUF: FAIL
>>
>> Total: 38, Succeeded: 35, Failed: 3, Warnings: 0
>
> Also test with 'v4l2-compliance -s' (streaming). The '-i' option is available 
> to
> test streaming from a specific input.
>
BTW the output is with -s option set.

Thanks,
--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 1/2] media: davinci: vpif capture: upgrade the driver with v4l offerings

2014-05-14 Thread Prabhakar Lad
Hi Hans,

Thanks for the review.

On Mon, May 12, 2014 at 3:20 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi Prabhakar,

 Thanks for the patch, but I have a few comments...

 On 05/12/2014 10:58 AM, Lad, Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 This patch upgrades the vpif display driver with
 v4l helpers, this patch does the following,

 1: initialize the vb2 queue and context at the time of probe
 and removes context at remove() callback.
 2: uses vb2_ioctl_*() helpers.
 3: uses vb2_fop_*() helpers.
 4: uses SIMPLE_DEV_PM_OPS.
 5: uses vb2_ioctl_*() helpers.
 6: vidioc_g/s_priority is now handled by v4l core.
 7: removed driver specific fh and now using one provided by v4l.
 8: fixes checkpatch warnings.

 This really packs too much in one patch. At the very least 1, 4, 6 and
 8 can be done in separate patches. 2 and 5 are duplicates, BTW. The way
 it is now makes this hard to review. So it would be much appreciated if
 you can split it up.

Ok, I have started working on splitting them up.


 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---

 root@da850-omapl138-evm:/usr# ./v4l2-compliance -d /dev/video0 -i -s -v
 Driver Info:
 Driver name   : vpif_capture
 vpif_capture vpif_capture: =  START STATUS  =

 Bus info  : platform:vpif_capture
 Drivervpif_capture vpif_capture: ==  END STATUS  
 ==
  version: 3.15.0
 Capabilities  : 0x8401
 Video Capture
 Streaming
 Device Capabilities
 Device Caps   : 0x0401
 Video Capture
 Streaming

 Compliance test for device /dev/video0 (not using libv4l2):

 Required ioctls:
 test VIDIOC_QUERYCAP: OK

 Allow for multiple opens:
 test second video open: OK
 test VIDIOC_QUERYCAP: OK
 test VIDIOC_G/S_PRIORITY: OK

 Debug ioctls:
 test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
 test VIDIOC_LOG_STATUS: OK

 Input ioctls:
 test VIDIOC_G/S_TUNER: OK (Not Supported)
 test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
 test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
 test VIDIOC_ENUMAUDIO: OK (Not Supported)
 test VIDIOC_G/S/ENUMINPUT: OK
 test VIDIOC_G/S_AUDIO: OK (Not Supported)
 Inputs: 1 Audio Inputs: 0 Tuners: 0

 Output ioctls:
 test VIDIOC_G/S_MODULATOR: OK (Not Supported)
 test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
 test VIDIOC_ENUMAUDOUT: OK (Not Supported)
 test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
 test VIDIOC_G/S_AUDOUT: OK (Not Supported)
 Outputs: 0 Audio Outputs: 0 Modulators: 0

 Input/Output configuration ioctls:
 test VIDIOC_ENUM/G/S/QUERY_STD: OK
 test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
 test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
 test VIDIOC_G/S_EDID: OK (Not Supported)

 Test input 0:

 Control ioctls:
 test VIDIOC_QUERYCTRL/MENU: OK (Not Supported)
 test VIDIOC_G/S_CTRL: OK (Not Supported)
 test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
 test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
 test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
 Standard Controls: 0 Private Controls: 0

 Format ioctls:
 info: found 1 formats for buftype 1
 test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
 fail: v4l2-test-formats.cpp(1003): cap-readbuffers

 Just set readbuffers to the minimum number of buffers that queue_setup uses.


OK.

 test VIDIOC_G/S_PARM: FAIL
 test VIDIOC_G_FBUF: OK (Not Supported)
 fail: v4l2-test-formats.cpp(405): !pix.sizeimage

 Highly dubious. You need to investigate this!

Fixed it.

 test VIDIOC_G_FMT: FAIL
 test VIDIOC_TRY_FMT: OK (Not Supported)
 test VIDIOC_S_FMT: OK (Not Supported)
 test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)

 Codec ioctls:
 test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
 test VIDIOC_G_ENC_INDEX: OK (Not Supported)
 test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

 Buffer ioctls:
 test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
 fail: v4l2-test-buffers.cpp(506): q.has_expbuf()

 This is weird. I'm not sure why this happens since you seem to have 
 VB2_DMABUF set
 and also vb2_ioctl_expbuf.

 test VIDIOC_EXPBUF: FAIL

 Total: 38, Succeeded: 35, Failed: 3, Warnings: 0

 Also test with 'v4l2-compliance -s' (streaming). The '-i' option is available 
 to
 test streaming from a specific input.

BTW the output is with -s option set.

Thanks,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-kernel

Re: [PATCH v2] ARM: configs: drop CONFIG_COMMON_CLK_DEBUG

2014-05-09 Thread Prabhakar Lad
Hi Santosh,

On Fri, May 9, 2014 at 1:01 AM, Santosh Shilimkar
 wrote:
> On Tuesday 29 April 2014 03:07 AM, Lad Prabhakar wrote:
>> From: "Lad, Prabhakar" 
>>
>> this patch removes COMMON_CLK_DEBUG config option
>> from defconfig file as this config option is obsolete.
>>
>> Signed-off-by: Lad, Prabhakar 
>> ---
>>  Changes for v2:
>>  a> Dropped imx defconfig files as suggested by Shawn.
>>
>
>>  arch/arm/configs/keystone_defconfig |1 -
> Am picking the keystone part from this patch. Thanks.
> You might want split this patch so that others can pick it
> up
>
Thanks for picking it up.

Thanks,
--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] mdio_bus: fix devm_mdiobus_alloc_size export

2014-05-09 Thread Prabhakar Lad
On Thu, May 8, 2014 at 8:16 PM, Arnd Bergmann  wrote:
> commit 6d48f44b7b2 "mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free"
> introduced a new function devm_mdiobus_alloc_size() but added an export
> for a different function devm_mdiobus_alloc(), which was obviously
> a simple mistake that leads to  build error whenever this function is
> used from a loadable module:
>
> ERROR: "devm_mdiobus_alloc_size" [drivers/net/ethernet/ti/davinci_mdio.ko] 
> undefined!
>
> Signed-off-by: Arnd Bergmann 
> Cc: Grygorii Strashko 
> Cc: Florian Fainelli 
> Cc: Sergei Shtylyov 
> Cc: Lad, Prabhakar 
> Cc: David S. Miller 
> Cc: net...@vger.kernel.org

Acked-by: Lad, Prabhakar 

Thanks,
--Prabhakar Lad

> ---
>  drivers/net/phy/mdio_bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 68a9a38..a628496 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -117,7 +117,7 @@ struct mii_bus *devm_mdiobus_alloc_size(struct device 
> *dev, int sizeof_priv)
>
> return bus;
>  }
> -EXPORT_SYMBOL_GPL(devm_mdiobus_alloc);
> +EXPORT_SYMBOL_GPL(devm_mdiobus_alloc_size);
>
>  /**
>   * devm_mdiobus_free - Resource-managed mdiobus_free()
> --
> 1.8.3.2
>
--
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] mdio_bus: fix devm_mdiobus_alloc_size export

2014-05-09 Thread Prabhakar Lad
On Thu, May 8, 2014 at 8:16 PM, Arnd Bergmann a...@arndb.de wrote:
 commit 6d48f44b7b2 mdio_bus: implement devm_mdiobus_alloc/devm_mdiobus_free
 introduced a new function devm_mdiobus_alloc_size() but added an export
 for a different function devm_mdiobus_alloc(), which was obviously
 a simple mistake that leads to  build error whenever this function is
 used from a loadable module:

 ERROR: devm_mdiobus_alloc_size [drivers/net/ethernet/ti/davinci_mdio.ko] 
 undefined!

 Signed-off-by: Arnd Bergmann a...@arndb.de
 Cc: Grygorii Strashko grygorii.stras...@ti.com
 Cc: Florian Fainelli f.faine...@gmail.com
 Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
 Cc: Lad, Prabhakar prabhakar.cse...@gmail.com
 Cc: David S. Miller da...@davemloft.net
 Cc: net...@vger.kernel.org

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Thanks,
--Prabhakar Lad

 ---
  drivers/net/phy/mdio_bus.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
 index 68a9a38..a628496 100644
 --- a/drivers/net/phy/mdio_bus.c
 +++ b/drivers/net/phy/mdio_bus.c
 @@ -117,7 +117,7 @@ struct mii_bus *devm_mdiobus_alloc_size(struct device 
 *dev, int sizeof_priv)

 return bus;
  }
 -EXPORT_SYMBOL_GPL(devm_mdiobus_alloc);
 +EXPORT_SYMBOL_GPL(devm_mdiobus_alloc_size);

  /**
   * devm_mdiobus_free - Resource-managed mdiobus_free()
 --
 1.8.3.2

--
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 v2] ARM: configs: drop CONFIG_COMMON_CLK_DEBUG

2014-05-09 Thread Prabhakar Lad
Hi Santosh,

On Fri, May 9, 2014 at 1:01 AM, Santosh Shilimkar
santosh.shilim...@ti.com wrote:
 On Tuesday 29 April 2014 03:07 AM, Lad Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 this patch removes COMMON_CLK_DEBUG config option
 from defconfig file as this config option is obsolete.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  Changes for v2:
  a Dropped imx defconfig files as suggested by Shawn.


  arch/arm/configs/keystone_defconfig |1 -
 Am picking the keystone part from this patch. Thanks.
 You might want split this patch so that others can pick it
 up

Thanks for picking it up.

Thanks,
--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/


  1   2   3   4   5   6   7   8   9   10   >