[PATCH] fbdev: goldfishfb: use devres api
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
: 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
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_*()
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
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
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
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
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
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
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
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
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_*()
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
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_*()
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_*()
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_*()
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_*()
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
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_*()
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
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
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_*()
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
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
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
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
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
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
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
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
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_*()
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
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
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_*()
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_*()
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_*()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/