Re: [PATCH 1/9] [media] pci: flexcop: Remove redundant pci_set_drvdata
On 09/20/2013 10:36 AM, Sachin Kamat wrote: Driver core sets driver data to NULL upon failure or remove. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Cc: Patrick Boettcher patrick.boettc...@desy.de For this patch series: Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/pci/b2c2/flexcop-pci.c |2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/media/pci/b2c2/flexcop-pci.c b/drivers/media/pci/b2c2/flexcop-pci.c index 447afbd..8b5e0b3 100644 --- a/drivers/media/pci/b2c2/flexcop-pci.c +++ b/drivers/media/pci/b2c2/flexcop-pci.c @@ -319,7 +319,6 @@ static int flexcop_pci_init(struct flexcop_pci *fc_pci) err_pci_iounmap: pci_iounmap(fc_pci-pdev, fc_pci-io_mem); - pci_set_drvdata(fc_pci-pdev, NULL); err_pci_release_regions: pci_release_regions(fc_pci-pdev); err_pci_disable_device: @@ -332,7 +331,6 @@ static void flexcop_pci_exit(struct flexcop_pci *fc_pci) if (fc_pci-init_state FC_PCI_INIT) { free_irq(fc_pci-pdev-irq, fc_pci); pci_iounmap(fc_pci-pdev, fc_pci-io_mem); - pci_set_drvdata(fc_pci-pdev, NULL); pci_release_regions(fc_pci-pdev); pci_disable_device(fc_pci-pdev); } -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] [media] pci: cx88-alsa: Use module_pci_driver
On 09/20/2013 10:32 AM, Sachin Kamat wrote: module_pci_driver removes some boilerplate and makes code simpler. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org For this patch series: Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/pci/cx88/cx88-alsa.c | 25 + 1 file changed, 1 insertion(+), 24 deletions(-) diff --git a/drivers/media/pci/cx88/cx88-alsa.c b/drivers/media/pci/cx88/cx88-alsa.c index 05428bf..11d0692 100644 --- a/drivers/media/pci/cx88/cx88-alsa.c +++ b/drivers/media/pci/cx88/cx88-alsa.c @@ -949,27 +949,4 @@ static struct pci_driver cx88_audio_pci_driver = { .remove = cx88_audio_finidev, }; -/ - LINUX MODULE INIT - / - -/* - * module init - */ -static int __init cx88_audio_init(void) -{ - printk(KERN_INFO cx2388x alsa driver version %s loaded\n, -CX88_VERSION); - return pci_register_driver(cx88_audio_pci_driver); -} - -/* - * module remove - */ -static void __exit cx88_audio_fini(void) -{ - pci_unregister_driver(cx88_audio_pci_driver); -} - -module_init(cx88_audio_init); -module_exit(cx88_audio_fini); +module_pci_driver(cx88_audio_pci_driver); -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] [media] radio-sf16fmr2: Remove redundant dev_set_drvdata
On 09/20/2013 10:37 AM, Sachin Kamat wrote: Driver core sets driver data to NULL upon failure or remove. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Acked-by: Hans Verkuil hans.verk...@cisco.com Regards, Hans --- drivers/media/radio/radio-sf16fmr2.c |1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/radio/radio-sf16fmr2.c b/drivers/media/radio/radio-sf16fmr2.c index f1e3714..448cac9 100644 --- a/drivers/media/radio/radio-sf16fmr2.c +++ b/drivers/media/radio/radio-sf16fmr2.c @@ -295,7 +295,6 @@ static void fmr2_remove(struct fmr2 *fmr2) static int fmr2_isa_remove(struct device *pdev, unsigned int ndev) { fmr2_remove(dev_get_drvdata(pdev)); - dev_set_drvdata(pdev, NULL); return 0; } -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v2] [RFC] v4l2: Support for multiple selections
Extend v4l2 selection API to support multiple selection areas, this way sensors that support multiple readout areas can work with multiple areas of insterest. The struct v4l2_selection and v4l2_subdev_selection has been extented with a new field rectangles. If it is value is different than zero the pr array is used instead of the r field. A new structure v4l2_ext_rect has been defined, containing 4 reserved fields for future improvements, as suggested by Hans. Two helper functiona are also added, to help the drivers that support a single selection. This Patch ONLY adds the modification to the core. Once it is agreed, a new version including changes on the drivers that handle the selection api will come. struct v4l2_selection has the same size on 32 and 64 bits, therefore I dont think that any change on v4l2-compat-ioctl32 is needed. ricardo@neopili:/tmp$ gcc kk.c -m32 ricardo@neopili:/tmp$ ./a.out Size of v4l2_selection=64 ricardo@neopili:/tmp$ gcc kk.c ricardo@neopili:/tmp$ ./a.out Size of v4l2_selection=64 This patch includes all the comments by Hans Verkuil. Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com --- drivers/media/v4l2-core/v4l2-common.c | 39 +++ drivers/media/v4l2-core/v4l2-ioctl.c | 37 - include/media/v4l2-common.h | 4 include/uapi/linux/v4l2-subdev.h | 10 +++-- include/uapi/linux/videodev2.h| 21 +++ 5 files changed, 100 insertions(+), 11 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index a95e5e2..007ab32 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -886,3 +886,42 @@ void v4l2_get_timestamp(struct timeval *tv) tv-tv_usec = ts.tv_nsec / NSEC_PER_USEC; } EXPORT_SYMBOL_GPL(v4l2_get_timestamp); + +int v4l2_selection_get_rect(struct v4l2_selection *s, struct v4l2_ext_rect *r) +{ + if (s-rectangles 1) + return -EINVAL; + if (s-rectangles == 1) { + *r = s-pr[0]; + return 0; + } + if (s-r.width 0 || s-r.height 0) + return -EINVAL; + r-left = s-r.left; + r-top = s-r.top; + r-width = s-r.width; + r-height = s-r.height; + memset(r-reserved, 0, sizeof(r-reserved)); + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_get_rect); + +int v4l2_selection_set_rect(struct v4l2_ext_rect *r, struct v4l2_selection *s) +{ + if (s-rectangles == 0) { + if (s-r.width UINT_MAX || s-r.height UINT_MAX) + return -EINVAL; + s-r.left = r-left; + s-r.top = r-top; + s-r.width = r-width; + s-r.height = r-height; + return 0; + } + if (s-rectangles 1) { + s-pr[0] = *r; + s-rectangles = 1; + return 0; + } + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_set_rect); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 68e6b5e..29f7cf1 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -572,11 +572,22 @@ static void v4l_print_crop(const void *arg, bool write_only) static void v4l_print_selection(const void *arg, bool write_only) { const struct v4l2_selection *p = arg; + int i; - pr_cont(type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n, - prt_names(p-type, v4l2_type_names), - p-target, p-flags, - p-r.width, p-r.height, p-r.left, p-r.top); + if (p-rectangles == 0) + pr_cont(type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n, + prt_names(p-type, v4l2_type_names), + p-target, p-flags, + p-r.width, p-r.height, p-r.left, p-r.top); + else{ + pr_cont(type=%s, target=%d, flags=0x%x rectangles=%d\n, + prt_names(p-type, v4l2_type_names), + p-target, p-flags, p-rectangles); + for (i = 0; i p-rectangles; i++) + pr_cont(rectangle %d: wxh=%dx%d, x,y=%d,%d\n, + i, p-r.width, p-r.height, + p-r.left, p-r.top); + } } static void v4l_print_jpegcompression(const void *arg, bool write_only) @@ -1692,7 +1703,9 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops *ops, struct file *file, void *fh, void *arg) { struct v4l2_cropcap *p = arg; - struct v4l2_selection s = { .type = p-type }; + struct v4l2_selection s = { + .type = p-type, + }; int ret; if (ops-vidioc_cropcap) @@ -2253,6 +2266,20 @@ static int check_array_args(unsigned int cmd, void *parg, size_t *array_size, }
Re: [PATCH] v4l2: Support for multiple selections
Hello Hans I have just posted a new patch that only takes care of the core. Thanks! On Mon, Sep 30, 2013 at 2:11 PM, Hans Verkuil hverk...@xs4all.nl wrote: On 09/30/2013 01:17 PM, Ricardo Ribalda Delgado wrote: Hello Hans As allways thank you very much for your comments. On Mon, Sep 30, 2013 at 12:21 PM, Hans Verkuil hverk...@xs4all.nl wrote: Hi Ricardo, Sorry for the delayed review, but I'm finally catching up with my backlog. I've got a number of comments regarding this patch. I'm ignoring the platform driver patches for now until the core support is correct. On 09/16/2013 02:54 PM, Ricardo Ribalda Delgado wrote: From: Ricardo Ribalda ricardo.riba...@gmail.com Extend v4l2 selection API to support multiple selection areas, this way sensors that support multiple readout areas can work with multiple areas of insterest. The struct v4l2_selection and v4l2_subdev_selection has been extented with a new field rectangles. If it is value is different than zero the pr array is used instead of the r field. A new structure v4l2_ext_rect has been defined, containing 4 reserved fields for future improvements, as suggested by Hans. A new function in v4l2-comon (v4l2_selection_flat_struct) is in charge of converting a pr pointer with one item to a flatten struct. This function is used in all the old drivers that dont support multiple selections. Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com --- drivers/media/platform/exynos-gsc/gsc-m2m.c | 6 +++ drivers/media/platform/exynos4-is/fimc-capture.c | 6 +++ drivers/media/platform/exynos4-is/fimc-lite.c| 6 +++ drivers/media/platform/s3c-camif/camif-capture.c | 6 +++ drivers/media/platform/s5p-jpeg/jpeg-core.c | 3 ++ drivers/media/platform/s5p-tv/mixer_video.c | 6 +++ drivers/media/platform/soc_camera/soc_camera.c | 6 +++ drivers/media/v4l2-core/v4l2-common.c| 13 ++ drivers/media/v4l2-core/v4l2-ioctl.c | 54 +--- include/media/v4l2-common.h | 2 + include/uapi/linux/v4l2-subdev.h | 10 - include/uapi/linux/videodev2.h | 15 ++- 12 files changed, 122 insertions(+), 11 deletions(-) snip diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index a95e5e2..cd20567 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -886,3 +886,16 @@ void v4l2_get_timestamp(struct timeval *tv) tv-tv_usec = ts.tv_nsec / NSEC_PER_USEC; } EXPORT_SYMBOL_GPL(v4l2_get_timestamp); + +int v4l2_selection_flat_struct(struct v4l2_selection *s) +{ + if (s-rectangles == 0) + return 0; + + if (s-rectangles != 1) + return -EINVAL; + + s-r = s-pr[0].r; This would overwrite the pr pointer. Not a good idea. That was exactly the point. The helper function convert the multi_selection, ext_rect to the legacy struct. This way the drivers needed almost no modification, just a call to the helper at the beginning of the handler. That doesn't work: G and S_SELECTION are IOWR, so the driver can modify the rectangles and those will have to be passed back to userspace. So you cannot just change the contents of struct v4l2_selection. Otherwise we need your get_rect helper, and then a set_rect helper at every exit. If you think this is the way, then lets do it. Right now there are not too many drivers that supports selection, so it is right time to make such a decisions. I would make a helper function like this: int v4l2_selection_get_rect(struct v4l2_selection *s, struct v4l2_ext_rect *r) { if (s-rectangles 1) return -EINVAL; if (s-rectangles == 1) { *r = s-pr[0]; return 0; } if (s-r.width 0 || s-r.height 0) return -EINVAL; r-left = s-r.left; r-top = s-r.top; r-width = s-r.width; r-height = s-r.height; memset(r-reserved, 0, sizeof(r-reserved)); return 0; } See also my proposed v4l2_ext_rect definition below. + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_flat_struct); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 68e6b5e..fe92f6b 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -572,11 +572,22 @@ static void v4l_print_crop(const void *arg, bool write_only) static void v4l_print_selection(const void *arg, bool write_only) { const struct v4l2_selection *p = arg; + int i; - pr_cont(type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n, - prt_names(p-type, v4l2_type_names), - p-target, p-flags, - p-r.width, p-r.height, p-r.left, p-r.top); + if (p-rectangles == 0) + pr_cont(type=%s,
Re: [PATCH 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Hi Sakari, On Tuesday 01 October 2013 02:28:23 Sakari Ailus wrote: On Tue, Oct 01, 2013 at 01:21:58AM +0200, Laurent Pinchart wrote: On Tuesday 01 October 2013 02:08:47 Sakari Ailus wrote: On Fri, Sep 20, 2013 at 11:08:47PM +0200, Laurent Pinchart wrote: On Thursday 19 September 2013 01:01:05 Sakari Ailus wrote: Pads that set this flag must be connected by an active link for the entity to stream. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi Acked-by: Sylwester Nawrocki sylvester.nawro...@gmail.com [snip] What about If the pad is linked to any other pad, at least one of the links must be enabled for the entity to be able to stream. There could be temporary reasons (e.g. device configuration dependent) for the pad to need enabled links; the absence of the flag doesn't imply there is none. The flag has no effect on pads without connected links. Thinking about this again, I'd add before the comma: and this flag is set. And if you put it like that then the last sentence is redundat --- I'd drop it. What do you think? What about When this flag is set, if the pad is linked to any other pad then at least one of those links must be enabled for the entity to be able to stream. There could be temporary reasons (e.g. device configuration dependent) for the pad to need enabled links even when this flag isn't set; the absence of the flag doesn't imply there is none. The flag has no effect on pads without connected links. Feel free to drop the last sentence. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
af9013 i2c errors
Hello, I don't know if this is a bug, but I have problems with my device: http://www.linuxtv.org/wiki/index.php/KWorld_USB_Dual_DVB-T_TV_Stick_(DVB-T_399U) I am using it in a Debian Wheezy machine: Linux server 3.2.0-4-amd64 #1 SMP Debian 3.2.46-1+deb7u1 x86_64 GNU/Linux With this firmware: http://palosaari.fi/linux/v4l-dvb/firmware/af9015/5.1.0.0/dvb-usb-af9015.fw The problem is that I have glitches when watching channels. This is the kernel logs: # tail /var/log/messages Oct 1 11:01:35 server kernel: [149863.986812] af9013: I2C read failed reg:d07d Oct 1 11:01:41 server kernel: [149870.513600] af9013: I2C read failed reg:d2e6 Oct 1 11:02:03 server kernel: [149891.697286] af9013: I2C read failed reg:d2e6 Oct 1 11:02:17 server kernel: [149906.348441] af9013: I2C read failed reg:d333 Oct 1 11:02:24 server kernel: [149912.894103] af9013: I2C read failed reg:d507 Oct 1 11:02:36 server kernel: [149925.169533] af9013: I2C read failed reg:d2e6 Oct 1 11:03:15 server kernel: [149964.404593] af9013: I2C read failed reg:d2e5 Oct 1 11:03:22 server kernel: [149970.917832] af9013: I2C read failed reg:d507 Oct 1 11:04:13 server kernel: [150022.431228] af9013: I2C read failed reg:d07c Oct 1 11:04:20 server kernel: [150028.955682] af9013: I2C read failed reg:d2e5 Is this really a bug? Thanks. -- Josu Lazkano -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC v2] [RFC] v4l2: Support for multiple selections
On Tue 1 October 2013 10:26:56 Ricardo Ribalda Delgado wrote: Extend v4l2 selection API to support multiple selection areas, this way sensors that support multiple readout areas can work with multiple areas of insterest. The struct v4l2_selection and v4l2_subdev_selection has been extented with a new field rectangles. If it is value is different than zero the pr array is used instead of the r field. A new structure v4l2_ext_rect has been defined, containing 4 reserved fields for future improvements, as suggested by Hans. Two helper functiona are also added, to help the drivers that support a single selection. This Patch ONLY adds the modification to the core. Once it is agreed, a new version including changes on the drivers that handle the selection api will come. struct v4l2_selection has the same size on 32 and 64 bits, therefore I dont think that any change on v4l2-compat-ioctl32 is needed. Yes, you do. The pr pointer is 32-bit for a 32-bit application and the compat code is needed to convert it to a 64-bit pointer. The struct as a whole may have the same size, but the pointer in the union definitely has a different size. ricardo@neopili:/tmp$ gcc kk.c -m32 ricardo@neopili:/tmp$ ./a.out Size of v4l2_selection=64 ricardo@neopili:/tmp$ gcc kk.c ricardo@neopili:/tmp$ ./a.out Size of v4l2_selection=64 This patch includes all the comments by Hans Verkuil. Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com --- drivers/media/v4l2-core/v4l2-common.c | 39 +++ drivers/media/v4l2-core/v4l2-ioctl.c | 37 - include/media/v4l2-common.h | 4 include/uapi/linux/v4l2-subdev.h | 10 +++-- include/uapi/linux/videodev2.h| 21 +++ 5 files changed, 100 insertions(+), 11 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index a95e5e2..007ab32 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -886,3 +886,42 @@ void v4l2_get_timestamp(struct timeval *tv) tv-tv_usec = ts.tv_nsec / NSEC_PER_USEC; } EXPORT_SYMBOL_GPL(v4l2_get_timestamp); + +int v4l2_selection_get_rect(struct v4l2_selection *s, struct v4l2_ext_rect *r) +{ + if (s-rectangles 1) + return -EINVAL; + if (s-rectangles == 1) { + *r = s-pr[0]; + return 0; + } + if (s-r.width 0 || s-r.height 0) + return -EINVAL; + r-left = s-r.left; + r-top = s-r.top; + r-width = s-r.width; + r-height = s-r.height; + memset(r-reserved, 0, sizeof(r-reserved)); + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_get_rect); + +int v4l2_selection_set_rect(struct v4l2_ext_rect *r, struct v4l2_selection *s) +{ + if (s-rectangles == 0) { + if (s-r.width UINT_MAX || s-r.height UINT_MAX) + return -EINVAL; This makes no sense. You probably mean r-width INT_MAX, but I would just skip this whole test: it is the driver that will set these values, and you can assume that it is correct. You can never rely on what you get from userspace, but I think this is reasonably in this case to assume that the rectangle is a reasonable size. That also means that this can be a void function. + s-r.left = r-left; + s-r.top = r-top; + s-r.width = r-width; + s-r.height = r-height; + return 0; + } + if (s-rectangles 1) { + s-pr[0] = *r; + s-rectangles = 1; + return 0; + } + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_set_rect); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 68e6b5e..29f7cf1 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -572,11 +572,22 @@ static void v4l_print_crop(const void *arg, bool write_only) static void v4l_print_selection(const void *arg, bool write_only) { const struct v4l2_selection *p = arg; + int i; - pr_cont(type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n, - prt_names(p-type, v4l2_type_names), - p-target, p-flags, - p-r.width, p-r.height, p-r.left, p-r.top); + if (p-rectangles == 0) + pr_cont(type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n, + prt_names(p-type, v4l2_type_names), + p-target, p-flags, + p-r.width, p-r.height, p-r.left, p-r.top); + else{ + pr_cont(type=%s, target=%d, flags=0x%x rectangles=%d\n, + prt_names(p-type, v4l2_type_names), + p-target, p-flags, p-rectangles); + for (i = 0; i p-rectangles; i++) + pr_cont(rectangle %d: wxh=%dx%d,
Re: [PATCH 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Hi Laurent, On Tue, Oct 01, 2013 at 10:55:04AM +0200, Laurent Pinchart wrote: On Tuesday 01 October 2013 02:28:23 Sakari Ailus wrote: On Tue, Oct 01, 2013 at 01:21:58AM +0200, Laurent Pinchart wrote: On Tuesday 01 October 2013 02:08:47 Sakari Ailus wrote: On Fri, Sep 20, 2013 at 11:08:47PM +0200, Laurent Pinchart wrote: On Thursday 19 September 2013 01:01:05 Sakari Ailus wrote: Pads that set this flag must be connected by an active link for the entity to stream. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi Acked-by: Sylwester Nawrocki sylvester.nawro...@gmail.com [snip] What about If the pad is linked to any other pad, at least one of the links must be enabled for the entity to be able to stream. There could be temporary reasons (e.g. device configuration dependent) for the pad to need enabled links; the absence of the flag doesn't imply there is none. The flag has no effect on pads without connected links. Thinking about this again, I'd add before the comma: and this flag is set. And if you put it like that then the last sentence is redundat --- I'd drop it. What do you think? What about When this flag is set, if the pad is linked to any other pad then at least How about: If this flag is set and the pad is linked to any other pad, then... I think it's cleaner like that. one of those links must be enabled for the entity to be able to stream. There could be temporary reasons (e.g. device configuration dependent) for the pad to need enabled links even when this flag isn't set; the absence of the flag doesn't imply there is none. The flag has no effect on pads without connected links. Feel free to drop the last sentence. Thinking about it again, I'm fine keeping it. :-) -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] media: Add pad flag MEDIA_PAD_FL_MUST_CONNECT
Hi Sakari, On Tuesday 01 October 2013 12:17:21 Sakari Ailus wrote: On Tue, Oct 01, 2013 at 10:55:04AM +0200, Laurent Pinchart wrote: On Tuesday 01 October 2013 02:28:23 Sakari Ailus wrote: On Tue, Oct 01, 2013 at 01:21:58AM +0200, Laurent Pinchart wrote: On Tuesday 01 October 2013 02:08:47 Sakari Ailus wrote: On Fri, Sep 20, 2013 at 11:08:47PM +0200, Laurent Pinchart wrote: On Thursday 19 September 2013 01:01:05 Sakari Ailus wrote: Pads that set this flag must be connected by an active link for the entity to stream. Signed-off-by: Sakari Ailus sakari.ai...@iki.fi Acked-by: Sylwester Nawrocki sylvester.nawro...@gmail.com [snip] What about If the pad is linked to any other pad, at least one of the links must be enabled for the entity to be able to stream. There could be temporary reasons (e.g. device configuration dependent) for the pad to need enabled links; the absence of the flag doesn't imply there is none. The flag has no effect on pads without connected links. Thinking about this again, I'd add before the comma: and this flag is set. And if you put it like that then the last sentence is redundat --- I'd drop it. What do you think? What about When this flag is set, if the pad is linked to any other pad then at least How about: If this flag is set and the pad is linked to any other pad, then... I think it's cleaner like that. Fine with me. one of those links must be enabled for the entity to be able to stream. There could be temporary reasons (e.g. device configuration dependent) for the pad to need enabled links even when this flag isn't set; the absence of the flag doesn't imply there is none. The flag has no effect on pads without connected links. Feel free to drop the last sentence. Thinking about it again, I'm fine keeping it. :-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC v3] [RFC] v4l2: Support for multiple selections
Extend v4l2 selection API to support multiple selection areas, this way sensors that support multiple readout areas can work with multiple areas of insterest. The struct v4l2_selection and v4l2_subdev_selection has been extented with a new field rectangles. If it is value is different than zero the pr array is used instead of the r field. A new structure v4l2_ext_rect has been defined, containing 4 reserved fields for future improvements, as suggested by Hans. Two helper functiona are also added, to help the drivers that support a single selection. This Patch ONLY adds the modification to the core. Once it is agreed, a new version including changes on the drivers that handle the selection api will come. This patch includes all the comments by Hans Verkuil. Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com --- v3: -Changes on compat-ioctl32 -Remove checks on v4l2_selection_set_rect drivers/media/v4l2-core/v4l2-common.c | 36 +++ drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 63 +++ drivers/media/v4l2-core/v4l2-ioctl.c | 37 +--- include/media/v4l2-common.h | 6 +++ include/uapi/linux/v4l2-subdev.h | 10 - include/uapi/linux/videodev2.h| 21 +++-- 6 files changed, 162 insertions(+), 11 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index a95e5e2..f60a2ce 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -886,3 +886,39 @@ void v4l2_get_timestamp(struct timeval *tv) tv-tv_usec = ts.tv_nsec / NSEC_PER_USEC; } EXPORT_SYMBOL_GPL(v4l2_get_timestamp); + +int v4l2_selection_get_rect(const struct v4l2_selection *s, + struct v4l2_ext_rect *r) +{ + if (s-rectangles 1) + return -EINVAL; + if (s-rectangles == 1) { + *r = s-pr[0]; + return 0; + } + if (s-r.width 0 || s-r.height 0) + return -EINVAL; + r-left = s-r.left; + r-top = s-r.top; + r-width = s-r.width; + r-height = s-r.height; + memset(r-reserved, 0, sizeof(r-reserved)); + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_get_rect); + +void v4l2_selection_set_rect(struct v4l2_selection *s, + const struct v4l2_ext_rect *r) +{ + if (s-rectangles == 0) { + s-r.left = r-left; + s-r.top = r-top; + s-r.width = r-width; + s-r.height = r-height; + return; + } + s-pr[0] = *r; + s-rectangles = 1; + return; +} +EXPORT_SYMBOL_GPL(v4l2_selection_set_rect); diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index 8f7a6a4..36ed3c3 100644 --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c @@ -777,6 +777,54 @@ static int put_v4l2_subdev_edid32(struct v4l2_subdev_edid *kp, struct v4l2_subde return 0; } +struct v4l2_selection32 { + __u32 type; + __u32 target; + __u32 flags; + union { + struct v4l2_rectr; + compat_uptr_t *pr; + }; + __u32 rectangles; + __u32 reserved[8]; +} __packed; + +static int get_v4l2_selection32(struct v4l2_selection *kp, + struct v4l2_selection32 __user *up) +{ + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_selection32)) + || (copy_from_user(kp, up, sizeof(*kp + return -EFAULT; + + if (kp-rectangles) { + compat_uptr_t usr_ptr; + if (get_user(usr_ptr, up-pr)) + return -EFAULT; + kp-pr = compat_ptr(usr_ptr); + } + + return 0; + +} + +static int put_v4l2_selection32(struct v4l2_selection *kp, + struct v4l2_selection32 __user *up) +{ + compat_uptr_t usr_ptr = 0; + + if ((kp-rectangles) get_user(usr_ptr, up-pr)) + return -EFAULT; + + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_selection32)) + || (copy_to_user(kp, up, sizeof(*kp + return -EFAULT; + + if (kp-rectangles) + put_user(usr_ptr, up-pr); + + return 0; + +} #define VIDIOC_G_FMT32 _IOWR('V', 4, struct v4l2_format32) #define VIDIOC_S_FMT32 _IOWR('V', 5, struct v4l2_format32) @@ -796,6 +844,8 @@ static int put_v4l2_subdev_edid32(struct v4l2_subdev_edid *kp, struct v4l2_subde #defineVIDIOC_DQEVENT32_IOR ('V', 89, struct v4l2_event32) #define VIDIOC_CREATE_BUFS32 _IOWR('V', 92, struct v4l2_create_buffers32) #define
Re: [RFC v2] [RFC] v4l2: Support for multiple selections
Hello Hans Thanks for your comments I have just posted a new version. Regards! On Tue, Oct 1, 2013 at 11:13 AM, Hans Verkuil hverk...@xs4all.nl wrote: On Tue 1 October 2013 10:26:56 Ricardo Ribalda Delgado wrote: Extend v4l2 selection API to support multiple selection areas, this way sensors that support multiple readout areas can work with multiple areas of insterest. The struct v4l2_selection and v4l2_subdev_selection has been extented with a new field rectangles. If it is value is different than zero the pr array is used instead of the r field. A new structure v4l2_ext_rect has been defined, containing 4 reserved fields for future improvements, as suggested by Hans. Two helper functiona are also added, to help the drivers that support a single selection. This Patch ONLY adds the modification to the core. Once it is agreed, a new version including changes on the drivers that handle the selection api will come. struct v4l2_selection has the same size on 32 and 64 bits, therefore I dont think that any change on v4l2-compat-ioctl32 is needed. Yes, you do. The pr pointer is 32-bit for a 32-bit application and the compat code is needed to convert it to a 64-bit pointer. The struct as a whole may have the same size, but the pointer in the union definitely has a different size. ricardo@neopili:/tmp$ gcc kk.c -m32 ricardo@neopili:/tmp$ ./a.out Size of v4l2_selection=64 ricardo@neopili:/tmp$ gcc kk.c ricardo@neopili:/tmp$ ./a.out Size of v4l2_selection=64 This patch includes all the comments by Hans Verkuil. Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com --- drivers/media/v4l2-core/v4l2-common.c | 39 +++ drivers/media/v4l2-core/v4l2-ioctl.c | 37 - include/media/v4l2-common.h | 4 include/uapi/linux/v4l2-subdev.h | 10 +++-- include/uapi/linux/videodev2.h| 21 +++ 5 files changed, 100 insertions(+), 11 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-common.c b/drivers/media/v4l2-core/v4l2-common.c index a95e5e2..007ab32 100644 --- a/drivers/media/v4l2-core/v4l2-common.c +++ b/drivers/media/v4l2-core/v4l2-common.c @@ -886,3 +886,42 @@ void v4l2_get_timestamp(struct timeval *tv) tv-tv_usec = ts.tv_nsec / NSEC_PER_USEC; } EXPORT_SYMBOL_GPL(v4l2_get_timestamp); + +int v4l2_selection_get_rect(struct v4l2_selection *s, struct v4l2_ext_rect *r) +{ + if (s-rectangles 1) + return -EINVAL; + if (s-rectangles == 1) { + *r = s-pr[0]; + return 0; + } + if (s-r.width 0 || s-r.height 0) + return -EINVAL; + r-left = s-r.left; + r-top = s-r.top; + r-width = s-r.width; + r-height = s-r.height; + memset(r-reserved, 0, sizeof(r-reserved)); + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_get_rect); + +int v4l2_selection_set_rect(struct v4l2_ext_rect *r, struct v4l2_selection *s) +{ + if (s-rectangles == 0) { + if (s-r.width UINT_MAX || s-r.height UINT_MAX) + return -EINVAL; This makes no sense. You probably mean r-width INT_MAX, but I would just skip this whole test: it is the driver that will set these values, and you can assume that it is correct. You can never rely on what you get from userspace, but I think this is reasonably in this case to assume that the rectangle is a reasonable size. That also means that this can be a void function. + s-r.left = r-left; + s-r.top = r-top; + s-r.width = r-width; + s-r.height = r-height; + return 0; + } + if (s-rectangles 1) { + s-pr[0] = *r; + s-rectangles = 1; + return 0; + } + return 0; +} +EXPORT_SYMBOL_GPL(v4l2_selection_set_rect); diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index 68e6b5e..29f7cf1 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -572,11 +572,22 @@ static void v4l_print_crop(const void *arg, bool write_only) static void v4l_print_selection(const void *arg, bool write_only) { const struct v4l2_selection *p = arg; + int i; - pr_cont(type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n, - prt_names(p-type, v4l2_type_names), - p-target, p-flags, - p-r.width, p-r.height, p-r.left, p-r.top); + if (p-rectangles == 0) + pr_cont(type=%s, target=%d, flags=0x%x, wxh=%dx%d, x,y=%d,%d\n, + prt_names(p-type, v4l2_type_names), + p-target, p-flags, + p-r.width, p-r.height, p-r.left, p-r.top); + else{ + pr_cont(type=%s, target=%d, flags=0x%x rectangles=%d\n, + prt_names(p-type, v4l2_type_names), +
Re: [PATCH 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Hi Sakari, On Monday 23 September 2013 22:57:02 Sakari Ailus wrote: On Fri, Sep 20, 2013 at 10:54:22PM +0200, Laurent Pinchart wrote: @@ -248,21 +250,46 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity, if (!entity-ops || !entity-ops-link_validate) continue; + bitmap_zero(active, entity-num_pads); + bitmap_fill(has_no_links, entity-num_pads); + for (i = 0; i entity-num_links; i++) { struct media_link *link = entity-links[i]; - - /* Is this pad part of an enabled link? */ - if (!(link-flags MEDIA_LNK_FL_ENABLED)) - continue; - - /* Are we the sink or not? */ - if (link-sink-entity != entity) + struct media_pad *pad = link-sink-entity == entity + ? link-sink : link-source; What about aligning the ? to the = ? With that change, How about to the beginning of the next operand rather than =? (Think of adding parentheses around the rvalue of =.) I think it's fine as it was, but if it's to be changed then it should be aligned to link-sink-entity IMHO. :-) My preference goes for aligning the ? under the =, but I agree it's not logical from an rvalue point of view :-) I would favor aligning the ? under the l of link, but enough bikeshedding for now, please pick whichever solution you prefer :-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] media: Check for active links on pads with MEDIA_PAD_FL_MUST_CONNECT flag
Hi Laurent, On Tue, Oct 01, 2013 at 02:39:14PM +0200, Laurent Pinchart wrote: On Monday 23 September 2013 22:57:02 Sakari Ailus wrote: On Fri, Sep 20, 2013 at 10:54:22PM +0200, Laurent Pinchart wrote: @@ -248,21 +250,46 @@ __must_check int media_entity_pipeline_start(struct media_entity *entity, if (!entity-ops || !entity-ops-link_validate) continue; + bitmap_zero(active, entity-num_pads); + bitmap_fill(has_no_links, entity-num_pads); + for (i = 0; i entity-num_links; i++) { struct media_link *link = entity-links[i]; - - /* Is this pad part of an enabled link? */ - if (!(link-flags MEDIA_LNK_FL_ENABLED)) - continue; - - /* Are we the sink or not? */ - if (link-sink-entity != entity) + struct media_pad *pad = link-sink-entity == entity + ? link-sink : link-source; What about aligning the ? to the = ? With that change, How about to the beginning of the next operand rather than =? (Think of adding parentheses around the rvalue of =.) I think it's fine as it was, but if it's to be changed then it should be aligned to link-sink-entity IMHO. :-) My preference goes for aligning the ? under the =, but I agree it's not logical from an rvalue point of view :-) I would favor aligning the ? under the l of link, but enough bikeshedding for now, please pick whichever solution you prefer :-) ? goes under l of the link; agreed. I'll resend the set. -- Cheers, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 1/5] ARM: dts: Add MIPI PHY node to exynos4.dtsi
On 10/01/2013 07:28 AM, Kishon Vijay Abraham I wrote: On Sunday 29 September 2013 12:57 AM, Sylwester Nawrocki wrote: Add PHY provider node for the MIPI CSIS and MIPI DSIM PHYs. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Acked-by: Felipe Balbiba...@ti.com Can this patch be taken through exynos dt tree? Yes, that makes more sense indeed. Kukjin, would you mind taking this patch to your tree ? Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Hauppauge HVR-900 HD and HVR 930C-HD with si2165
On 01.10.2013 08:34, Matthias Schwarzott wrote: On 26.09.2013 16:54, Antti Palosaari wrote: On 25.09.2013 07:50, Matthias Schwarzott wrote: On 17.08.2013 13:30, Ulf wrote: Hi, I know the topic Hauppauge HVR-900 HD and HVR 930C-HD with si2165 demodulator was already discussed http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/40982 and http://permalink.gmane.org/gmane.linux.drivers.video-input-infrastructure/46266. Just for me as a confirmation nobody plans to work on a driver for si2165. Is there any chance how to push the development? Ulf Hi! I also bought one of these to find out it is not supported. But my plan is to try to write a driver for this. I want to get DVB-C working, but I also have DVB-T and analog reception available. My current status is I got it working in windows in qemu and did a usb snoop. I also have a second system to test it in windows vista directly on the hardware. Current status is documented here. http://www.linuxtv.org/wiki/index.php/Hauppauge_WinTV-HVR-930C-HD Until now I only have a component list summarized from this list. * Conexant http://www.linuxtv.org/wiki/index.php/Conexant CX231xx http://www.linuxtv.org/wiki/index.php/Conexant_CX2310x * Silicon Labs http://www.linuxtv.org/wiki/index.php?title=Silicon_Labsaction=editredlink=1 si2165 http://www.linuxtv.org/wiki/index.php/Silicon_Labs_si2165 (Multi-Standard DVB-T and DVB-C Demodulator) * NXP TDA18271 http://www.linuxtv.org/wiki/index.php/NXP/Philips_TDA182xx (silicon tuner IC, most likely i2c-addr: 0x60) * eeprom (windows driver reads 1kb, i2c-addr: 0x50) Is this correct? Did anyone open his device and can show pictures? I now need to know which component is at which i2c address. Windows driver does upload file hcw10mlD.rom of 16kb to device 0x44. I have opened it. There was similar sandwich PCB than used by rev1 too. So you cannot see all the chip unless you use metal saw to separate PCBs. PCB side A: TDA18271HDC2 16.000 MHz Si2165-GM 16.000 MHz PCB side B: 24C02H regards Antti Hi Antti, thanks for that information. The only real new information for me is the 16.000MHz xtal value. Sad to know that the other chips are hidden. I assigned more i2c addresses to functions, but not yet all (no idea if more addresses are real, or bad interpretations of snooped data). I now try to check what already works: - This is video via composite input. - Next is to try video via analog input - see I see if the tuner in general works in this device. In parallel I try to capture usb in different setups. 1. kvm+tcpdump (using usbmon) 2. usbsnoop on windows vista Only setup 1 does provide a real list of usb packets. Matthias, you likely try to do things too complex :) I am not going to comment analog side as I simply has no experience. Missing piece of code from the DTV point of view is only si2165 demod driver. My technique is to make successful tune one channel and take sniffs. From sniffs I generate C-code register writes (and sometimes reads too) using scripts. Reading that C-code is much more visual and easier than looking correct bytes from the raw sniffs. It is essential to find out from the sniffs what are tuner register writes, what are demod register writes and what are for USB-bridge itself. There may be some other chips which are needed to init in order to operate, like in cases I2C bus is connected through analog demodulator to digital demodulator. Usually it is rather trivial to make skeleton driver from the code generated from sniffs which just shows that single channel sniffs were taken. I have been looking simple example for reverse-engineer demodulator driver how-to blog post, but I haven't found suitable device yet. That was one device I looked, but I given-up as simplest sniff after parsing was over 1MB. Looks like there is multiple firmwares to download and also CX231xx usb protocol generates a lot of I/O = not very good example for simple how-to. Take a look of that post to see some practical example about sniffing and code generation. http://blog.palosaari.fi/2013/07/generating-rtl2832u-driver-code.html regards Antti -- http://palosaari.fi/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] media: rc: OF: Add Generic bindings for remote-control
Em Mon, 30 Sep 2013 09:27:02 +0100 Srinivas KANDAGATLA srinivas.kandaga...@st.com escreveu: On 27/09/13 14:57, Mauro Carvalho Chehab wrote: Em Fri, 27 Sep 2013 14:26:12 +0100 Srinivas KANDAGATLA srinivas.kandaga...@st.com escreveu: On 27/09/13 12:34, Mark Rutland wrote: + - rx-mode: Can be infrared or uhf. rx-mode should be present iff + the rx pins are wired up. I'm unsure on this. What if the device has multiple receivers that can be independently configured? What if it supports something other than infrared or uhf? What if a device can only be wired up as infrared? I'm not sure how generic these are, though we should certainly encourage bindings that can be described this way to be described in the same way. + - tx-mode: Can be infrared or uhf. tx-mode should be present iff + the tx pins are wired up. I have similar concerns here to those for the rx-mode property. Initially rx-mode and tx-mode sounded like more generic properties that's the reason I ended up in this route. But after this discussion it looks like its not really generic enough to cater all the use cases. It make sense for me to perfix st, for these properties in the st-rc driver rather than considering them as generic properties. Well, for sure the direction (TX, RX, both) is a generic property. I'd say that the level 1 protocol (IR, UHF, Bluetooth, ...) is also a generic property. Most remotes are IR, but there are some that are bluetooth, and your hardware is using UHF. Yes these are generic. Btw, we're even thinking on mapping HDMI-CEC remote controller RX/TX via the RC subsystem. So, another L1 protocol would be hdmi-cec. Ok. Yet, it seems unlikely that the very same remote controller IP would use a different protocol for RX and TX, while sharing the same registers. ST IRB block has one IR processor which has both TX and RX support and one UHF Processor which has RX support only. However the register map for all these support is in single IRB IP block. So the driver can configure the IP as TX in infrared and RX in uhf. This is supported in ST IRB IP. This case can not be represented in a single device tree node with l1-protocol and direction properties. IMHO, having tx-mode and rx-mode or tx-protocol and rx-protocol properties will give more flexibility. What do you think? Yeah, if they're using the same registers, then your proposal works better. I would prefer to not call it as just protocol, as IR has an upper layer protocol that defines how the bits are encoded, e. g. RC5, RC6, NEC, SONY, ..., with is what we generally call as protocol on rc-core. A proper naming for it is hard to find. Well, for IR/UHF, it is actually specifying the medium, but for Bluetooth, HDMI-CEC, it defines a protocol stack to be used, with covers not only the physical layer of the OSI model. Perhaps the better would be to call it as: tx-proto-stack/rx-proto-stack. So, for example, a hardware with hdmi-cec and infrared will actually have two remote controller devices. Eventually, the infrared being just RX, while hdmi-cec being bi-directional. So, IMHO, this could be mapped as l1_protocol (infrared, uhf, ...) and another one direction (rx, tx, bi-directional). Thanks, srini Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 1/5] ARM: dts: Add MIPI PHY node to exynos4.dtsi
On Wednesday 02 October 2013 02:43 AM, Sylwester Nawrocki wrote: On 10/01/2013 07:28 AM, Kishon Vijay Abraham I wrote: On Sunday 29 September 2013 12:57 AM, Sylwester Nawrocki wrote: Add PHY provider node for the MIPI CSIS and MIPI DSIM PHYs. Signed-off-by: Sylwester Nawrockis.nawro...@samsung.com Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com Acked-by: Felipe Balbiba...@ti.com Can this patch be taken through exynos dt tree? Yes, that makes more sense indeed. Kukjin, would you mind taking this patch to your tree ? FWIW Acked-by: Kishon Vijay Abraham I kis...@ti.com Thanks, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
New USB core API to change interval and max packet size
On Tue, Oct 01, 2013 at 10:01:08PM +0300, Xenia Ragiadakou wrote: Hi Sarah, I read the mail on 'possible conflict between xhci_hcd and a patched usbhid'. For reference to others: http://marc.info/?l=linux-usbm=138064948726038w=2 http://marc.info/?l=linux-usbm=138065201426880w=2 I looked in xhci and the problem arises in xhci_queue_intr_tx() when if (xhci_interval != ep_interval) { ... urb-interval = xhci_interval; } right? Yes. The underlying problem is that the xHCI host sets up the endpoint contexts during the Configure Endpoint command, using the interval from the device's endpoint descriptors. It also uses the endpoint descriptor wMaxPacketSize, which can be wrong as well. If the device driver wants to use a different urb-interval than is in the endpoint descriptor, the xHCI driver will simply ignore it. (I'm Ccing the linux-media list, as I've discussed some of these devices with broken descriptors before.) When you say a new API, what do you mean? New functions in usbcore to be used by usb device drivers? Yes. You would export the function in the USB core, and put a prototype in a USB include file (probably in include/linux/usb.h). Let's say that function is called usb_change_ep_bandwidth. Drivers could call into that function when they needed to change either the bInterval or wMaxPacketSize of a particular endpoint. This could be during the driver's probe function, or before switching alternate interface settings, or even after the alt setting is in place, but userspace dictates the driver use a different bandwidth. Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint they need to change, along with the bInterval and wMaxPacketSize values they would like the endpoint to have. Those values could be stored as new values in struct usb_host_endpoint. usb_change_ep_bandwidth would then call into the xHCI driver to drop the endpoint, re-add it, and then issue a bandwidth change. The xHCI driver would have to be changed to look at the new fields in usb_host_endpoint, and set up the endpoint contexts with the interval and packet size from those fields, instead of the endpoint descriptor. We should probably set the new values in usb_host_endpoint to zero after the driver unbinds from the device. Not sure if they should be reset after the driver switches interfaces. I would have to see the use cases in the driver. Here, it is needed to change the endpoint descriptors with the new value in urb so that xhci takes the correct value? I mean the fix should be made in usbcore and xhci shall remain intact? No, we need to fix both the xHCI driver and the USB core. I have time to work on that but i 'm not sure that i understood. Sure. I would actually suggest you first finish up the patch to issue a configure endpoint if userspace wants to clear a halt, but the endpoint isn't actually halted. Did your most current patch work? I can't remember what the status was. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
On 10/01/2013 11:45 PM, Sarah Sharp wrote: On Tue, Oct 01, 2013 at 10:01:08PM +0300, Xenia Ragiadakou wrote: Hi Sarah, I read the mail on 'possible conflict between xhci_hcd and a patched usbhid'. For reference to others: http://marc.info/?l=linux-usbm=138064948726038w=2 http://marc.info/?l=linux-usbm=138065201426880w=2 I looked in xhci and the problem arises in xhci_queue_intr_tx() when if (xhci_interval != ep_interval) { ... urb-interval = xhci_interval; } right? Yes. The underlying problem is that the xHCI host sets up the endpoint contexts during the Configure Endpoint command, using the interval from the device's endpoint descriptors. It also uses the endpoint descriptor wMaxPacketSize, which can be wrong as well. If the device driver wants to use a different urb-interval than is in the endpoint descriptor, the xHCI driver will simply ignore it. (I'm Ccing the linux-media list, as I've discussed some of these devices with broken descriptors before.) When you say a new API, what do you mean? New functions in usbcore to be used by usb device drivers? Yes. You would export the function in the USB core, and put a prototype in a USB include file (probably in include/linux/usb.h). Let's say that function is called usb_change_ep_bandwidth. Drivers could call into that function when they needed to change either the bInterval or wMaxPacketSize of a particular endpoint. This could be during the driver's probe function, or before switching alternate interface settings, or even after the alt setting is in place, but userspace dictates the driver use a different bandwidth. Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint they need to change, along with the bInterval and wMaxPacketSize values they would like the endpoint to have. Those values could be stored as new values in struct usb_host_endpoint. usb_change_ep_bandwidth would then call into the xHCI driver to drop the endpoint, re-add it, and then issue a bandwidth change. The xHCI driver would have to be changed to look at the new fields in usb_host_endpoint, and set up the endpoint contexts with the interval and packet size from those fields, instead of the endpoint descriptor. We should probably set the new values in usb_host_endpoint to zero after the driver unbinds from the device. Not sure if they should be reset after the driver switches interfaces. I would have to see the use cases in the driver. Here, it is needed to change the endpoint descriptors with the new value in urb so that xhci takes the correct value? I mean the fix should be made in usbcore and xhci shall remain intact? No, we need to fix both the xHCI driver and the USB core. I have time to work on that but i 'm not sure that i understood. Sure. I would actually suggest you first finish up the patch to issue a configure endpoint if userspace wants to clear a halt, but the endpoint isn't actually halted. Did your most current patch work? I can't remember what the status was. Sarah Sharp Thanks for the clarification, I understand better now. As far as concerns the reset endpoint fix, I am not sure if it works I have to send an RFC so that it can be further tested but I have a lot of pending RFCs for xhci on the mailing list so i was waiting for those to be reviewed before sending new ones. Or it is not necessary to wait and just send the RFC based on current usb-next tree? regards, ksenia -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Oct 2 04:00:16 CEST 2013 git branch: test git hash: ffee921033e64edf8579a3b21c7f15d1a6c3ef71 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: 0.4.5-rc1 host hardware: x86_64 host os:3.10.1 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.10.1-i686: OK linux-3.1.10-i686: OK linux-3.11.1-i686: OK linux-3.12-rc1-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-2.6.31.14-x86_64: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.10.1-x86_64: OK linux-3.1.10-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12-rc1-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK apps: WARNINGS spec-git: OK sparse version: 0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[no subject]
Good day,I am Mr. Cham Tao Soon, Chairman Audit Committee of UOB Bank, Singapore. I have a project for you in the tons of One Hundred Five Million EUR, after successful transfer, we shall share in the ratio of forty for you and sixty for me. Please reply for specifics.Yours,Mr. Cham Tao -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html