* [PATCH 0/4] HDPVR series of patches to replace Apr 22 patch @ 2013-04-25 9:59 Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() Leonid Kegulskiy ` (3 more replies) 0 siblings, 4 replies; 7+ messages in thread From: Leonid Kegulskiy @ 2013-04-25 9:59 UTC (permalink / raw) To: hverkuil; +Cc: Leonid Kegulskiy, Linux Media Mailing List Hi Hans, This series of patches replace the previous patch sent on Apr 22: [PATCH] [media] hdpvr_ error handling and alloc abuse cleanup. Thank you, -Leo. Leonid Kegulskiy (4): [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() [media] hdpvr: Added some error handling in hdpvr_start_streaming() [media] hdpvr: Cleaned up error handling drivers/media/usb/hdpvr/hdpvr-control.c | 20 ++------- drivers/media/usb/hdpvr/hdpvr-core.c | 8 ---- drivers/media/usb/hdpvr/hdpvr-video.c | 70 +++++++++++++++++-------------- drivers/media/usb/hdpvr/hdpvr.h | 2 +- 4 files changed, 43 insertions(+), 57 deletions(-) -- 1.7.10.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() 2013-04-25 9:59 [PATCH 0/4] HDPVR series of patches to replace Apr 22 patch Leonid Kegulskiy @ 2013-04-25 9:59 ` Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() Leonid Kegulskiy ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Leonid Kegulskiy @ 2013-04-25 9:59 UTC (permalink / raw) To: hverkuil; +Cc: Leonid Kegulskiy, Linux Media Mailing List Signed-off-by: Leonid Kegulskiy <leo@lumanate.com> --- drivers/media/usb/hdpvr/hdpvr-core.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c b/drivers/media/usb/hdpvr/hdpvr-core.c index 8247c19..cb69405 100644 --- a/drivers/media/usb/hdpvr/hdpvr-core.c +++ b/drivers/media/usb/hdpvr/hdpvr-core.c @@ -220,7 +220,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) { int ret; u8 *buf; - struct hdpvr_video_info *vidinf; if (device_authorization(dev)) return -EACCES; @@ -242,13 +241,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) "control request returned %d\n", ret); mutex_unlock(&dev->usbc_mutex); - vidinf = get_video_info(dev); - if (!vidinf) - v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev, - "no valid video signal or device init failed\n"); - else - kfree(vidinf); - /* enable fan and bling leds */ mutex_lock(&dev->usbc_mutex); buf[0] = 0x1; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() 2013-04-25 9:59 [PATCH 0/4] HDPVR series of patches to replace Apr 22 patch Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() Leonid Kegulskiy @ 2013-04-25 9:59 ` Leonid Kegulskiy 2013-05-10 8:29 ` Hans Verkuil 2013-04-25 9:59 ` [PATCH 3/4] [media] hdpvr: Added some error handling in hdpvr_start_streaming() Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 4/4] [media] hdpvr: Cleaned up error handling Leonid Kegulskiy 3 siblings, 1 reply; 7+ messages in thread From: Leonid Kegulskiy @ 2013-04-25 9:59 UTC (permalink / raw) To: hverkuil; +Cc: Leonid Kegulskiy, Linux Media Mailing List Signed-off-by: Leonid Kegulskiy <leo@lumanate.com> --- drivers/media/usb/hdpvr/hdpvr-control.c | 21 ++++-------- drivers/media/usb/hdpvr/hdpvr-video.c | 54 +++++++++++++++---------------- drivers/media/usb/hdpvr/hdpvr.h | 2 +- 3 files changed, 34 insertions(+), 43 deletions(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c index ae8f229..5265b75 100644 --- a/drivers/media/usb/hdpvr/hdpvr-control.c +++ b/drivers/media/usb/hdpvr/hdpvr-control.c @@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf) return ret < 0 ? ret : 0; } -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf) { - struct hdpvr_video_info *vidinf = NULL; -#ifdef HDPVR_DEBUG - char print_buf[15]; -#endif int ret; - vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL); - if (!vidinf) { - v4l2_err(&dev->v4l2_dev, "out of memory\n"); - goto err; - } - mutex_lock(&dev->usbc_mutex); ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0), @@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #ifdef HDPVR_DEBUG if (hdpvr_debug & MSG_INFO) { + char print_buf[15]; hex_dump_to_buffer(dev->usbc_buf, 5, 16, 1, print_buf, sizeof(print_buf), 0); v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev, @@ -82,12 +73,12 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #endif mutex_unlock(&dev->usbc_mutex); + /* preserve original behavior - fail if no signal is detected */ if (!vidinf->width || !vidinf->height || !vidinf->fps) { - kfree(vidinf); - vidinf = NULL; + ret = -EFAULT; } -err: - return vidinf; + + return ret < 0 ? ret : 0; } int get_input_lines_info(struct hdpvr_device *dev) diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index 774ba0e..d9eb8e1 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -277,20 +277,19 @@ error: static int hdpvr_start_streaming(struct hdpvr_device *dev) { int ret; - struct hdpvr_video_info *vidinf; + struct hdpvr_video_info vidinf; if (dev->status == STATUS_STREAMING) return 0; else if (dev->status != STATUS_IDLE) return -EAGAIN; - vidinf = get_video_info(dev); + ret = get_video_info(dev, &vidinf); - if (vidinf) { + if (!ret) { v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev, - "video signal: %dx%d@%dhz\n", vidinf->width, - vidinf->height, vidinf->fps); - kfree(vidinf); + "video signal: %dx%d@%dhz\n", vidinf.width, + vidinf.height, vidinf.fps); /* start streaming 2 request */ ret = usb_control_msg(dev->udev, @@ -606,21 +605,22 @@ static int vidioc_g_std(struct file *file, void *_fh, static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a) { struct hdpvr_device *dev = video_drvdata(file); - struct hdpvr_video_info *vid_info; + struct hdpvr_video_info vid_info; struct hdpvr_fh *fh = _fh; + int ret; *a = V4L2_STD_ALL; if (dev->options.video_input == HDPVR_COMPONENT) return fh->legacy_mode ? 0 : -ENODATA; - vid_info = get_video_info(dev); - if (vid_info == NULL) + ret = get_video_info(dev, &vid_info); + if (ret) return 0; - if (vid_info->width == 720 && - (vid_info->height == 480 || vid_info->height == 576)) { - *a = (vid_info->height == 480) ? + if (vid_info.width == 720 && + (vid_info.height == 480 || vid_info.height == 576)) { + *a = (vid_info.height == 480) ? V4L2_STD_525_60 : V4L2_STD_625_50; } - kfree(vid_info); + return 0; } @@ -665,7 +665,7 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, { struct hdpvr_device *dev = video_drvdata(file); struct hdpvr_fh *fh = _fh; - struct hdpvr_video_info *vid_info; + struct hdpvr_video_info vid_info; bool interlaced; int ret = 0; int i; @@ -673,10 +673,10 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, fh->legacy_mode = false; if (dev->options.video_input) return -ENODATA; - vid_info = get_video_info(dev); - if (vid_info == NULL) + ret = get_video_info(dev, &vid_info); + if (ret) return -ENOLCK; - interlaced = vid_info->fps <= 30; + interlaced = vid_info.fps <= 30; for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) { const struct v4l2_bt_timings *bt = &hdpvr_dv_timings[i].bt; unsigned hsize; @@ -688,17 +688,17 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, bt->il_vfrontporch + bt->il_vsync + bt->il_vbackporch + bt->height; fps = (unsigned)bt->pixelclock / (hsize * vsize); - if (bt->width != vid_info->width || - bt->height != vid_info->height || + if (bt->width != vid_info.width || + bt->height != vid_info.height || bt->interlaced != interlaced || - (fps != vid_info->fps && fps + 1 != vid_info->fps)) + (fps != vid_info.fps && fps + 1 != vid_info.fps)) continue; *timings = hdpvr_dv_timings[i]; break; } if (i == ARRAY_SIZE(hdpvr_dv_timings)) ret = -ERANGE; - kfree(vid_info); + return ret; } @@ -988,6 +988,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh, { struct hdpvr_device *dev = video_drvdata(file); struct hdpvr_fh *fh = _fh; + int ret; /* * The original driver would always returns the current detected @@ -1000,14 +1001,13 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh, * last set format. */ if (fh->legacy_mode) { - struct hdpvr_video_info *vid_info; + struct hdpvr_video_info vid_info; - vid_info = get_video_info(dev); - if (!vid_info) + ret = get_video_info(dev, &vid_info); + if (ret) return -EFAULT; - f->fmt.pix.width = vid_info->width; - f->fmt.pix.height = vid_info->height; - kfree(vid_info); + f->fmt.pix.width = vid_info.width; + f->fmt.pix.height = vid_info.height; } else { f->fmt.pix.width = dev->width; f->fmt.pix.height = dev->height; diff --git a/drivers/media/usb/hdpvr/hdpvr.h b/drivers/media/usb/hdpvr/hdpvr.h index 1478f3d..808ea7a 100644 --- a/drivers/media/usb/hdpvr/hdpvr.h +++ b/drivers/media/usb/hdpvr/hdpvr.h @@ -303,7 +303,7 @@ int hdpvr_set_audio(struct hdpvr_device *dev, u8 input, int hdpvr_config_call(struct hdpvr_device *dev, uint value, unsigned char valbuf); -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev); +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vid_info); /* :0 s b8 81 1800 0003 0003 3 < */ /* :0 0 3 = 0301ff */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() 2013-04-25 9:59 ` [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() Leonid Kegulskiy @ 2013-05-10 8:29 ` Hans Verkuil 0 siblings, 0 replies; 7+ messages in thread From: Hans Verkuil @ 2013-05-10 8:29 UTC (permalink / raw) To: Leonid Kegulskiy; +Cc: Linux Media Mailing List Hi Leonid, I've accepted patch 1 & 3, but this patch needs a bit more work, see below: On Thu April 25 2013 11:59:55 Leonid Kegulskiy wrote: > Signed-off-by: Leonid Kegulskiy <leo@lumanate.com> > --- > drivers/media/usb/hdpvr/hdpvr-control.c | 21 ++++-------- > drivers/media/usb/hdpvr/hdpvr-video.c | 54 +++++++++++++++---------------- > drivers/media/usb/hdpvr/hdpvr.h | 2 +- > 3 files changed, 34 insertions(+), 43 deletions(-) > > diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c > index ae8f229..5265b75 100644 > --- a/drivers/media/usb/hdpvr/hdpvr-control.c > +++ b/drivers/media/usb/hdpvr/hdpvr-control.c > @@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf) > return ret < 0 ? ret : 0; > } > > -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) > +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf) > { > - struct hdpvr_video_info *vidinf = NULL; > -#ifdef HDPVR_DEBUG > - char print_buf[15]; > -#endif > int ret; > > - vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL); > - if (!vidinf) { > - v4l2_err(&dev->v4l2_dev, "out of memory\n"); > - goto err; > - } > - > mutex_lock(&dev->usbc_mutex); > ret = usb_control_msg(dev->udev, > usb_rcvctrlpipe(dev->udev, 0), > @@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) > > #ifdef HDPVR_DEBUG > if (hdpvr_debug & MSG_INFO) { > + char print_buf[15]; > hex_dump_to_buffer(dev->usbc_buf, 5, 16, 1, print_buf, > sizeof(print_buf), 0); > v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev, > @@ -82,12 +73,12 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) > #endif > mutex_unlock(&dev->usbc_mutex); > > + /* preserve original behavior - fail if no signal is detected */ > if (!vidinf->width || !vidinf->height || !vidinf->fps) { This check fails if usb_control_msg returns a value >= 0 but != 5 since in that case the contents of vidinf is undefined. I would rewrite this so that any return value != 5 results in an error. Regards, Hans > - kfree(vidinf); > - vidinf = NULL; > + ret = -EFAULT; > } > -err: > - return vidinf; > + > + return ret < 0 ? ret : 0; > } > > int get_input_lines_info(struct hdpvr_device *dev) > diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c > index 774ba0e..d9eb8e1 100644 > --- a/drivers/media/usb/hdpvr/hdpvr-video.c > +++ b/drivers/media/usb/hdpvr/hdpvr-video.c > @@ -277,20 +277,19 @@ error: > static int hdpvr_start_streaming(struct hdpvr_device *dev) > { > int ret; > - struct hdpvr_video_info *vidinf; > + struct hdpvr_video_info vidinf; > > if (dev->status == STATUS_STREAMING) > return 0; > else if (dev->status != STATUS_IDLE) > return -EAGAIN; > > - vidinf = get_video_info(dev); > + ret = get_video_info(dev, &vidinf); > > - if (vidinf) { > + if (!ret) { > v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev, > - "video signal: %dx%d@%dhz\n", vidinf->width, > - vidinf->height, vidinf->fps); > - kfree(vidinf); > + "video signal: %dx%d@%dhz\n", vidinf.width, > + vidinf.height, vidinf.fps); > > /* start streaming 2 request */ > ret = usb_control_msg(dev->udev, > @@ -606,21 +605,22 @@ static int vidioc_g_std(struct file *file, void *_fh, > static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a) > { > struct hdpvr_device *dev = video_drvdata(file); > - struct hdpvr_video_info *vid_info; > + struct hdpvr_video_info vid_info; > struct hdpvr_fh *fh = _fh; > + int ret; > > *a = V4L2_STD_ALL; > if (dev->options.video_input == HDPVR_COMPONENT) > return fh->legacy_mode ? 0 : -ENODATA; > - vid_info = get_video_info(dev); > - if (vid_info == NULL) > + ret = get_video_info(dev, &vid_info); > + if (ret) > return 0; > - if (vid_info->width == 720 && > - (vid_info->height == 480 || vid_info->height == 576)) { > - *a = (vid_info->height == 480) ? > + if (vid_info.width == 720 && > + (vid_info.height == 480 || vid_info.height == 576)) { > + *a = (vid_info.height == 480) ? > V4L2_STD_525_60 : V4L2_STD_625_50; > } > - kfree(vid_info); > + > return 0; > } > > @@ -665,7 +665,7 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, > { > struct hdpvr_device *dev = video_drvdata(file); > struct hdpvr_fh *fh = _fh; > - struct hdpvr_video_info *vid_info; > + struct hdpvr_video_info vid_info; > bool interlaced; > int ret = 0; > int i; > @@ -673,10 +673,10 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, > fh->legacy_mode = false; > if (dev->options.video_input) > return -ENODATA; > - vid_info = get_video_info(dev); > - if (vid_info == NULL) > + ret = get_video_info(dev, &vid_info); > + if (ret) > return -ENOLCK; > - interlaced = vid_info->fps <= 30; > + interlaced = vid_info.fps <= 30; > for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) { > const struct v4l2_bt_timings *bt = &hdpvr_dv_timings[i].bt; > unsigned hsize; > @@ -688,17 +688,17 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, > bt->il_vfrontporch + bt->il_vsync + bt->il_vbackporch + > bt->height; > fps = (unsigned)bt->pixelclock / (hsize * vsize); > - if (bt->width != vid_info->width || > - bt->height != vid_info->height || > + if (bt->width != vid_info.width || > + bt->height != vid_info.height || > bt->interlaced != interlaced || > - (fps != vid_info->fps && fps + 1 != vid_info->fps)) > + (fps != vid_info.fps && fps + 1 != vid_info.fps)) > continue; > *timings = hdpvr_dv_timings[i]; > break; > } > if (i == ARRAY_SIZE(hdpvr_dv_timings)) > ret = -ERANGE; > - kfree(vid_info); > + > return ret; > } > > @@ -988,6 +988,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh, > { > struct hdpvr_device *dev = video_drvdata(file); > struct hdpvr_fh *fh = _fh; > + int ret; > > /* > * The original driver would always returns the current detected > @@ -1000,14 +1001,13 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh, > * last set format. > */ > if (fh->legacy_mode) { > - struct hdpvr_video_info *vid_info; > + struct hdpvr_video_info vid_info; > > - vid_info = get_video_info(dev); > - if (!vid_info) > + ret = get_video_info(dev, &vid_info); > + if (ret) > return -EFAULT; > - f->fmt.pix.width = vid_info->width; > - f->fmt.pix.height = vid_info->height; > - kfree(vid_info); > + f->fmt.pix.width = vid_info.width; > + f->fmt.pix.height = vid_info.height; > } else { > f->fmt.pix.width = dev->width; > f->fmt.pix.height = dev->height; > diff --git a/drivers/media/usb/hdpvr/hdpvr.h b/drivers/media/usb/hdpvr/hdpvr.h > index 1478f3d..808ea7a 100644 > --- a/drivers/media/usb/hdpvr/hdpvr.h > +++ b/drivers/media/usb/hdpvr/hdpvr.h > @@ -303,7 +303,7 @@ int hdpvr_set_audio(struct hdpvr_device *dev, u8 input, > int hdpvr_config_call(struct hdpvr_device *dev, uint value, > unsigned char valbuf); > > -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev); > +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vid_info); > > /* :0 s b8 81 1800 0003 0003 3 < */ > /* :0 0 3 = 0301ff */ > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/4] [media] hdpvr: Added some error handling in hdpvr_start_streaming() 2013-04-25 9:59 [PATCH 0/4] HDPVR series of patches to replace Apr 22 patch Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() Leonid Kegulskiy @ 2013-04-25 9:59 ` Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 4/4] [media] hdpvr: Cleaned up error handling Leonid Kegulskiy 3 siblings, 0 replies; 7+ messages in thread From: Leonid Kegulskiy @ 2013-04-25 9:59 UTC (permalink / raw) To: hverkuil; +Cc: Leonid Kegulskiy, Linux Media Mailing List Signed-off-by: Leonid Kegulskiy <leo@lumanate.com> --- drivers/media/usb/hdpvr/hdpvr-video.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index d9eb8e1..2d02b49 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -297,8 +297,12 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev) 0xb8, 0x38, 0x1, 0, NULL, 0, 8000); v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev, "encoder start control request returned %d\n", ret); + if (ret < 0) + return ret; - hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00); + ret = hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00); + if (ret) + return ret; dev->status = STATUS_STREAMING; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] [media] hdpvr: Cleaned up error handling 2013-04-25 9:59 [PATCH 0/4] HDPVR series of patches to replace Apr 22 patch Leonid Kegulskiy ` (2 preceding siblings ...) 2013-04-25 9:59 ` [PATCH 3/4] [media] hdpvr: Added some error handling in hdpvr_start_streaming() Leonid Kegulskiy @ 2013-04-25 9:59 ` Leonid Kegulskiy 3 siblings, 0 replies; 7+ messages in thread From: Leonid Kegulskiy @ 2013-04-25 9:59 UTC (permalink / raw) To: hverkuil; +Cc: Leonid Kegulskiy, Linux Media Mailing List Changed vidioc_g_fmt_vid_cap() implementation not to return -EFAULT when video lock is not detected, but return empty width/height fields (legacy mode only). This new behavior is supported by MythTV. Signed-off-by: Leonid Kegulskiy <leo@lumanate.com> --- drivers/media/usb/hdpvr/hdpvr-control.c | 5 ----- drivers/media/usb/hdpvr/hdpvr-video.c | 10 +++++++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c index 5265b75..16d2d64 100644 --- a/drivers/media/usb/hdpvr/hdpvr-control.c +++ b/drivers/media/usb/hdpvr/hdpvr-control.c @@ -73,11 +73,6 @@ int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf) #endif mutex_unlock(&dev->usbc_mutex); - /* preserve original behavior - fail if no signal is detected */ - if (!vidinf->width || !vidinf->height || !vidinf->fps) { - ret = -EFAULT; - } - return ret < 0 ? ret : 0; } diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index 2d02b49..5e8d6c2 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -285,8 +285,10 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev) return -EAGAIN; ret = get_video_info(dev, &vidinf); + if (ret) /* device is dead */ + return ret; /* let the caller know */ - if (!ret) { + if (vidinf.width && vidinf.height) { v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev, "video signal: %dx%d@%dhz\n", vidinf.width, vidinf.height, vidinf.fps); @@ -618,7 +620,7 @@ static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a) return fh->legacy_mode ? 0 : -ENODATA; ret = get_video_info(dev, &vid_info); if (ret) - return 0; + return ret; if (vid_info.width == 720 && (vid_info.height == 480 || vid_info.height == 576)) { *a = (vid_info.height == 480) ? @@ -679,6 +681,8 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, return -ENODATA; ret = get_video_info(dev, &vid_info); if (ret) + return ret; + if (vid_info.fps == 0) return -ENOLCK; interlaced = vid_info.fps <= 30; for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) { @@ -1009,7 +1013,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh, ret = get_video_info(dev, &vid_info); if (ret) - return -EFAULT; + return ret; f->fmt.pix.width = vid_info.width; f->fmt.pix.height = vid_info.height; } else { -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() @ 2013-05-13 11:10 Leonid Kegulskiy 2013-05-13 11:10 ` [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() Leonid Kegulskiy 0 siblings, 1 reply; 7+ messages in thread From: Leonid Kegulskiy @ 2013-05-13 11:10 UTC (permalink / raw) To: hverkuil; +Cc: Leonid Kegulskiy, Linux Media Mailing List Signed-off-by: Leonid Kegulskiy <leo@lumanate.com> --- drivers/media/usb/hdpvr/hdpvr-core.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c b/drivers/media/usb/hdpvr/hdpvr-core.c index 8247c19..cb69405 100644 --- a/drivers/media/usb/hdpvr/hdpvr-core.c +++ b/drivers/media/usb/hdpvr/hdpvr-core.c @@ -220,7 +220,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) { int ret; u8 *buf; - struct hdpvr_video_info *vidinf; if (device_authorization(dev)) return -EACCES; @@ -242,13 +241,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev) "control request returned %d\n", ret); mutex_unlock(&dev->usbc_mutex); - vidinf = get_video_info(dev); - if (!vidinf) - v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev, - "no valid video signal or device init failed\n"); - else - kfree(vidinf); - /* enable fan and bling leds */ mutex_lock(&dev->usbc_mutex); buf[0] = 0x1; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() 2013-05-13 11:10 [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() Leonid Kegulskiy @ 2013-05-13 11:10 ` Leonid Kegulskiy 0 siblings, 0 replies; 7+ messages in thread From: Leonid Kegulskiy @ 2013-05-13 11:10 UTC (permalink / raw) To: hverkuil; +Cc: Leonid Kegulskiy, Linux Media Mailing List Signed-off-by: Leonid Kegulskiy <leo@lumanate.com> --- drivers/media/usb/hdpvr/hdpvr-control.c | 25 ++++++-------- drivers/media/usb/hdpvr/hdpvr-video.c | 54 +++++++++++++++---------------- drivers/media/usb/hdpvr/hdpvr.h | 2 +- 3 files changed, 37 insertions(+), 44 deletions(-) diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c index ae8f229..7d1bfb3 100644 --- a/drivers/media/usb/hdpvr/hdpvr-control.c +++ b/drivers/media/usb/hdpvr/hdpvr-control.c @@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf) return ret < 0 ? ret : 0; } -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf) { - struct hdpvr_video_info *vidinf = NULL; -#ifdef HDPVR_DEBUG - char print_buf[15]; -#endif int ret; - vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL); - if (!vidinf) { - v4l2_err(&dev->v4l2_dev, "out of memory\n"); - goto err; - } - mutex_lock(&dev->usbc_mutex); ret = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0), @@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #ifdef HDPVR_DEBUG if (hdpvr_debug & MSG_INFO) { + char print_buf[15]; hex_dump_to_buffer(dev->usbc_buf, 5, 16, 1, print_buf, sizeof(print_buf), 0); v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev, @@ -82,12 +73,14 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev) #endif mutex_unlock(&dev->usbc_mutex); - if (!vidinf->width || !vidinf->height || !vidinf->fps) { - kfree(vidinf); - vidinf = NULL; + if ((ret>0 && ret!=5) ||/* fail if unexpected byte count returned */ + !vidinf->width || /* preserve original behavior - */ + !vidinf->height || /* fail if no signal is detected */ + !vidinf->fps) { + ret = -EFAULT; } -err: - return vidinf; + + return ret < 0 ? ret : 0; } int get_input_lines_info(struct hdpvr_device *dev) diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c index 774ba0e..d9eb8e1 100644 --- a/drivers/media/usb/hdpvr/hdpvr-video.c +++ b/drivers/media/usb/hdpvr/hdpvr-video.c @@ -277,20 +277,19 @@ error: static int hdpvr_start_streaming(struct hdpvr_device *dev) { int ret; - struct hdpvr_video_info *vidinf; + struct hdpvr_video_info vidinf; if (dev->status == STATUS_STREAMING) return 0; else if (dev->status != STATUS_IDLE) return -EAGAIN; - vidinf = get_video_info(dev); + ret = get_video_info(dev, &vidinf); - if (vidinf) { + if (!ret) { v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev, - "video signal: %dx%d@%dhz\n", vidinf->width, - vidinf->height, vidinf->fps); - kfree(vidinf); + "video signal: %dx%d@%dhz\n", vidinf.width, + vidinf.height, vidinf.fps); /* start streaming 2 request */ ret = usb_control_msg(dev->udev, @@ -606,21 +605,22 @@ static int vidioc_g_std(struct file *file, void *_fh, static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a) { struct hdpvr_device *dev = video_drvdata(file); - struct hdpvr_video_info *vid_info; + struct hdpvr_video_info vid_info; struct hdpvr_fh *fh = _fh; + int ret; *a = V4L2_STD_ALL; if (dev->options.video_input == HDPVR_COMPONENT) return fh->legacy_mode ? 0 : -ENODATA; - vid_info = get_video_info(dev); - if (vid_info == NULL) + ret = get_video_info(dev, &vid_info); + if (ret) return 0; - if (vid_info->width == 720 && - (vid_info->height == 480 || vid_info->height == 576)) { - *a = (vid_info->height == 480) ? + if (vid_info.width == 720 && + (vid_info.height == 480 || vid_info.height == 576)) { + *a = (vid_info.height == 480) ? V4L2_STD_525_60 : V4L2_STD_625_50; } - kfree(vid_info); + return 0; } @@ -665,7 +665,7 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, { struct hdpvr_device *dev = video_drvdata(file); struct hdpvr_fh *fh = _fh; - struct hdpvr_video_info *vid_info; + struct hdpvr_video_info vid_info; bool interlaced; int ret = 0; int i; @@ -673,10 +673,10 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, fh->legacy_mode = false; if (dev->options.video_input) return -ENODATA; - vid_info = get_video_info(dev); - if (vid_info == NULL) + ret = get_video_info(dev, &vid_info); + if (ret) return -ENOLCK; - interlaced = vid_info->fps <= 30; + interlaced = vid_info.fps <= 30; for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) { const struct v4l2_bt_timings *bt = &hdpvr_dv_timings[i].bt; unsigned hsize; @@ -688,17 +688,17 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh, bt->il_vfrontporch + bt->il_vsync + bt->il_vbackporch + bt->height; fps = (unsigned)bt->pixelclock / (hsize * vsize); - if (bt->width != vid_info->width || - bt->height != vid_info->height || + if (bt->width != vid_info.width || + bt->height != vid_info.height || bt->interlaced != interlaced || - (fps != vid_info->fps && fps + 1 != vid_info->fps)) + (fps != vid_info.fps && fps + 1 != vid_info.fps)) continue; *timings = hdpvr_dv_timings[i]; break; } if (i == ARRAY_SIZE(hdpvr_dv_timings)) ret = -ERANGE; - kfree(vid_info); + return ret; } @@ -988,6 +988,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh, { struct hdpvr_device *dev = video_drvdata(file); struct hdpvr_fh *fh = _fh; + int ret; /* * The original driver would always returns the current detected @@ -1000,14 +1001,13 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh, * last set format. */ if (fh->legacy_mode) { - struct hdpvr_video_info *vid_info; + struct hdpvr_video_info vid_info; - vid_info = get_video_info(dev); - if (!vid_info) + ret = get_video_info(dev, &vid_info); + if (ret) return -EFAULT; - f->fmt.pix.width = vid_info->width; - f->fmt.pix.height = vid_info->height; - kfree(vid_info); + f->fmt.pix.width = vid_info.width; + f->fmt.pix.height = vid_info.height; } else { f->fmt.pix.width = dev->width; f->fmt.pix.height = dev->height; diff --git a/drivers/media/usb/hdpvr/hdpvr.h b/drivers/media/usb/hdpvr/hdpvr.h index 1478f3d..808ea7a 100644 --- a/drivers/media/usb/hdpvr/hdpvr.h +++ b/drivers/media/usb/hdpvr/hdpvr.h @@ -303,7 +303,7 @@ int hdpvr_set_audio(struct hdpvr_device *dev, u8 input, int hdpvr_config_call(struct hdpvr_device *dev, uint value, unsigned char valbuf); -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev); +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vid_info); /* :0 s b8 81 1800 0003 0003 3 < */ /* :0 0 3 = 0301ff */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-05-13 11:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-04-25 9:59 [PATCH 0/4] HDPVR series of patches to replace Apr 22 patch Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() Leonid Kegulskiy 2013-05-10 8:29 ` Hans Verkuil 2013-04-25 9:59 ` [PATCH 3/4] [media] hdpvr: Added some error handling in hdpvr_start_streaming() Leonid Kegulskiy 2013-04-25 9:59 ` [PATCH 4/4] [media] hdpvr: Cleaned up error handling Leonid Kegulskiy 2013-05-13 11:10 [PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init() Leonid Kegulskiy 2013-05-13 11:10 ` [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info() Leonid Kegulskiy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.