Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: [snip] > > The point when the kernel started complaining about the use of a stack > > based USB I/O buffers is the relevant point, which was not back in > > 2.6.12. I learned of this behavior (that is, receiving warnings about > > the usage) as being new in the 2.6.34 timeframe, the point when a user > > pointed out the complaint message in his kernel log; at that time I had > > not yet tested against that kernel version. > > Older kernels also complain if the stack were actually out of the > DMA range and you try to program DMA there. Yet, only now we've seen > consumer PC's with lots of RAM. One of my test targets is a PC in 32 bit mode with 4GB of RAM; strange then that I've never seen the kernel complain there. The bad buffer has been in the driver for even longer than that and nobody raised the issue to me until about a month ago (the fix was created back then but for other reasons that you already know it didn't become available in -hg until last week). > > > Leave it as is. What's done is done. Your replaced comment is of > > course still correct. I would just appreciate some better sensitivity > > in the future about replacing authors' comments, especially since in > > this case your interpretation of my original comment was wrong. > > Ok. Thanks. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote: > On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > >> Mike Isely wrote: >>> On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: >>> Mike Isely wrote: > Mauro: > > You are reading too much into that comment. > > I never said it was valid to do what had been done, only that for the > longest time this is what the driver did and it never caused a problem > that I was made aware of. What I said there was correct, that this is > what the driver had been doing in the past, that it's definitely causing > a problem now and thus that is why this patch exists. As I said, this is not right: "Apparently later kernels don't like this behavior" >>> Mauro: >>> >>> That statement was in reference to the fact that previously the problem >>> had gone undetected, but now later kernels can notice and complain about >>> this, thus "later kernels don't like this behavior". >>> >>> We can debate that perhaps the statement can be worded better, but that >>> doesn't make it *wrong*. >>> >> Calling 2.6.12 kernel as "later kernels" doesn't seem right to me (that was >> about the kernel were em28xx driver were introduced). > > The point when the em28xx driver appeared has nothing to do with this. > > The point when the kernel started complaining about the use of a stack > based USB I/O buffers is the relevant point, which was not back in > 2.6.12. I learned of this behavior (that is, receiving warnings about > the usage) as being new in the 2.6.34 timeframe, the point when a user > pointed out the complaint message in his kernel log; at that time I had > not yet tested against that kernel version. Older kernels also complain if the stack were actually out of the DMA range and you try to program DMA there. Yet, only now we've seen consumer PC's with lots of RAM. > Leave it as is. What's done is done. Your replaced comment is of > course still correct. I would just appreciate some better sensitivity > in the future about replacing authors' comments, especially since in > this case your interpretation of my original comment was wrong. Ok. -- Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: > > On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > > > >> Mike Isely wrote: > >>> Mauro: > >>> > >>> You are reading too much into that comment. > >>> > >>> I never said it was valid to do what had been done, only that for the > >>> longest time this is what the driver did and it never caused a problem > >>> that I was made aware of. What I said there was correct, that this is > >>> what the driver had been doing in the past, that it's definitely causing > >>> a problem now and thus that is why this patch exists. > >> As I said, this is not right: > >>"Apparently later kernels don't like this behavior" > > > > Mauro: > > > > That statement was in reference to the fact that previously the problem > > had gone undetected, but now later kernels can notice and complain about > > this, thus "later kernels don't like this behavior". > > > > We can debate that perhaps the statement can be worded better, but that > > doesn't make it *wrong*. > > > > Calling 2.6.12 kernel as "later kernels" doesn't seem right to me (that was > about the kernel were em28xx driver were introduced). The point when the em28xx driver appeared has nothing to do with this. The point when the kernel started complaining about the use of a stack based USB I/O buffers is the relevant point, which was not back in 2.6.12. I learned of this behavior (that is, receiving warnings about the usage) as being new in the 2.6.34 timeframe, the point when a user pointed out the complaint message in his kernel log; at that time I had not yet tested against that kernel version. > > > > >> It is not "later kernels". DMA over stack were never supported. Your driver > >> had a bug that you didn't noticed for long time, probably because nobody > >> reported you this issue, since it appears only on some non-Intel archs and > >> on i386 with more than 3.12 Gb of RAM, and when the stack happens to be > >> after > >> the first 3.12 Gb (with is a somewhat rare condition). > > > > I understand your point perfectly that this was never right or valid. > > In fact, I also understood that point long before you decided to explain > > it to me here - after all my realization of this problem in the driver > > is why I wrote the patch in the first place. Absolutely no argument > > there about the importance of the change. > > > > None of that however justifies putting words into an author's mouth, > > which is effectively what you did by replacing that commit comment. > > > > First of all, it is clearly stated at the patch that the description > were changed and by whom: > [mche...@redhat.com: fix patch description] > > Second, it is an usual upstream practice to fix descriptions where needed. > The patch description is a precious resource for people that are seeking > for similar bugs, and for those that are trying to understand some code. > So, it is important to not send broken/incomplete comments to kernel, > or comments that may have a dubious interpretation. So, subsystem maintainers > frequently need to fix comments. > > Anyway, to end this discussion, I can simply revert the patch from the > staging tree, waiting for a new patch from you with a fixed comment. > > Also, if you prefer, next time, I won't apply any patch from you if I > found a bad comment without your ack, even if it means that you'll > probably loose a merge window. Leave it as is. What's done is done. Your replaced comment is of course still correct. I would just appreciate some better sensitivity in the future about replacing authors' comments, especially since in this case your interpretation of my original comment was wrong. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote: > On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > >> Mike Isely wrote: >>> Mauro: >>> >>> You are reading too much into that comment. >>> >>> I never said it was valid to do what had been done, only that for the >>> longest time this is what the driver did and it never caused a problem >>> that I was made aware of. What I said there was correct, that this is >>> what the driver had been doing in the past, that it's definitely causing >>> a problem now and thus that is why this patch exists. >> As I said, this is not right: >> "Apparently later kernels don't like this behavior" > > Mauro: > > That statement was in reference to the fact that previously the problem > had gone undetected, but now later kernels can notice and complain about > this, thus "later kernels don't like this behavior". > > We can debate that perhaps the statement can be worded better, but that > doesn't make it *wrong*. > Calling 2.6.12 kernel as "later kernels" doesn't seem right to me (that was about the kernel were em28xx driver were introduced). > >> It is not "later kernels". DMA over stack were never supported. Your driver >> had a bug that you didn't noticed for long time, probably because nobody >> reported you this issue, since it appears only on some non-Intel archs and >> on i386 with more than 3.12 Gb of RAM, and when the stack happens to be after >> the first 3.12 Gb (with is a somewhat rare condition). > > I understand your point perfectly that this was never right or valid. > In fact, I also understood that point long before you decided to explain > it to me here - after all my realization of this problem in the driver > is why I wrote the patch in the first place. Absolutely no argument > there about the importance of the change. > > None of that however justifies putting words into an author's mouth, > which is effectively what you did by replacing that commit comment. > First of all, it is clearly stated at the patch that the description were changed and by whom: [mche...@redhat.com: fix patch description] Second, it is an usual upstream practice to fix descriptions where needed. The patch description is a precious resource for people that are seeking for similar bugs, and for those that are trying to understand some code. So, it is important to not send broken/incomplete comments to kernel, or comments that may have a dubious interpretation. So, subsystem maintainers frequently need to fix comments. Anyway, to end this discussion, I can simply revert the patch from the staging tree, waiting for a new patch from you with a fixed comment. Also, if you prefer, next time, I won't apply any patch from you if I found a bad comment without your ack, even if it means that you'll probably loose a merge window. -- Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: > > Mauro: > > > > You are reading too much into that comment. > > > > I never said it was valid to do what had been done, only that for the > > longest time this is what the driver did and it never caused a problem > > that I was made aware of. What I said there was correct, that this is > > what the driver had been doing in the past, that it's definitely causing > > a problem now and thus that is why this patch exists. > > As I said, this is not right: > "Apparently later kernels don't like this behavior" Mauro: That statement was in reference to the fact that previously the problem had gone undetected, but now later kernels can notice and complain about this, thus "later kernels don't like this behavior". We can debate that perhaps the statement can be worded better, but that doesn't make it *wrong*. > > It is not "later kernels". DMA over stack were never supported. Your driver > had a bug that you didn't noticed for long time, probably because nobody > reported you this issue, since it appears only on some non-Intel archs and > on i386 with more than 3.12 Gb of RAM, and when the stack happens to be after > the first 3.12 Gb (with is a somewhat rare condition). I understand your point perfectly that this was never right or valid. In fact, I also understood that point long before you decided to explain it to me here - after all my realization of this problem in the driver is why I wrote the patch in the first place. Absolutely no argument there about the importance of the change. None of that however justifies putting words into an author's mouth, which is effectively what you did by replacing that commit comment. When I've propagated commits from other authors who have sent me patches, I try very hard to ensure that their comments are preserved verbatim. Those are after all the author's words not mine. If I think such a comment has a problem, then I will discuss it with the author and let him/her provide the replacement statement (or at least explain to me why (s)he thinks it is correct). Failing that then I will at least make it clear in the comment which edits came from me. I would never want to be accused of unilaterally putting words into peoples' mouths and I would hope others will treat me the same way. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote:
> Mauro:
>
> You are reading too much into that comment.
>
> I never said it was valid to do what had been done, only that for the
> longest time this is what the driver did and it never caused a problem
> that I was made aware of. What I said there was correct, that this is
> what the driver had been doing in the past, that it's definitely causing
> a problem now and thus that is why this patch exists.
As I said, this is not right:
"Apparently later kernels don't like this behavior"
It is not "later kernels". DMA over stack were never supported. Your driver
had a bug that you didn't noticed for long time, probably because nobody
reported you this issue, since it appears only on some non-Intel archs and
on i386 with more than 3.12 Gb of RAM, and when the stack happens to be after
the first 3.12 Gb (with is a somewhat rare condition).
If you take a look at the old logs, you'll see, at commit
a6c2ba283565dbc9f055dcb2ecba1971460bb535 (nov, 2005) the fix on one of the
drivers:
+int em2820_write_regs_req(struct em2820 *dev, u8 req, u16 reg, char *buf,
+ int len)
+{
+ int ret;
+
+ /*usb_control_msg seems to expect a kmalloced buffer */
+ unsigned char *bufs = kmalloc(len, GFP_KERNEL);
The same bug hit me in 2006/2007 when writing tm6000 driver. Due to a problem
at the videobuf free logic, on that time, it were very frequent to hit this
bug, as, with memory being spent, eventually stack address would be above the
3Gb address.
Cheers,
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mauro: You are reading too much into that comment. I never said it was valid to do what had been done, only that for the longest time this is what the driver did and it never caused a problem that I was made aware of. What I said there was correct, that this is what the driver had been doing in the past, that it's definitely causing a problem now and thus that is why this patch exists. I'd really rather you not mess with my comment. Probably too late however. -Mike On Fri, 21 May 2010, Mauro Carvalho Chehab wrote: > Mike Isely wrote: > > Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the > > following pvrusb2 driver fixes / improvements: > > > > - pvrusb2: Minor debug code fixup > > - pvrusb2: Fix Gotview hardware support > > - pvrusb2: Avoid using stack allocated buffers when performing USB I/O > > Your comment for this patch is wrong: > > pvrusb2: The pvrusb2 driver has for the longest time used a (tiny) > stack allocated buffer for some of its I/O with the hardware. > Apparently later kernels don't like this behavior and trap it at > run-time, causing nasty complaints to the kernel log. This trivial > change fixes the one case in the driver where this had been happening. > > It were never valid to use stack for DMA, as kernel provides > no warranty that the stack would be on a page that can do DMA. > In a matter of fact, as most x86 USB drivers accept DMA at the first > 3Gb of RAM space, this bug is generally not noticed on i386/x86_64 > archs. Yet, if your machine has more than 3Gb, there are some chances that > the stack would be at the HIGHMEM area, where DMA is not supported by the > processor. > > As this is a common error, newer kernels have some instrumentation support to > warn about such troubles. > > I'll be fixing the comment. > > > -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Mike Isely wrote: > Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the > following pvrusb2 driver fixes / improvements: > > - pvrusb2: Minor debug code fixup > - pvrusb2: Fix Gotview hardware support > - pvrusb2: Avoid using stack allocated buffers when performing USB I/O Your comment for this patch is wrong: pvrusb2: The pvrusb2 driver has for the longest time used a (tiny) stack allocated buffer for some of its I/O with the hardware. Apparently later kernels don't like this behavior and trap it at run-time, causing nasty complaints to the kernel log. This trivial change fixes the one case in the driver where this had been happening. It were never valid to use stack for DMA, as kernel provides no warranty that the stack would be on a page that can do DMA. In a matter of fact, as most x86 USB drivers accept DMA at the first 3Gb of RAM space, this bug is generally not noticed on i386/x86_64 archs. Yet, if your machine has more than 3Gb, there are some chances that the stack would be at the HIGHMEM area, where DMA is not supported by the processor. As this is a common error, newer kernels have some instrumentation support to warn about such troubles. I'll be fixing the comment. -- Cheers, 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the following pvrusb2 driver fixes / improvements: - pvrusb2: Minor debug code fixup - pvrusb2: Fix Gotview hardware support - pvrusb2: Avoid using stack allocated buffers when performing USB I/O - pvrusb2: New feature to mark specific hardware support as experimental - pvrusb2: Fix kernel oops at device unregistration - pvrusb2: Fix missing header include - pvrusb2: Fix USB parent device reference count - pvrusb2: Fix minor internal array allocation - pvrusb2: Fix kernel oops on device tear-down - pvrusb2: Call sysfs_attr_init() appropriately... pvrusb2-devattr.c |1 pvrusb2-devattr.h |5 pvrusb2-hdw.c | 26 + pvrusb2-main.c|4 +-- pvrusb2-sysfs.c | 64 +++--- pvrusb2-v4l2.c| 16 ++--- 6 files changed, 107 insertions(+), 9 deletions(-) These are primarily a collection of stability fixes. Thanks, -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-patches
Please from http://linuxtv.org/hg/~mcisely/pvrusb2-patches for the following pvrusb2 driver fixes / improvements: - pvrusb2: Enforce a 300msec stabilization interval during stream strart - pvrusb2: Reduce encoder quiet period - pvrusb2: Adjust 300msec digitizer wait to be more selective pvrusb2-hdw-internal.h | 12 - pvrusb2-hdw.c | 61 - pvrusb2-hdw.h |1 3 files changed, 61 insertions(+), 13 deletions(-) These fixes improve mpeg streaming startup stability for any pvrusb2-driven device which contains an saa7115 video digitizer. Thanks, -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-20091124
Mauro: Please from http://linuxtv.org/hg/~mcisely/pvrusb2-20091124 for the following pvrusb2 driver fixes / improvements: - pvrusb2: Support 16KB FX2 firmware - pvrusb2: Support manual extraction of 16KB FX2 firmware - pvrusb2: Shorten device hardware description text to work around V4L shortcoming - pvrusb2: Bind I2C address 0x71 for Zilog IR devices - pvrusb2: Cosmetic tweak to minimize size_t exposure - pvrusb2: Fix lingering 16KB FX2 Firmware issues pvrusb2-debugifc.c | 14 +- pvrusb2-devattr.c | 13 - pvrusb2-devattr.h |1 + pvrusb2-hdw.c | 44 +--- pvrusb2-hdw.h |2 +- pvrusb2-i2c-core.c |1 + 6 files changed, 49 insertions(+), 26 deletions(-) The 16KB fixes are pretty important - Hauppauge has updated the FX2 firmware for HVR-1950 and HVR-1900 devices such that it is 16KB in size now. Unfortunately the driver for years had been enforcing the size to be 8KB which made it unable to load the newer firmware. This causes a problem for new users of the driver since the driver CD from Hauppauge contains this newer firmware. With the 16KB fixes this problem is solved. They are marked high priority. Thanks, -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-20091011
On Thu, 29 Oct 2009, Mauro Carvalho Chehab wrote: > Em Sun, 11 Oct 2009 22:53:14 -0500 (CDT) > Mike Isely escreveu: > > > > > Mauro: > > > > Please from http://linuxtv.org/hg/~mcisely/pvrusb2-20091011 for a few > > various pvrusb2 fixes / improvements. No critical bug fixes here, just > > a bunch of little things: > > > > - pvrusb2: Make more info available to udev > > - pvrusb2: Soften encoder warning message > > - pvrusb2: Improve diagnostic info on driver initialization failure > > - pvrusb2: Report hardware description to kernel log upon initialization > > - pvrusb2: Add hardware description to debuginfo output > > - pvrusb2: Fix redundant message on driver initialization failure (missing > > break) > > - pvrusb2: Cosmetic kernel log tweak > > Committed. > > Please fix the existing CodingStyle erros added on this changeset on a next > pull request: We've had this discussion many times in the past. I am not going to have that debate yet again. These will NOT be changed. -Mike > > ERROR: trailing statements should be on next line > #26: FILE: linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c:919: > + if (!dip) return; > + if (!dip) return; > ERROR: trailing statements should be on next line > #27: FILE: linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c:920: > + if (!dip->devbase.parent) return; > + if (!dip->devbase.parent) return; > > > > Cheers, > Mauro > -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-20091011
Em Sun, 11 Oct 2009 22:53:14 -0500 (CDT) Mike Isely escreveu: > > Mauro: > > Please from http://linuxtv.org/hg/~mcisely/pvrusb2-20091011 for a few > various pvrusb2 fixes / improvements. No critical bug fixes here, just > a bunch of little things: > > - pvrusb2: Make more info available to udev > - pvrusb2: Soften encoder warning message > - pvrusb2: Improve diagnostic info on driver initialization failure > - pvrusb2: Report hardware description to kernel log upon initialization > - pvrusb2: Add hardware description to debuginfo output > - pvrusb2: Fix redundant message on driver initialization failure (missing > break) > - pvrusb2: Cosmetic kernel log tweak Committed. Please fix the existing CodingStyle erros added on this changeset on a next pull request: ERROR: trailing statements should be on next line #26: FILE: linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c:919: + if (!dip) return; + if (!dip) return; ERROR: trailing statements should be on next line #27: FILE: linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c:920: + if (!dip->devbase.parent) return; + if (!dip->devbase.parent) return; Cheers, 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-20091011
Mauro: Please from http://linuxtv.org/hg/~mcisely/pvrusb2-20091011 for a few various pvrusb2 fixes / improvements. No critical bug fixes here, just a bunch of little things: - pvrusb2: Make more info available to udev - pvrusb2: Soften encoder warning message - pvrusb2: Improve diagnostic info on driver initialization failure - pvrusb2: Report hardware description to kernel log upon initialization - pvrusb2: Add hardware description to debuginfo output - pvrusb2: Fix redundant message on driver initialization failure (missing break) - pvrusb2: Cosmetic kernel log tweak pvrusb2-debugifc.c |3 +++ pvrusb2-encoder.c |5 - pvrusb2-hdw-internal.h |1 + pvrusb2-hdw.c | 33 - pvrusb2-v4l2.c | 21 - 5 files changed, 52 insertions(+), 11 deletions(-) -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev
[sorry, reposted with corrected subject tag] Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for a number of pvrusb2 driver fixes. The first two in particular are high-priority critical fixes that clean up a nasty regression involving analog capture on the HVR-1950 and the older 24xxx model series. It would be good to see those at least backported to a 2.6.30.x release. - pvrusb2: Fix hardware scaling when used with cx25840 - pvrusb2: Re-fix hardware scaling on video standard change - pvrusb2: Change initial default frequency setting - pvrusb2: Improve handling of routing schemes - pvrusb2: De-obfuscate code which handles routing schemes pvrusb2-audio.c | 14 ++- pvrusb2-cs53l32a.c| 26 +++-- pvrusb2-cx2584x-v4l.c | 39 +--- pvrusb2-hdw.c | 60 -- pvrusb2-video-v4l.c | 37 +- 5 files changed, 98 insertions(+), 78 deletions(-) -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev
Em Tue, 12 May 2009 01:36:17 -0500 (CDT) Mike Isely escreveu: > On Mon, 11 May 2009, Mauro Carvalho Chehab wrote: > > > Em Mon, 11 May 2009 22:09:26 -0300 > > Mauro Carvalho Chehab escreveu: > > > > > Em Sat, 9 May 2009 16:49:31 -0500 (CDT) > > > Mike Isely escreveu: > > > > > > > > > > > Mauro: > > > > > > > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for various > > > > pvrusb2 changesets. Several have to do with IR as previously discussed > > > > with Jean Delvare. He's waiting for these changes. Other stuff is > > > > more > > > > of a miscellaneous / cleanup nature. > > > > Hmm... this one failed when importing on -git: > > > > Changeset: 11749 > > From: Greg Kroah-Hartman > > Commiter: Mike Isely > > Date: Fri May 01 22:36:33 2009 -0500 > > Subject: pvrusb2: remove driver_data direct access of struct device > > > > In the near future, the driver core is going to not allow direct access > > to the driver_data pointer in struct device. Instead, the functions > > dev_get_drvdata() and dev_set_drvdata() should be used. These functions > > have been around since the beginning, so are backwards compatible with > > all older kernel versions. > > > > Priority: normal > > > > Cc: Mauro Carvalho Chehab > > Cc: linux-media@vger.kernel.org > > > > $ patch -p1 -i 11749.patch > > patching file drivers/media/video/pvrusb2/pvrusb2-sysfs.c > > Reversed (or previously applied) patch detected! Assume -R? [n] > > > > It seems that you've got a Greg's patch, removed the parts that touch on > > other > > files, applied on your tree and asked me to merge it. Please, never do it, > > since this will cause merge problems when exporting the patches to git. Next > > time, just reply with an acked-by, and let Patchwork to add your ack on the > > original patch. > > > > I in fact did what you are asking for here (i.e. wait for it to show up > on its own) before for another change that got rid of usb_settoggle(). > It took a LONG time - WEEKS - for that fix to get back into v4l-dvb via > the mechanism you just described. And this had the effect of breaking > the v4l-dvb repository for a period of time when the kernel mainline > then unpublished the usb_settoggle() function. I do NOT like to see > that happen. That caused me to decide not to rely on what you are now > telling me to do. > > I also disagree with this for another reason. What happens if, say, > Greg generates a patch that I need before I can proceed with other > changes? Do I just sit around and wait for it to trickle back before I > can continue? That seems wrong. In addition in the past when there > have been pvrusb2 changes generated from upstream you have asked if I > was planning on pulling them in myself - which I've done in the past. > > It seems wrong on its face to tell me that I can't go get a patch that > affects my driver. > > AND... In the case of the "remove usb_settoggle()" patch, I *DID* in > fact add my acked-by to that patch. Greg dutifully took note of this > and ensured it was part of the git patch. However when it got back into > v4l-dvb, the acked-by attribution was missing. I complained about this > to you and your response was that this was a fault of the pathway / > mechanism and that I should basically accept this. So now you're > telling me to do this anyway? Mike, As I've explained you in priv, and already talked several times at the ML, our entire mechanism of using -hg is broken, for several reasons. We should really move to -git. My intention were to start discussing it just after the end of the last merging cycle, but, unfortunately, I haven't enough time to finish some scripts to allow people to use a git sub-tree. What are the main problems with -hg and with our current merging proccess? 1) The way we work don't allow us to have the full Signed-off-by/Acked-by historic on -hg. For example, all patches submitted upstream should have the maintainers Signed-off-by (SOB). However, by doing hg pull, I can't add my SOB. If, on the other hand, I just cherry pick all patches from your tree and add my SOB, you'll need to drop your tree and clone again from mine, since the patches they will be understood by hg as a different patch. This means also that, if you have a second tree that has your patches, plus some newer patches, that you'll have to cherry-pick the newer patches on that tree, drop it and re-create [1]; 2) We have several extra efforts when picking a patch that are upstream and merge it back on our tree. This requires me to do a diff between our tree (with all backport symbols stripped) and -git, and manually seeking for each diff line on -git, in order to identify what -git patch added such line. Then, for each -git patch, I need to cherry-pick the patch and apply on our tree. On most cases, the patch doesn't apply cleanly, so I need to manually apply it. Also, in general, those patches are generally KABI changes, so they touch not only on
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev
On Mon, 11 May 2009, Mauro Carvalho Chehab wrote: > Em Mon, 11 May 2009 22:09:26 -0300 > Mauro Carvalho Chehab escreveu: > > > Em Sat, 9 May 2009 16:49:31 -0500 (CDT) > > Mike Isely escreveu: > > > > > > > > Mauro: > > > > > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for various > > > pvrusb2 changesets. Several have to do with IR as previously discussed > > > with Jean Delvare. He's waiting for these changes. Other stuff is more > > > of a miscellaneous / cleanup nature. > > Hmm... this one failed when importing on -git: > > Changeset: 11749 > From: Greg Kroah-Hartman > Commiter: Mike Isely > Date: Fri May 01 22:36:33 2009 -0500 > Subject: pvrusb2: remove driver_data direct access of struct device > > In the near future, the driver core is going to not allow direct access > to the driver_data pointer in struct device. Instead, the functions > dev_get_drvdata() and dev_set_drvdata() should be used. These functions > have been around since the beginning, so are backwards compatible with > all older kernel versions. > > Priority: normal > > Cc: Mauro Carvalho Chehab > Cc: linux-media@vger.kernel.org > > $ patch -p1 -i 11749.patch > patching file drivers/media/video/pvrusb2/pvrusb2-sysfs.c > Reversed (or previously applied) patch detected! Assume -R? [n] > > It seems that you've got a Greg's patch, removed the parts that touch on other > files, applied on your tree and asked me to merge it. Please, never do it, > since this will cause merge problems when exporting the patches to git. Next > time, just reply with an acked-by, and let Patchwork to add your ack on the > original patch. > I in fact did what you are asking for here (i.e. wait for it to show up on its own) before for another change that got rid of usb_settoggle(). It took a LONG time - WEEKS - for that fix to get back into v4l-dvb via the mechanism you just described. And this had the effect of breaking the v4l-dvb repository for a period of time when the kernel mainline then unpublished the usb_settoggle() function. I do NOT like to see that happen. That caused me to decide not to rely on what you are now telling me to do. I also disagree with this for another reason. What happens if, say, Greg generates a patch that I need before I can proceed with other changes? Do I just sit around and wait for it to trickle back before I can continue? That seems wrong. In addition in the past when there have been pvrusb2 changes generated from upstream you have asked if I was planning on pulling them in myself - which I've done in the past. It seems wrong on its face to tell me that I can't go get a patch that affects my driver. AND... In the case of the "remove usb_settoggle()" patch, I *DID* in fact add my acked-by to that patch. Greg dutifully took note of this and ensured it was part of the git patch. However when it got back into v4l-dvb, the acked-by attribution was missing. I complained about this to you and your response was that this was a fault of the pathway / mechanism and that I should basically accept this. So now you're telling me to do this anyway? Whatever. -Mike -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev
Em Mon, 11 May 2009 22:09:26 -0300 Mauro Carvalho Chehab escreveu: > Em Sat, 9 May 2009 16:49:31 -0500 (CDT) > Mike Isely escreveu: > > > > > Mauro: > > > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for various > > pvrusb2 changesets. Several have to do with IR as previously discussed > > with Jean Delvare. He's waiting for these changes. Other stuff is more > > of a miscellaneous / cleanup nature. Hmm... this one failed when importing on -git: Changeset: 11749 From: Greg Kroah-Hartman Commiter: Mike Isely Date: Fri May 01 22:36:33 2009 -0500 Subject: pvrusb2: remove driver_data direct access of struct device In the near future, the driver core is going to not allow direct access to the driver_data pointer in struct device. Instead, the functions dev_get_drvdata() and dev_set_drvdata() should be used. These functions have been around since the beginning, so are backwards compatible with all older kernel versions. Priority: normal Cc: Mauro Carvalho Chehab Cc: linux-media@vger.kernel.org $ patch -p1 -i 11749.patch patching file drivers/media/video/pvrusb2/pvrusb2-sysfs.c Reversed (or previously applied) patch detected! Assume -R? [n] It seems that you've got a Greg's patch, removed the parts that touch on other files, applied on your tree and asked me to merge it. Please, never do it, since this will cause merge problems when exporting the patches to git. Next time, just reply with an acked-by, and let Patchwork to add your ack on the original patch. Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev
Em Sat, 9 May 2009 16:49:31 -0500 (CDT) Mike Isely escreveu: > > Mauro: > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for various > pvrusb2 changesets. Several have to do with IR as previously discussed > with Jean Delvare. He's waiting for these changes. Other stuff is more > of a miscellaneous / cleanup nature. Applied, thanks. Cheers, 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-dev
Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-dev for various pvrusb2 changesets. Several have to do with IR as previously discussed with Jean Delvare. He's waiting for these changes. Other stuff is more of a miscellaneous / cleanup nature. -Mike - pvrusb2: Select, track, and report IR scheme in use with the device - pvrusb2: Update to work with upcoming ir_video changes in v4l-dvb core - pvrusb2: Set ir_video autoloading to default disabled - pvrusb2: Bump up version advertised through v4l interface - pvrusb2: Don't use the internal i2c client list - pvrusb2: remove driver_data direct access of struct device - pvrusb2: Allocate a routing ID for future support of Terratec Grabster AV400 pvrusb2-devattr.c |1 pvrusb2-devattr.h | 23 +- pvrusb2-hdw-internal.h |3 + pvrusb2-hdw.c | 78 - pvrusb2-i2c-core.c | 53 +++-- pvrusb2-sysfs.c| 22 ++--- pvrusb2-v4l2.c |2 - 7 files changed, 106 insertions(+), 76 deletions(-) -- Mike Isely isely @ isely (dot) net PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Mon, 6 Apr 2009, Jean Delvare wrote: > Hi Mike, > > Please note: I think the long post I just sent makes part of this > discussion obsolete from a technical perspective. But still interesting > from a functional perspective, which is why I am following up. I plan a reply to your RFC as well, probably later on tonight. > > On Mon, 6 Apr 2009 10:03:00 -0500 (CDT), Mike Isely wrote: > > On Mon, 6 Apr 2009, Jean Delvare wrote: > > > Again, ir-kbd-i2c does _not_ auto-load. What my code (and now yours) > > > does is instantiating an i2c device named "ir-kbd". _If_ the ir-kbd-i2c > > > driver is later loaded, it will bind to that device. We might let udev > > > load the ir-kbd-i2c driver at some point in the future, but clearly we > > > need to sort out the lirc case beforehand, otherwise some users will > > > hit the problems you have anticipated, and we don't want this to happen. > > > > > > So, your module parameter is improperly named. Setting it to 1 does > > > prevent the pvrusb2 driver from instantiating the "ir-kbd" device, and > > > that's all. > > > > The module parameter is named "disabled_autoload_ir_kbd", how is that > > wrong? The name is based on the internal receiver name "ir-kbd". > > There's no "[-_]i2c" in the name. The parameter's description also does > > not reference ir-kbd-i2c by name either. I did it that way specifically > > for the reason you point out here. > > I was perfectly fine with the "ir_kbd" part. The part I complain about > is "autoload", because the original mechanism doesn't involve any > autoloading. But from the viewpoint of a user of the pvrusb2 driver, that is in fact what will appear to happen. 1. User plugs in device. 2. Driver (pvrusb2) instantiates self. 3. Driver automatically attempts to load and bind ir-kbd-i2c to the IR receiver, hence "autoload". Yes I know ir-kbd-i2c no longer autoloads itself; that is a major goal of the conversion. But with the pvrusb2 change to explicitly bind it, the behavior from the view of the user is still that of an autoload operation. I think we're just arguing semantics, but it's why I put "autoload" in the name. However given the realization below about multiple devices, I think I need to step back and look at this from a larger scope (e.g. make it an array, merge with the fairly clunky ir_mode switch already in the driver and clean that up, etc). [...] > > This morning I actually realized another use-case that has been missed. > > It was likely an issue before anyway, but it got me thinking about this: > > If a user has multiple devices attached to one system, he probably won't > > want all of the corresponding IR receivers enabled - that would just > > trigger redundant input events. With a PCI-hosted ivtv device this is > > not really an issue - because there one can just decline to plug in the > > actual IR sensor. But with USB-hosted devices, the IR sensor is an > > integral part of the device and can't be unplugged. OK, well such a > > user could instead just choose to put a piece of tape over the window or > > stick all but one device in a box (and causing potential heat problems > > if it isn't ventilated), but that approach is well, inelegant. I think > > this argues for a better solution in the bridge driver to selectively > > disable IR on a per-device instance basis. There's already some logic > > in the pvrusb2 driver to do this, but it's clumsy and wasn't intended to > > solve that particular use-case. I need to consider this further and do > > some cleanup. This use-case may actually suggest the disable option > > should be expanded and possibly made permanent. > > I agree. I presume that this is one of the reasons why some bridge > drivers have a disable_ir parameter (the other reason being lirc). It > would probably make sense to not only keep these module parameters even > after lirc is merged into the kernel tree, but turn them into arrays, > so that IR receivers can be disabled selectively. This would address > the problem you just raised. > > But all this can be done after the conversion work it finished. I believe I can solve this for the pvrusb2 driver without entanglement with the conversion work underway. Pieces of the solution are already in the driver; I just need to clean this up. In any case, I'm not going to worry about it immediately. I've been neglecting some non-Linux tasks, like filing bills and finishing my tax return :-) -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Hi Mike, Please note: I think the long post I just sent makes part of this discussion obsolete from a technical perspective. But still interesting from a functional perspective, which is why I am following up. On Mon, 6 Apr 2009 10:03:00 -0500 (CDT), Mike Isely wrote: > On Mon, 6 Apr 2009, Jean Delvare wrote: > > Again, ir-kbd-i2c does _not_ auto-load. What my code (and now yours) > > does is instantiating an i2c device named "ir-kbd". _If_ the ir-kbd-i2c > > driver is later loaded, it will bind to that device. We might let udev > > load the ir-kbd-i2c driver at some point in the future, but clearly we > > need to sort out the lirc case beforehand, otherwise some users will > > hit the problems you have anticipated, and we don't want this to happen. > > > > So, your module parameter is improperly named. Setting it to 1 does > > prevent the pvrusb2 driver from instantiating the "ir-kbd" device, and > > that's all. > > The module parameter is named "disabled_autoload_ir_kbd", how is that > wrong? The name is based on the internal receiver name "ir-kbd". > There's no "[-_]i2c" in the name. The parameter's description also does > not reference ir-kbd-i2c by name either. I did it that way specifically > for the reason you point out here. I was perfectly fine with the "ir_kbd" part. The part I complain about is "autoload", because the original mechanism doesn't involve any autoloading. > > (...) > > For completeness, your second patch actually breaks the ir-kbd-i2c use > > case. Your code will instantiate a new-style "ir-kbd" device which the > > legacy ir-kbd-i2c can't use. As the instantiated device makes the > > address busy, the probing logic of legacy ir-kbd-i2c driver will skip > > it. Without my changes, all users will have to pass > > disable_autoload_ir_kbd=1, whether they want to use lirc or ir-kbd-i2c, > > otherwise they lose IR support. > > Well yes. I was saying "no harm" in the sense that everything that was > possible before is still possible now, though perhaps with the module > option set. We agree then. > (...) > This morning I actually realized another use-case that has been missed. > It was likely an issue before anyway, but it got me thinking about this: > If a user has multiple devices attached to one system, he probably won't > want all of the corresponding IR receivers enabled - that would just > trigger redundant input events. With a PCI-hosted ivtv device this is > not really an issue - because there one can just decline to plug in the > actual IR sensor. But with USB-hosted devices, the IR sensor is an > integral part of the device and can't be unplugged. OK, well such a > user could instead just choose to put a piece of tape over the window or > stick all but one device in a box (and causing potential heat problems > if it isn't ventilated), but that approach is well, inelegant. I think > this argues for a better solution in the bridge driver to selectively > disable IR on a per-device instance basis. There's already some logic > in the pvrusb2 driver to do this, but it's clumsy and wasn't intended to > solve that particular use-case. I need to consider this further and do > some cleanup. This use-case may actually suggest the disable option > should be expanded and possibly made permanent. I agree. I presume that this is one of the reasons why some bridge drivers have a disable_ir parameter (the other reason being lirc). It would probably make sense to not only keep these module parameters even after lirc is merged into the kernel tree, but turn them into arrays, so that IR receivers can be disabled selectively. This would address the problem you just raised. But all this can be done after the conversion work it finished. -- Jean Delvare -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Mon, 6 Apr 2009, Jean Delvare wrote: > Hi Mike, > > I'll answer all your questions and express my concerns in this reply, to > avoid spreading the info all around the discussion thread. > > On Mon, 6 Apr 2009 00:19:23 -0500 (CDT), Mike Isely wrote: > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for two pvrusb2 > > changesets. One sets up the ability to track what kind of IR receiver > > is expected, > > Looks very good. The more we know about each board, the less probing we > have to do, the better. > > > the other optionally autoloads ir-kbd-i2c based on that > > result and a module option that can be used to disable that autoloading. > > Again, ir-kbd-i2c does _not_ auto-load. What my code (and now yours) > does is instantiating an i2c device named "ir-kbd". _If_ the ir-kbd-i2c > driver is later loaded, it will bind to that device. We might let udev > load the ir-kbd-i2c driver at some point in the future, but clearly we > need to sort out the lirc case beforehand, otherwise some users will > hit the problems you have anticipated, and we don't want this to happen. > > So, your module parameter is improperly named. Setting it to 1 does > prevent the pvrusb2 driver from instantiating the "ir-kbd" device, and > that's all. The module parameter is named "disabled_autoload_ir_kbd", how is that wrong? The name is based on the internal receiver name "ir-kbd". There's no "[-_]i2c" in the name. The parameter's description also does not reference ir-kbd-i2c by name either. I did it that way specifically for the reason you point out here. I was originally going to use the name that Hans had suggested but then decided otherwise because (1) I decided to follow your desire and make it a disable option, (2) Hans' suggested name did in fact encode the name ir-kbd-i2c in the module option. > > Speaking of this module parameter, I was a little worried at first that > you wanted an inverted logic for it in pvrusb2 compared to what some > other bridge drivers (saa7134, em28xx, cx231xx), but thinking a bit > more about I came to the conclusion that it was OK because it matched > the history of the pvrusb2 driver. Now I see that you followed their > logic (enabled is the default) but you use a different module parameter > name (disable_autoload_ir_kbd vs. disable_ir). I think there would be > some value in sticking to a common name in all bridge drivers (like we > have the standard video_nr module parameter.) > > That being said, I will not insist on this. My feeling is that this is > all temporary anyway, because the removal of the legacy i2c model will > break lirc and the main point is to decide how we will fix it. I'll > post a separate summary with proposals. Depending on what we do, the > module parameter you added is likely to become obsolete. I did want it to be an enable parameter, in order to match previous driver behavior. But whether it is an enable or a disable option, in one use-case somebody has to set it. So I relented and made it a disable option. > > > This is the result of what I had posted about an hour ago. It looks > > like a lot of lines, but it was all basically trivial stuff. > > > > Note that these changes are safe; nothing is regressed here and this > > does not depend on anyone else's changes. Even though that second > > changeset won't really do much until Jean's ir-kbd-i2c stuff is merged, > > the fact is that it won't cause any harm either. Since the pvrusb2 > > driver wasn't previously autoloading ir-kbd-i2c anyway, this change > > therefore breaks nothing. > > For completeness, your second patch actually breaks the ir-kbd-i2c use > case. Your code will instantiate a new-style "ir-kbd" device which the > legacy ir-kbd-i2c can't use. As the instantiated device makes the > address busy, the probing logic of legacy ir-kbd-i2c driver will skip > it. Without my changes, all users will have to pass > disable_autoload_ir_kbd=1, whether they want to use lirc or ir-kbd-i2c, > otherwise they lose IR support. Well yes. I was saying "no harm" in the sense that everything that was possible before is still possible now, though perhaps with the module option set. > > But honestly that doesn't matter much, I think, because the idea is to > merge my changes quickly. Yes, exactly. [...] This morning I actually realized another use-case that has been missed. It was likely an issue before anyway, but it got me thinking about this: If a user has multiple devices attached to one system, he probably won't want all of the corresponding IR receivers enabled - that would just trigger redundant input events. With a PCI-hosted ivtv device this is not really an issue - because there one can just decline to plug in the actual IR sensor. But with USB-hosted devices, the IR sensor is an integral part of the device and can't be unplugged. OK, well such a user could instead just choose to put a piece of tape over the window or stick all but
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Hi Mike, I'll answer all your questions and express my concerns in this reply, to avoid spreading the info all around the discussion thread. On Mon, 6 Apr 2009 00:19:23 -0500 (CDT), Mike Isely wrote: > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for two pvrusb2 > changesets. One sets up the ability to track what kind of IR receiver > is expected, Looks very good. The more we know about each board, the less probing we have to do, the better. > the other optionally autoloads ir-kbd-i2c based on that > result and a module option that can be used to disable that autoloading. Again, ir-kbd-i2c does _not_ auto-load. What my code (and now yours) does is instantiating an i2c device named "ir-kbd". _If_ the ir-kbd-i2c driver is later loaded, it will bind to that device. We might let udev load the ir-kbd-i2c driver at some point in the future, but clearly we need to sort out the lirc case beforehand, otherwise some users will hit the problems you have anticipated, and we don't want this to happen. So, your module parameter is improperly named. Setting it to 1 does prevent the pvrusb2 driver from instantiating the "ir-kbd" device, and that's all. Speaking of this module parameter, I was a little worried at first that you wanted an inverted logic for it in pvrusb2 compared to what some other bridge drivers (saa7134, em28xx, cx231xx), but thinking a bit more about I came to the conclusion that it was OK because it matched the history of the pvrusb2 driver. Now I see that you followed their logic (enabled is the default) but you use a different module parameter name (disable_autoload_ir_kbd vs. disable_ir). I think there would be some value in sticking to a common name in all bridge drivers (like we have the standard video_nr module parameter.) That being said, I will not insist on this. My feeling is that this is all temporary anyway, because the removal of the legacy i2c model will break lirc and the main point is to decide how we will fix it. I'll post a separate summary with proposals. Depending on what we do, the module parameter you added is likely to become obsolete. > This is the result of what I had posted about an hour ago. It looks > like a lot of lines, but it was all basically trivial stuff. > > Note that these changes are safe; nothing is regressed here and this > does not depend on anyone else's changes. Even though that second > changeset won't really do much until Jean's ir-kbd-i2c stuff is merged, > the fact is that it won't cause any harm either. Since the pvrusb2 > driver wasn't previously autoloading ir-kbd-i2c anyway, this change > therefore breaks nothing. For completeness, your second patch actually breaks the ir-kbd-i2c use case. Your code will instantiate a new-style "ir-kbd" device which the legacy ir-kbd-i2c can't use. As the instantiated device makes the address busy, the probing logic of legacy ir-kbd-i2c driver will skip it. Without my changes, all users will have to pass disable_autoload_ir_kbd=1, whether they want to use lirc or ir-kbd-i2c, otherwise they lose IR support. But honestly that doesn't matter much, I think, because the idea is to merge my changes quickly. > But it does have the desirable effect in that > it should take me out of the IR debate for now and I can stop being a > pain to Jean :-) I am totally fine removing the pvrusb2 parts from my patch set, and letting you deal with it. As you know, my main concern here is to get rid of the legacy i2c model. So I am virtually happy with any solution that leads there, and I am gladly accepting all the help I can get to go there. > Jean: I hope I didn't break any etiquette here. Though that second > change is "from" me, the majority of the changeset really is from your > patch to the pvrusb2 driver. I made changes to it so I didn't feel > right in still trying to blame you for it ;-) Plus, if I were to hand > it back to you for inclusion in your patch series then you would be > dependant upon the pvrusb2 change I made to drive the decision about > loading based on the device hardware. Since this change as a whole is > mergeable without the rest of your changeset I felt it most expedient > just to push this up now. That's alright. In this specific case I really don't care who gets the fame and flames, as longs as things get done quickly. Thank you for jumping in and helping me sorting it all out, this is very appreciated. -- Jean Delvare -- 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for two pvrusb2 changesets. One sets up the ability to track what kind of IR receiver is expected, the other optionally autoloads ir-kbd-i2c based on that result and a module option that can be used to disable that autoloading. This is the result of what I had posted about an hour ago. It looks like a lot of lines, but it was all basically trivial stuff. Note that these changes are safe; nothing is regressed here and this does not depend on anyone else's changes. Even though that second changeset won't really do much until Jean's ir-kbd-i2c stuff is merged, the fact is that it won't cause any harm either. Since the pvrusb2 driver wasn't previously autoloading ir-kbd-i2c anyway, this change therefore breaks nothing. But it does have the desirable effect in that it should take me out of the IR debate for now and I can stop being a pain to Jean :-) Jean: I hope I didn't break any etiquette here. Though that second change is "from" me, the majority of the changeset really is from your patch to the pvrusb2 driver. I made changes to it so I didn't feel right in still trying to blame you for it ;-) Plus, if I were to hand it back to you for inclusion in your patch series then you would be dependant upon the pvrusb2 change I made to drive the decision about loading based on the device hardware. Since this change as a whole is mergeable without the rest of your changeset I felt it most expedient just to push this up now. -Mike - pvrusb2: Select, track, and report IR scheme in use with the device - pvrusb2: autoload ir-kbd when appropriate pvrusb2-devattr.c |1 + pvrusb2-devattr.h | 22 +++--- pvrusb2-hdw-internal.h |3 +++ pvrusb2-hdw.c | 18 +- pvrusb2-i2c-core.c | 40 ++-- 5 files changed, 66 insertions(+), 18 deletions(-) -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for a few pvrusb2 changesets. All should find their way into 2.6.30; two in particular are important bug fixes. -Mike - pvrusb2: Fix incorrect reporting of default value for non-integer controls - pvrusb2: Report def_val items in sysfs symbolically, consistent with cur_val - pvrusb2: Fix uninitialized tuner_setup field(s) pvrusb2-ctrl.c | 12 +--- pvrusb2-hdw.c |1 + pvrusb2-sysfs.c | 14 -- 3 files changed, 14 insertions(+), 13 deletions(-) -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Thu, 26 Mar 2009 09:16:55 -0500 (CDT) Mike Isely wrote: > On Thu, 26 Mar 2009, Mauro Carvalho Chehab wrote: > > > On Tue, 24 Mar 2009 23:07:02 -0500 (CDT) > > Mike Isely wrote: > > > > > > > > Mauro: > > > > > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for a large > > > collection of pvrusb2 changesets (see below). > > > > You forgot to add pvrusb2-cs53l32a.o on your Makefile. > > > > I'll add it and merge with the correspond patch that added this patch, to > > avoid > > bisect breakage. > > Damn. Sorry about that. Usually I *do* catch things like that :-( > Thanks for spotting and fixing it. > > There might be another problem - I did test compile and run all this > from within v4l-dvb. With that file missing it should not have loaded > correctly, which means I must have missed something else too. I will > double check this. Ok. PS: I couldn't push the patch to linuxtv yet, although it is already on my tree. It seems that the linuxtv machine migration is happening right now (or the file system sync). After it returning back, I'll push the patch there. I'll anyway do the -git fix. Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Thu, 26 Mar 2009, Mauro Carvalho Chehab wrote: > On Tue, 24 Mar 2009 23:07:02 -0500 (CDT) > Mike Isely wrote: > > > > > Mauro: > > > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for a large > > collection of pvrusb2 changesets (see below). > > You forgot to add pvrusb2-cs53l32a.o on your Makefile. > > I'll add it and merge with the correspond patch that added this patch, to > avoid > bisect breakage. Damn. Sorry about that. Usually I *do* catch things like that :-( Thanks for spotting and fixing it. There might be another problem - I did test compile and run all this from within v4l-dvb. With that file missing it should not have loaded correctly, which means I must have missed something else too. I will double check this. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Tue, 24 Mar 2009 23:07:02 -0500 (CDT) Mike Isely wrote: > > Mauro: > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for a large > collection of pvrusb2 changesets (see below). You forgot to add pvrusb2-cs53l32a.o on your Makefile. I'll add it and merge with the correspond patch that added this patch, to avoid bisect breakage. Cheers, Mauro. --- linux-2.6.29.noarch.orig/drivers/media/video/pvrusb2/Makefile +++ linux-2.6.29.noarch/drivers/media/video/pvrusb2/Makefile @@ -10,6 +10,7 @@ pvrusb2-objs := pvrusb2-i2c-core.o \ pvrusb2-ctrl.o pvrusb2-std.o pvrusb2-devattr.o \ pvrusb2-context.o pvrusb2-io.o pvrusb2-ioread.o \ pvrusb2-cx2584x-v4l.o pvrusb2-wm8775.o \ + pvrusb2-cs53l32a.o \ $(obj-pvrusb2-dvb-y) \ $(obj-pvrusb2-sysfs-y) $(obj-pvrusb2-debugifc-y) Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Tue, 24 Mar 2009 23:07:02 -0500 (CDT)
Mike Isely wrote:
>
> Mauro:
>
> Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for a large
> collection of pvrusb2 changesets (see below).
Applied, thanks!
> Way back around
> November 2005, I had written a large chunk of code in the driver to
> provide uniform control of the associated i2c modules. With the
> advent of the v4l2-subdev framework within V4L, that old i2c layer in
> the pvrusb2 driver has become obsolete. The vast majority of these
> changes transform the pvrusb2 driver such that it now uses the
> v4l2-subdev framework, replacing the old i2c module control layer. In
> the end, the pvrusb2 driver is somewhat simpler now.
Great!
> I recommend that this whole things find its way into the 2.6.30 kernel
> merge window.
Ok.
>
> Note: This will not be checkpatch.pl "clean".
It was not that bad. For a big series like this, there aren't many violations
(see bellow).
> Though I have taken great care to abide by the questionable space-after-comma
> silliness
> (and boy did that suck - it's not AT ALL easy to change a 21 year
> habit),
I know. I have the same habit of not using spaces after comma. I always need to
review this.
> other warnings will still be present. The high-runner will be
> things related to single-line or compound if-statements. We've had
> that discussion before and I anticipate not to have to repeat it
> again. I don't remember any other warnings of any real consequence.
Except for the if statements, the remaining ones seem to be due to the usage of
more than one instruction per line.
> Oh one thing, part of this series involved an early changeset that
> moves a large chunk of old code to a new file. That changeset will be
> rife with checkpatch.pl complaints, but it's code that is otherwise
> unchanged and I'm not about to sanitize it all purely for the sake of
> that script. (Besides, most if not all of that old code goes away in
> a later changeset anyway.)
I suspect that all code were removed, at least by looking at the checkpatch
when you apply everything just like one big patch.
Cheers,
Mauro
---
FYI, this is the checkpatch.pl for the entire series.
/home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-audio.c: In
' if ((sid < ARRAY_SIZE(routing_schemes))
&&': /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-audio.c:68:
ERROR: do not use assignment in if
condition
/home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-cx2584x-v4l.c:
In 'if ((sid < ARRAY_SIZE(routing_schemes))
&&':
/home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-cx2584x-v4l.c:117:
ERROR: do not use assignment in if
condition /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' if (!src) return
0;': /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:1989:
ERROR: trailing statements should be on next
line /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' if (!i2ccnt && ((p = (mid <
ARRAY_SIZE(module_i2c_addresses)) ?':
/home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:2026:
ERROR: do not use assignment in if
condition /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' default:
break;': /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:2117:
ERROR: trailing statements should be on next
line /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' }':
/home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:2132:
warning: braces {} are not necessary for single statement
blocks /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' if (pvr2_hdw_load_subdev(hdw, &ct->lst[idx]) < 0) okFl =
0;': /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:2138:
ERROR: trailing statements should be on next
line /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' if (!okFl)
pvr2_hdw_render_useless(hdw);':
/home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:2140:
ERROR: trailing statements should be on next
line /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' if (!pvr2_hdw_dev_ok(hdw))
return;': /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:2203:
ERROR: trailing statements should be on next
line /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' if (hdw->tuner_signal_stale)
pvr2_hdw_status_poll(hdw);':
/home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:2996:
ERROR: trailing statements should be on next
line /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' } else
{': /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c:3006:
warning: braces {} are not necessary for any arm of this
statement /home/v4l/master/linux/drivers/media/video/pvrusb2/pvrusb2-hdw.c: In
' if (id >= ARRAY_SIZE(pvr2_module_update_function
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for a large collection of pvrusb2 changesets (see below). Way back around November 2005, I had written a large chunk of code in the driver to provide uniform control of the associated i2c modules. With the advent of the v4l2-subdev framework within V4L, that old i2c layer in the pvrusb2 driver has become obsolete. The vast majority of these changes transform the pvrusb2 driver such that it now uses the v4l2-subdev framework, replacing the old i2c module control layer. In the end, the pvrusb2 driver is somewhat simpler now. I recommend that this whole things find its way into the 2.6.30 kernel merge window. Note: This will not be checkpatch.pl "clean". Though I have taken great care to abide by the questionable space-after-comma silliness (and boy did that suck - it's not AT ALL easy to change a 21 year habit), other warnings will still be present. The high-runner will be things related to single-line or compound if-statements. We've had that discussion before and I anticipate not to have to repeat it again. I don't remember any other warnings of any real consequence. Oh one thing, part of this series involved an early changeset that moves a large chunk of old code to a new file. That changeset will be rife with checkpatch.pl complaints, but it's code that is otherwise unchanged and I'm not about to sanitize it all purely for the sake of that script. (Besides, most if not all of that old code goes away in a later changeset anyway.) -Mike - pvrusb2: Split i2c module handling from i2c adapter - pvrusb2: Set up v4l2_device instance - pvrusb2: Changes to further isolate old i2c layer - pvrusb2: whitespace trivial tweaks - pvrusb2: New device attribute mechanism to specify sub-devices - pvrusb2: Providing means to stop tracking an old i2c module - pvrusb2: whitespace tweaks - pvrusb2: Set i2c autoprobing to be off by default - pvrusb2: Tie up loose ends with v4l2-subdev setup - pvrusb2: Lay foundation for triggering sub-device updates - pvrusb2: Tie-in sub-device log requests - pvrusb2: Tie in debug register access to sub-devices - pvrusb2: Implement status fetching from sub-devices - pvrusb2: Tie in various v4l2 operations into the sub-device mechanism - pvrusb2: Define value for a null sub-device ID - pvrusb2: Note who our video decoder sub-device is, and set it up - pvrusb2: Clean-up / placeholders inserted for additional development - pvrusb2: Tie in sub-device decoder start/stop - pvrusb2: Cause overall initialization to fail if sub-driver(s) fail - pvrusb2: Fix backwards function header comments - pvrusb2: Implement reporting of connected sub-devices - pvrusb2: Implement sub-device specific update framework - pvrusb2: Tie in wm8775 sub-device handling - pvrusb2: Tie in saa7115 sub-device handling - pvrusb2: Make audio sample rate update into a sub-device broadcast - pvrusb2: make sub-device specific update function names uniform - pvrusb2: Tie in msp3400 sub-device support - pvrusb2: Fix silly 80 column issue - pvrusb2: Tie in cx25840 sub-device support - pvrusb2: Implement more sub-device loading trace and improve error handling - pvrusb2: Define default i2c address for wm8775 sub-device - pvrusb2: Fix uninitialized counter - pvrusb2: Fix bugs involved in listing of sub-devices - pvrusb2: Allow sub-devices to insert correctly - pvrusb2: Sub-device update must happen BEFORE state dirty bits are cleared - pvrusb2: Deal with space-after-comma coding style idiocy - pvrusb2: Broadcast tuner type change to sub-devices - pvrusb2: Define default I2C address for cx25840 sub-device - pvrusb2: Implement trace print for stream on / off action - pvrusb2: Correct some trace print inaccuracies - pvrusb2: Implement mechanism to force a full sub-device update - pvrusb2: Issue required core init broadcast to all sub-devices - pvrusb2: Define default I2C addresses for msp3400 and saa7115 sub-devices - pvrusb2: Fix incorrectly named sub-device ID - pvrusb2: Define default I2C address for CS53L32A sub-device - pvrusb2: Convert all device definitions to use new sub-device declarations - pvrusb2: Make a bunch of dvb config structures const (trivial) - pvrusb2: Fix space-after-comma idiocy - pvrusb2: Fix slightly mis-leading header in debug interface output - pvrusb2: Implement better reporting on attached sub-devices - pvrusb2: Remove old i2c layer; we use v4l2-subdev now - pvrusb2: Remove ancient IVTV specific ioctl functions - pvrusb2: Add sub-device for demod - pvrusb2: Add composite and s-video input support for OnAir devices - pvrusb2: Use v4l2_device_disconnect() a/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-chips-v4l2.c | 119 - a/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-cmd-v4l2.c | 339 a/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-cmd-v4l2.h | 53 a/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-track.c | 504 -- a/linux/drivers/media/video/pvrusb2/pvrusb2-i2c-track.h | 102 - a/linux/drivers/medi
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Hi Carsten, On Tue, 27 Jan 2009 14:56:26 +0100 Carsten Meier wrote: > > Hi Thierry, > > > > It may be a good idea to add some code at the v4l2-apps dir for > > retrieving the sysfs place. Maybe we may add it at qv4l2 and at > > v4l2-ctl. > > > > Cheers, > > Mauro > > Hi, > > here is code which I've written for it. It is in C++ with Boost, but > adaption to C shouldn't be hard. It's not extensively tested and put > into the public domain. Thank you for your code. I've converted it to C, and improved it to work also with PCI and PCIe devices. I've made it available at: http://linuxtv.org/hg/v4l-dvb/rev/6e636c8969e8 (and two subsequent patches improving it a little bit) > Maybe it is of use for one or another. For now, it is a separate small application. This makes easy for scripts to use it. It will output both the "raw" data returned by VIDIOC_QUERYCAP and their sysfs path: For a USB device: device = /dev/video0 bus info = usb-:00:1d.7-1 sysfs path = /sys/devices/pci:00/:00:1d.7/usb2/2-1 For a PCI device: device = /dev/video0 bus info = PCI::01:02.0 sysfs path = /sys/devices/pci:00/:00:1e.0/:01:02.0 It works even with devices with internal PCI bridges, like this one: device = /dev/video4 bus info = PCI::02:0f.0 sysfs path = /sys/devices/pci:00/:00:1e.0/:01:01.0/:02:0f.0 device = /dev/video3 bus info = PCI::02:0e.0 sysfs path = /sys/devices/pci:00/:00:1e.0/:01:01.0/:02:0e.0 device = /dev/video2 bus info = PCI::02:0d.0 sysfs path = /sys/devices/pci:00/:00:1e.0/:01:01.0/:02:0d.0 device = /dev/video1 bus info = PCI::02:0c.0 sysfs path = /sys/devices/pci:00/:00:1e.0/:01:01.0/:02:0c.0 I tested here with bttv (a board with 4 bttv chips), saa7134, uvcvideo and em28xx. It should work fine also with other devices. Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Tue, 27 Jan 2009 10:56:35 -0200
schrieb Mauro Carvalho Chehab :
> On Thu, 22 Jan 2009 22:12:08 +0100
> Thierry Merle wrote:
>
> > Laurent Pinchart a écrit :
> > > On Thursday 22 January 2009, Carsten Meier wrote:
> > >> Am Thu, 22 Jan 2009 00:20:00 +0100
> > >>
> > >> schrieb Laurent Pinchart :
> > >>> Hi Carsten,
> > >>>
> > >>> On Wednesday 21 January 2009, Carsten Meier wrote:
> > now I want to translate bus_info into a sysfs-path to obtain
> > device-info like serial numbers. Given a device reports
> > "usb-:00:1d.2-2" as bus_info, then the device-info is
> > located under "/sys/bus/usb/devices/2-2", which is a symlink
> > to the appropriate /sys/devices/ directory, right?
> > >>> I'm afraid not. In the above bus_info value, :00:1d.2 is
> > >>> the PCI bus path of your USB controller, and the last digit
> > >>> after the dash is the USB device path.
> > >>>
> > All I have to do is to compare the first 4 chars of bus_info
> > against "usb-", get the chars after "." and append it to
> > "/sys/bus/usb/devices/" to obatin a sysfs-path, right?
> >
> > Is there a more elegant solution or already a function for
> > this? Can the "." appear more than once before the last one?
> > >>> Probably not before, but definitely after.
> > >>>
> > >>> Root hubs get a USB device path set to '0'. Every other device
> > >>> is numbered according to the hub port number it is connected
> > >>> to. If you have an external hub connected on port 2 of your
> > >>> root hub, and have a webcam connected to port 3 of the external
> > >>> hub, usb_make_path() will return "usb-:00:1d.2-2.3".
> > >>>
> > >>> Cheers,
> > >>>
> > >>> Laurent Pinchart
> > >> Hi,
> > >>
> > >> On my machine, my pvrusb2 (connected directly to my mini-pc)
> > >> shows up under "/sys/bus/usb/devices/7-2/" which is a symbolic
> > >> link to "../../../devices/pci:00/:00:1d.7/usb7/7-2"
> > >
> > > You're just lucky that USB bus 7 (usb7/7) is connected to the 7th
> > > function of your USB host controller (1d.7).
> > >
> > > Here's an example of what I get on my computer:
> > >
> > > /sys/bus/usb/devices/4-2
> > > -> ../../../devices/pci:00/:00:1d.2/usb4/4-2
> > >
> > >> I can't test for the new bus_info-string, because it's not fixed
> > >> yet in the driver. But if I got it correctly it should be
> > >> "usb-:00:1d.7-7.2" ?
> > >
> > > I think you will get usb-:00:1d.7-2
> > >
> > >> Then I've to simply take the string after the last dash, replace
> > >> "." by "-" and append it to "/sys/bus/usb/devices/" for a
> > >> sysfs-path?
> > >
> > > Unfortunately the mapping is not that direct. The part before the
> > > last dash identifies the USB host controller. The part after the
> > > last dash identifies the device path related to the controller,
> > > expressed as a combination of port numbers.
> > >
> > > The sysfs device path /sys/bus/usb/devices/7-2/ includes a USB
> > > bus number (in this case 7) that is not present in
> > > usb_make_path()'s output.
> > >
> > > To find the sysfs path of your USB peripheral, you will have to
> > > find out which bus number the bus name (:00:1d.7) corresponds
> > > to. You might be able to find that by checking each usb[0-9]+
> > > links in /sys/bus/usb/devices and comparing the link's target
> > > with the bus name.
> > >
> > To ease this processing, using libsysfs can be a good idea...
> > On my system, the documentation of libsysfs is here:
> > /usr/doc/sysfsutils-2.1.0/libsysfs.txt
> > Knowing the bus-id, it won't be hard to look at it in data
> > structures. Just my 2 cents.
>
> Hi Thierry,
>
> It may be a good idea to add some code at the v4l2-apps dir for
> retrieving the sysfs place. Maybe we may add it at qv4l2 and at
> v4l2-ctl.
>
> Cheers,
> Mauro
Hi,
here is code which I've written for it. It is in C++ with Boost, but
adaption to C shouldn't be hard. It's not extensively tested and put
into the public domain.
#include
#include
#include
#include
extern "C" {
#include
}
bool obtain_usb_sysfs_path(
const char *busname, const char *usbpath, string &sysfspath_out)
{
bool rc = false;
::sysfs_device *usbctl = NULL;
::sysfs_bus *usb = NULL;
try
{
// open usb host controller
usbctl = ::sysfs_open_device("pci", busname);
if(!usbctl)
throw runtime_error(
"Unable to open usb host controller");
// find matching usb bus
usb = ::sysfs_open_bus("usb");
if(!usb)
throw runtime_error("Unable to open usb");
::dlist *usbdevs = ::sysfs_get_bus_devices(usb);
if(!usbdevs)
throw runtime_error("Unable to get usb devices");
::sysfs_device *usbdev = NULL;
dlist_for_each_data(
us
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Thu, 22 Jan 2009 22:12:08 +0100 Thierry Merle wrote: > Laurent Pinchart a écrit : > > On Thursday 22 January 2009, Carsten Meier wrote: > >> Am Thu, 22 Jan 2009 00:20:00 +0100 > >> > >> schrieb Laurent Pinchart : > >>> Hi Carsten, > >>> > >>> On Wednesday 21 January 2009, Carsten Meier wrote: > now I want to translate bus_info into a sysfs-path to obtain > device-info like serial numbers. Given a device reports > "usb-:00:1d.2-2" as bus_info, then the device-info is located > under "/sys/bus/usb/devices/2-2", which is a symlink to the > appropriate /sys/devices/ directory, right? > >>> I'm afraid not. In the above bus_info value, :00:1d.2 is the PCI > >>> bus path of your USB controller, and the last digit after the dash is > >>> the USB device path. > >>> > All I have to do is to compare the first 4 chars of bus_info against > "usb-", get the chars after "." and append it to > "/sys/bus/usb/devices/" to obatin a sysfs-path, right? > > Is there a more elegant solution or already a function for this? Can > the "." appear more than once before the last one? > >>> Probably not before, but definitely after. > >>> > >>> Root hubs get a USB device path set to '0'. Every other device is > >>> numbered according to the hub port number it is connected to. If you > >>> have an external hub connected on port 2 of your root hub, and have a > >>> webcam connected to port 3 of the external hub, usb_make_path() will > >>> return "usb-:00:1d.2-2.3". > >>> > >>> Cheers, > >>> > >>> Laurent Pinchart > >> Hi, > >> > >> On my machine, my pvrusb2 (connected directly to my mini-pc) shows up > >> under "/sys/bus/usb/devices/7-2/" which is a symbolic link to > >> "../../../devices/pci:00/:00:1d.7/usb7/7-2" > > > > You're just lucky that USB bus 7 (usb7/7) is connected to the 7th function > > of > > your USB host controller (1d.7). > > > > Here's an example of what I get on my computer: > > > > /sys/bus/usb/devices/4-2 -> > > ../../../devices/pci:00/:00:1d.2/usb4/4-2 > > > >> I can't test for the new bus_info-string, because it's not fixed yet in > >> the driver. But if I got it correctly it should be > >> "usb-:00:1d.7-7.2" ? > > > > I think you will get usb-:00:1d.7-2 > > > >> Then I've to simply take the string after the last dash, replace "." by "-" > >> and append it to "/sys/bus/usb/devices/" for a sysfs-path? > > > > Unfortunately the mapping is not that direct. The part before the last dash > > identifies the USB host controller. The part after the last dash identifies > > the device path related to the controller, expressed as a combination of > > port > > numbers. > > > > The sysfs device path /sys/bus/usb/devices/7-2/ includes a USB bus number > > (in > > this case 7) that is not present in usb_make_path()'s output. > > > > To find the sysfs path of your USB peripheral, you will have to find out > > which > > bus number the bus name (:00:1d.7) corresponds to. You might be able to > > find that by checking each usb[0-9]+ links in /sys/bus/usb/devices and > > comparing the link's target with the bus name. > > > To ease this processing, using libsysfs can be a good idea... > On my system, the documentation of libsysfs is here: > /usr/doc/sysfsutils-2.1.0/libsysfs.txt > Knowing the bus-id, it won't be hard to look at it in data structures. > Just my 2 cents. Hi Thierry, It may be a good idea to add some code at the v4l2-apps dir for retrieving the sysfs place. Maybe we may add it at qv4l2 and at v4l2-ctl. Cheers, 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2-pull
Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2-pull for the following: - pvrusb2: Use usb_make_path() to determine device bus location pvrusb2-hdw.c |5 + 1 file changed, 1 insertion(+), 4 deletions(-) This is the usb_make_path() change that's been talked about. Hopefully you'll see this as a real live actual pull request in spite of the subject line having been thoroughly spoiled / thrashed over the past week due to all the subsequent discussion from the last pull request :-) Note also that the pull location is slightly different than what I usually use. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Laurent Pinchart a écrit : > On Thursday 22 January 2009, Carsten Meier wrote: >> Am Thu, 22 Jan 2009 00:20:00 +0100 >> >> schrieb Laurent Pinchart : >>> Hi Carsten, >>> >>> On Wednesday 21 January 2009, Carsten Meier wrote: now I want to translate bus_info into a sysfs-path to obtain device-info like serial numbers. Given a device reports "usb-:00:1d.2-2" as bus_info, then the device-info is located under "/sys/bus/usb/devices/2-2", which is a symlink to the appropriate /sys/devices/ directory, right? >>> I'm afraid not. In the above bus_info value, :00:1d.2 is the PCI >>> bus path of your USB controller, and the last digit after the dash is >>> the USB device path. >>> All I have to do is to compare the first 4 chars of bus_info against "usb-", get the chars after "." and append it to "/sys/bus/usb/devices/" to obatin a sysfs-path, right? Is there a more elegant solution or already a function for this? Can the "." appear more than once before the last one? >>> Probably not before, but definitely after. >>> >>> Root hubs get a USB device path set to '0'. Every other device is >>> numbered according to the hub port number it is connected to. If you >>> have an external hub connected on port 2 of your root hub, and have a >>> webcam connected to port 3 of the external hub, usb_make_path() will >>> return "usb-:00:1d.2-2.3". >>> >>> Cheers, >>> >>> Laurent Pinchart >> Hi, >> >> On my machine, my pvrusb2 (connected directly to my mini-pc) shows up >> under "/sys/bus/usb/devices/7-2/" which is a symbolic link to >> "../../../devices/pci:00/:00:1d.7/usb7/7-2" > > You're just lucky that USB bus 7 (usb7/7) is connected to the 7th function of > your USB host controller (1d.7). > > Here's an example of what I get on my computer: > > /sys/bus/usb/devices/4-2 -> ../../../devices/pci:00/:00:1d.2/usb4/4-2 > >> I can't test for the new bus_info-string, because it's not fixed yet in >> the driver. But if I got it correctly it should be >> "usb-:00:1d.7-7.2" ? > > I think you will get usb-:00:1d.7-2 > >> Then I've to simply take the string after the last dash, replace "." by "-" >> and append it to "/sys/bus/usb/devices/" for a sysfs-path? > > Unfortunately the mapping is not that direct. The part before the last dash > identifies the USB host controller. The part after the last dash identifies > the device path related to the controller, expressed as a combination of port > numbers. > > The sysfs device path /sys/bus/usb/devices/7-2/ includes a USB bus number (in > this case 7) that is not present in usb_make_path()'s output. > > To find the sysfs path of your USB peripheral, you will have to find out > which > bus number the bus name (:00:1d.7) corresponds to. You might be able to > find that by checking each usb[0-9]+ links in /sys/bus/usb/devices and > comparing the link's target with the bus name. > To ease this processing, using libsysfs can be a good idea... On my system, the documentation of libsysfs is here: /usr/doc/sysfsutils-2.1.0/libsysfs.txt Knowing the bus-id, it won't be hard to look at it in data structures. Just my 2 cents. Regard, Thierry -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Thu, 22 Jan 2009 16:57:36 +0100 schrieb Laurent Pinchart : > On Thursday 22 January 2009, Carsten Meier wrote: > > Am Thu, 22 Jan 2009 00:20:00 +0100 > > > > schrieb Laurent Pinchart : > > > Hi Carsten, > > > > > > On Wednesday 21 January 2009, Carsten Meier wrote: > > > > now I want to translate bus_info into a sysfs-path to obtain > > > > device-info like serial numbers. Given a device reports > > > > "usb-:00:1d.2-2" as bus_info, then the device-info is > > > > located under "/sys/bus/usb/devices/2-2", which is a symlink to > > > > the appropriate /sys/devices/ directory, right? > > > > > > I'm afraid not. In the above bus_info value, :00:1d.2 is the > > > PCI bus path of your USB controller, and the last digit after the > > > dash is the USB device path. > > > > > > > All I have to do is to compare the first 4 chars of bus_info > > > > against "usb-", get the chars after "." and append it to > > > > "/sys/bus/usb/devices/" to obatin a sysfs-path, right? > > > > > > > > Is there a more elegant solution or already a function for > > > > this? Can the "." appear more than once before the last one? > > > > > > Probably not before, but definitely after. > > > > > > Root hubs get a USB device path set to '0'. Every other device is > > > numbered according to the hub port number it is connected to. If > > > you have an external hub connected on port 2 of your root hub, > > > and have a webcam connected to port 3 of the external hub, > > > usb_make_path() will return "usb-:00:1d.2-2.3". > > > > > > Cheers, > > > > > > Laurent Pinchart > > > > Hi, > > > > On my machine, my pvrusb2 (connected directly to my mini-pc) shows > > up under "/sys/bus/usb/devices/7-2/" which is a symbolic link to > > "../../../devices/pci:00/:00:1d.7/usb7/7-2" > > You're just lucky that USB bus 7 (usb7/7) is connected to the 7th > function of your USB host controller (1d.7). > > Here's an example of what I get on my computer: > > /sys/bus/usb/devices/4-2 > -> ../../../devices/pci:00/:00:1d.2/usb4/4-2 > > > I can't test for the new bus_info-string, because it's not fixed > > yet in the driver. But if I got it correctly it should be > > "usb-:00:1d.7-7.2" ? > > I think you will get usb-:00:1d.7-2 > > > Then I've to simply take the string after the last dash, replace > > "." by "-" and append it to "/sys/bus/usb/devices/" for a > > sysfs-path? > > Unfortunately the mapping is not that direct. The part before the > last dash identifies the USB host controller. The part after the last > dash identifies the device path related to the controller, expressed > as a combination of port numbers. > > The sysfs device path /sys/bus/usb/devices/7-2/ includes a USB bus > number (in this case 7) that is not present in usb_make_path()'s > output. > > To find the sysfs path of your USB peripheral, you will have to find > out which bus number the bus name (:00:1d.7) corresponds to. You > might be able to find that by checking each usb[0-9]+ links > in /sys/bus/usb/devices and comparing the link's target with the bus > name. > > Best regards, > > Laurent Pinchart Hi Laurent, could you please post what your video-device reports for bus_info together with the /sys/bus/usb-path where it shows up and the location where the symlink points to? This thing makes me going nuts... :( Thanks, Carsten -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Thursday 22 January 2009, Carsten Meier wrote: > Am Thu, 22 Jan 2009 00:20:00 +0100 > > schrieb Laurent Pinchart : > > Hi Carsten, > > > > On Wednesday 21 January 2009, Carsten Meier wrote: > > > now I want to translate bus_info into a sysfs-path to obtain > > > device-info like serial numbers. Given a device reports > > > "usb-:00:1d.2-2" as bus_info, then the device-info is located > > > under "/sys/bus/usb/devices/2-2", which is a symlink to the > > > appropriate /sys/devices/ directory, right? > > > > I'm afraid not. In the above bus_info value, :00:1d.2 is the PCI > > bus path of your USB controller, and the last digit after the dash is > > the USB device path. > > > > > All I have to do is to compare the first 4 chars of bus_info against > > > "usb-", get the chars after "." and append it to > > > "/sys/bus/usb/devices/" to obatin a sysfs-path, right? > > > > > > Is there a more elegant solution or already a function for this? Can > > > the "." appear more than once before the last one? > > > > Probably not before, but definitely after. > > > > Root hubs get a USB device path set to '0'. Every other device is > > numbered according to the hub port number it is connected to. If you > > have an external hub connected on port 2 of your root hub, and have a > > webcam connected to port 3 of the external hub, usb_make_path() will > > return "usb-:00:1d.2-2.3". > > > > Cheers, > > > > Laurent Pinchart > > Hi, > > On my machine, my pvrusb2 (connected directly to my mini-pc) shows up > under "/sys/bus/usb/devices/7-2/" which is a symbolic link to > "../../../devices/pci:00/:00:1d.7/usb7/7-2" You're just lucky that USB bus 7 (usb7/7) is connected to the 7th function of your USB host controller (1d.7). Here's an example of what I get on my computer: /sys/bus/usb/devices/4-2 -> ../../../devices/pci:00/:00:1d.2/usb4/4-2 > I can't test for the new bus_info-string, because it's not fixed yet in > the driver. But if I got it correctly it should be > "usb-:00:1d.7-7.2" ? I think you will get usb-:00:1d.7-2 > Then I've to simply take the string after the last dash, replace "." by "-" > and append it to "/sys/bus/usb/devices/" for a sysfs-path? Unfortunately the mapping is not that direct. The part before the last dash identifies the USB host controller. The part after the last dash identifies the device path related to the controller, expressed as a combination of port numbers. The sysfs device path /sys/bus/usb/devices/7-2/ includes a USB bus number (in this case 7) that is not present in usb_make_path()'s output. To find the sysfs path of your USB peripheral, you will have to find out which bus number the bus name (:00:1d.7) corresponds to. You might be able to find that by checking each usb[0-9]+ links in /sys/bus/usb/devices and comparing the link's target with the bus name. Best 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Thu, 22 Jan 2009 00:20:00 +0100 schrieb Laurent Pinchart : > Hi Carsten, > > On Wednesday 21 January 2009, Carsten Meier wrote: > > > > now I want to translate bus_info into a sysfs-path to obtain > > device-info like serial numbers. Given a device reports > > "usb-:00:1d.2-2" as bus_info, then the device-info is located > > under "/sys/bus/usb/devices/2-2", which is a symlink to the > > appropriate /sys/devices/ directory, right? > > I'm afraid not. In the above bus_info value, :00:1d.2 is the PCI > bus path of your USB controller, and the last digit after the dash is > the USB device path. > > > All I have to do is to compare the first 4 chars of bus_info against > > "usb-", get the chars after "." and append it to > > "/sys/bus/usb/devices/" to obatin a sysfs-path, right? > > > > Is there a more elegant solution or already a function for this? Can > > the "." appear more than once before the last one? > > Probably not before, but definitely after. > > Root hubs get a USB device path set to '0'. Every other device is > numbered according to the hub port number it is connected to. If you > have an external hub connected on port 2 of your root hub, and have a > webcam connected to port 3 of the external hub, usb_make_path() will > return "usb-:00:1d.2-2.3". > > Cheers, > > Laurent Pinchart Hi, On my machine, my pvrusb2 (connected directly to my mini-pc) shows up under "/sys/bus/usb/devices/7-2/" which is a symbolic link to "../../../devices/pci:00/:00:1d.7/usb7/7-2" I can't test for the new bus_info-string, because it's not fixed yet in the driver. But if I got it correctly it should be "usb-:00:1d.7-7.2" ? Then I've to simply take the string after the last dash, replace "." by "-" and append it to "/sys/bus/usb/devices/" for a sysfs-path? Regards, Carsten -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Hi Carsten, On Wednesday 21 January 2009, Carsten Meier wrote: > > now I want to translate bus_info into a sysfs-path to obtain > device-info like serial numbers. Given a device reports > "usb-:00:1d.2-2" as bus_info, then the device-info is located > under "/sys/bus/usb/devices/2-2", which is a symlink to the > appropriate /sys/devices/ directory, right? I'm afraid not. In the above bus_info value, :00:1d.2 is the PCI bus path of your USB controller, and the last digit after the dash is the USB device path. > All I have to do is to compare the first 4 chars of bus_info against > "usb-", get the chars after "." and append it to > "/sys/bus/usb/devices/" to obatin a sysfs-path, right? > > Is there a more elegant solution or already a function for this? Can > the "." appear more than once before the last one? Probably not before, but definitely after. Root hubs get a USB device path set to '0'. Every other device is numbered according to the hub port number it is connected to. If you have an external hub connected on port 2 of your root hub, and have a webcam connected to port 3 of the external hub, usb_make_path() will return "usb-:00:1d.2-2.3". Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Mon, 19 Jan 2009 04:25:54 +0100
schrieb Carsten Meier :
> Am Mon, 19 Jan 2009 06:11:33 +0300
> schrieb "Alexey Klimov" :
>
> > On Mon, Jan 19, 2009 at 6:09 AM, Alexey Klimov
> > wrote:
> > > On Mon, 2009-01-19 at 03:55 +0100, Carsten Meier wrote:
> > >> Am Mon, 19 Jan 2009 05:25:15 +0300
> > >> schrieb Alexey Klimov :
> > >>
> > >> > On Sun, 2009-01-18 at 23:36 -0200, Mauro Carvalho Chehab wrote:
> > >> > > On Sat, 17 Jan 2009 19:09:51 +0100
> > >> > > Carsten Meier wrote:
> > >> > >
> > >> > > > Am Fri, 16 Jan 2009 02:47:50 -0200
> > >> > > > schrieb Mauro Carvalho Chehab :
> > >> > > >
> > >> > > > > For usb devices, usb_make_path() provide a canonical name
> > >> > > > > for the device. For PCI ones, we have pci_name() for the
> > >> > > > > same function. in the case of pci devices, I suspect that
> > >> > > > > all use pci_name(). We just need to use usb_make_path()
> > >> > > > > at the usb ones.
> > >> > > >
> > >> > > > I looked at the sources for what string gets generated for
> > >> > > > bus_info by usb_make_path(). If it gets used by pvrusb2, my
> > >> > > > problems are solved, because it is constant across
> > >> > > > standby-wake-up-cycles. The pvrusb2's implementation
> > >> > > > currently delivers "usb 7-2 address 6" here. "address 6"
> > >> > > > corresponds to devnum which gets constantly increased,
> > >> > > > which results in always changing strings here. Sorry for my
> > >> > > > unneccessary complaints.
> > >> > >
> > >> > > Mike, Thierry, Jean-Francois, Laurent and others:
> > >> > >
> > >> > > IMO, we should patch all usb drivers to use usb_make_path().
> > >> > > It will be more transparent to userspace, if all drivers
> > >> > > provide the bus_info using the same notation. Comments?
> > >> >
> > >> > I did this thing to dsbr100.c usb radio driver:
> > >> >
> > >> > diff -r de513684aca2 linux/drivers/media/radio/dsbr100.c
> > >> > --- a/linux/drivers/media/radio/dsbr100.c Sun Jan 18 23:06:34
> > >> > 2009 -0200 +++ b/linux/drivers/media/radio/dsbr100.cMon
> > >> > Jan 19 05:18:36 2009 +0300 @@ -393,9 +393,12 @@
> > >> > static int vidioc_querycap(struct file *file, void *priv,
> > >> > struct v4l2_capability *v)
> > >> > {
> > >> > + struct dsbr100_device *radio = video_drvdata(file);
> > >> > +
> > >> > strlcpy(v->driver, "dsbr100", sizeof(v->driver));
> > >> > strlcpy(v->card, "D-Link R-100 USB FM Radio",
> > >> > sizeof(v->card));
> > >> > - sprintf(v->bus_info, "USB");
> > >> > + usb_make_path(radio->usbdev, v->bus_info,
> > >> > sizeof(v->bus_info));
> > >> > + printk(KERN_INFO "%s\n", v->bus_info);
> > >> > v->version = RADIO_VERSION;
> > >> > v->capabilities = V4L2_CAP_TUNER;
> > >> > return 0;
> > >> >
> > >> > And get such dmesg messages for different usb ports:
> > >> >
> > >> > usb-:00:1d.2-2
> > >> >
> > >> > usb-:00:1d.0-1
> > >> >
> > >> > Looks okay to my eyes or may be i missed something. Anyway it's
> > >> > more useful than just simple "USB" string.
> > >> >
> > >> Do you get the same string if you unplug and then replug the same
> > >> device to the same port? If not, I have the same problems than
> > >> before, otherwise I'll be happy with it.
> > >
> > > To be sure that i did that you want me to: i plug device in, run
> > > application that use it, close application, unplug device, and
> > > then plug device in (and i didn't reload the kernel module during
> > > this), run app again..
> > >
> > > Yes, i have the same string in this case. btw, kernel 2.6.29-rc2.
> >
> > Oh, i forget to tell you that it was the same usb port and the same
> > usb device. :)
> >
> >
> Yes, that's what I meant. :) Userspace-land would be very happy if
> usb_make_path() is used by all usb-device-drivers.
Hi,
now I want to translate bus_info into a sysfs-path to obtain
device-info like serial numbers. Given a device reports
"usb-:00:1d.2-2" as bus_info, then the device-info is located
under "/sys/bus/usb/devices/2-2", which is a symlink to the
appropriate /sys/devices/ directory, right?
All I have to do is to compare the first 4 chars of bus_info against
"usb-", get the chars after "." and append it to
"/sys/bus/usb/devices/" to obatin a sysfs-path, right?
Is there a more elegant solution or already a function for this? Can
the "." appear more than once before the last one?
Thaks,
Carsten
--
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Tue, 20 Jan 2009 20:54:24 +0100 Thierry Merle wrote: > Mauro Carvalho Chehab a écrit : > > On Sat, 17 Jan 2009 19:09:51 +0100 > > Carsten Meier wrote: > > > >> Am Fri, 16 Jan 2009 02:47:50 -0200 > >> schrieb Mauro Carvalho Chehab : > >> > >>> For usb devices, usb_make_path() provide a canonical name for the > >>> device. For PCI ones, we have pci_name() for the same function. in > >>> the case of pci devices, I suspect that all use pci_name(). We just > >>> need to use usb_make_path() at the usb ones. > >>> > >> I looked at the sources for what string gets generated for bus_info by > >> usb_make_path(). If it gets used by pvrusb2, my problems are solved, > >> because it is constant across standby-wake-up-cycles. The pvrusb2's > >> implementation currently delivers "usb 7-2 address 6" here. "address > >> 6" corresponds to devnum which gets constantly increased, which results > >> in always changing strings here. Sorry for my unneccessary complaints. > > > > Mike, Thierry, Jean-Francois, Laurent and others: > > > > IMO, we should patch all usb drivers to use usb_make_path(). It will be more > > transparent to userspace, if all drivers provide the bus_info using the same > > notation. Comments? > > > In fact the usbvision code was reporting already a portable information using > dev_name. > strlcpy(vc->bus_info, dev_name(&usbvision->dev->dev), > sizeof(vc->bus_info)); > changed to: > usb_make_path(usbvision->dev, vc->bus_info, sizeof(vc->bus_info)); > shows the same information on my system. > It is simpler so I am OK with this change. > Do you want us to make separate patch or did you start a global patch? I prefer if you do. I'm very busy today, and I still have lots of patches pending at patchwork. If you have some time, it would help if you could make a patch fixing the other drivers as well. Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Tue, 20 Jan 2009, Thierry Merle wrote: > Mauro Carvalho Chehab a écrit : > > On Sat, 17 Jan 2009 19:09:51 +0100 > > Carsten Meier wrote: > > > >> Am Fri, 16 Jan 2009 02:47:50 -0200 > >> schrieb Mauro Carvalho Chehab : > >> > >>> For usb devices, usb_make_path() provide a canonical name for the > >>> device. For PCI ones, we have pci_name() for the same function. in > >>> the case of pci devices, I suspect that all use pci_name(). We just > >>> need to use usb_make_path() at the usb ones. > >>> > >> I looked at the sources for what string gets generated for bus_info by > >> usb_make_path(). If it gets used by pvrusb2, my problems are solved, > >> because it is constant across standby-wake-up-cycles. The pvrusb2's > >> implementation currently delivers "usb 7-2 address 6" here. "address > >> 6" corresponds to devnum which gets constantly increased, which results > >> in always changing strings here. Sorry for my unneccessary complaints. > > > > Mike, Thierry, Jean-Francois, Laurent and others: > > > > IMO, we should patch all usb drivers to use usb_make_path(). It will be more > > transparent to userspace, if all drivers provide the bus_info using the same > > notation. Comments? > > > In fact the usbvision code was reporting already a portable information using > dev_name. > strlcpy(vc->bus_info, dev_name(&usbvision->dev->dev), > sizeof(vc->bus_info)); > changed to: > usb_make_path(usbvision->dev, vc->bus_info, sizeof(vc->bus_info)); > shows the same information on my system. > It is simpler so I am OK with this change. > Do you want us to make separate patch or did you start a global patch? > Cheers, > Thierry I will update the pvrusb2 driver. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Mauro Carvalho Chehab a écrit : > On Sat, 17 Jan 2009 19:09:51 +0100 > Carsten Meier wrote: > >> Am Fri, 16 Jan 2009 02:47:50 -0200 >> schrieb Mauro Carvalho Chehab : >> >>> For usb devices, usb_make_path() provide a canonical name for the >>> device. For PCI ones, we have pci_name() for the same function. in >>> the case of pci devices, I suspect that all use pci_name(). We just >>> need to use usb_make_path() at the usb ones. >>> >> I looked at the sources for what string gets generated for bus_info by >> usb_make_path(). If it gets used by pvrusb2, my problems are solved, >> because it is constant across standby-wake-up-cycles. The pvrusb2's >> implementation currently delivers "usb 7-2 address 6" here. "address >> 6" corresponds to devnum which gets constantly increased, which results >> in always changing strings here. Sorry for my unneccessary complaints. > > Mike, Thierry, Jean-Francois, Laurent and others: > > IMO, we should patch all usb drivers to use usb_make_path(). It will be more > transparent to userspace, if all drivers provide the bus_info using the same > notation. Comments? > In fact the usbvision code was reporting already a portable information using dev_name. strlcpy(vc->bus_info, dev_name(&usbvision->dev->dev), sizeof(vc->bus_info)); changed to: usb_make_path(usbvision->dev, vc->bus_info, sizeof(vc->bus_info)); shows the same information on my system. It is simpler so I am OK with this change. Do you want us to make separate patch or did you start a global patch? Cheers, Thierry -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Hi Mauro, On Monday 19 January 2009, Mauro Carvalho Chehab wrote: > On Sat, 17 Jan 2009 19:09:51 +0100 > > Carsten Meier wrote: > > Am Fri, 16 Jan 2009 02:47:50 -0200 > > > > schrieb Mauro Carvalho Chehab : > > > For usb devices, usb_make_path() provide a canonical name for the > > > device. For PCI ones, we have pci_name() for the same function. in > > > the case of pci devices, I suspect that all use pci_name(). We just > > > need to use usb_make_path() at the usb ones. > > > > I looked at the sources for what string gets generated for bus_info by > > usb_make_path(). If it gets used by pvrusb2, my problems are solved, > > because it is constant across standby-wake-up-cycles. The pvrusb2's > > implementation currently delivers "usb 7-2 address 6" here. "address > > 6" corresponds to devnum which gets constantly increased, which results > > in always changing strings here. Sorry for my unneccessary complaints. > > Mike, Thierry, Jean-Francois, Laurent and others: > > IMO, we should patch all usb drivers to use usb_make_path(). It will be > more transparent to userspace, if all drivers provide the bus_info using > the same notation. Comments? According to the V4L2 spec, bus_info is the "location of the device in the system [...]. For example: 'PCI Slot 4'." Using usb_make_path() makes sense for USB devices (although this might still not be stable enough according to Carsten). The V4L2 specification should be updated to mandate usage of usb_make_path() for USB devices. Do you plan to submit a patch to fix/switch all the V4L2 USB drivers ? Best 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Sun, 18 Jan 2009, Mauro Carvalho Chehab wrote: > On Sat, 17 Jan 2009 19:09:51 +0100 > Carsten Meier wrote: > > > Am Fri, 16 Jan 2009 02:47:50 -0200 > > schrieb Mauro Carvalho Chehab : > > > > > For usb devices, usb_make_path() provide a canonical name for the > > > device. For PCI ones, we have pci_name() for the same function. in > > > the case of pci devices, I suspect that all use pci_name(). We just > > > need to use usb_make_path() at the usb ones. > > > > > > > I looked at the sources for what string gets generated for bus_info by > > usb_make_path(). If it gets used by pvrusb2, my problems are solved, > > because it is constant across standby-wake-up-cycles. The pvrusb2's > > implementation currently delivers "usb 7-2 address 6" here. "address > > 6" corresponds to devnum which gets constantly increased, which results > > in always changing strings here. Sorry for my unneccessary complaints. > > Mike, Thierry, Jean-Francois, Laurent and others: > > IMO, we should patch all usb drivers to use usb_make_path(). It will be more > transparent to userspace, if all drivers provide the bus_info using the same > notation. Comments? On the surface this sounds agreeable. As soon as I am done cleaning up after my plumbing near-disaster here, I plan on investigating and making the appropriate change(s) to the pvrusb2 driver for this. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Mon, 19 Jan 2009 06:11:33 +0300
schrieb "Alexey Klimov" :
> On Mon, Jan 19, 2009 at 6:09 AM, Alexey Klimov
> wrote:
> > On Mon, 2009-01-19 at 03:55 +0100, Carsten Meier wrote:
> >> Am Mon, 19 Jan 2009 05:25:15 +0300
> >> schrieb Alexey Klimov :
> >>
> >> > On Sun, 2009-01-18 at 23:36 -0200, Mauro Carvalho Chehab wrote:
> >> > > On Sat, 17 Jan 2009 19:09:51 +0100
> >> > > Carsten Meier wrote:
> >> > >
> >> > > > Am Fri, 16 Jan 2009 02:47:50 -0200
> >> > > > schrieb Mauro Carvalho Chehab :
> >> > > >
> >> > > > > For usb devices, usb_make_path() provide a canonical name
> >> > > > > for the device. For PCI ones, we have pci_name() for the
> >> > > > > same function. in the case of pci devices, I suspect that
> >> > > > > all use pci_name(). We just need to use usb_make_path() at
> >> > > > > the usb ones.
> >> > > >
> >> > > > I looked at the sources for what string gets generated for
> >> > > > bus_info by usb_make_path(). If it gets used by pvrusb2, my
> >> > > > problems are solved, because it is constant across
> >> > > > standby-wake-up-cycles. The pvrusb2's implementation
> >> > > > currently delivers "usb 7-2 address 6" here. "address 6"
> >> > > > corresponds to devnum which gets constantly increased, which
> >> > > > results in always changing strings here. Sorry for my
> >> > > > unneccessary complaints.
> >> > >
> >> > > Mike, Thierry, Jean-Francois, Laurent and others:
> >> > >
> >> > > IMO, we should patch all usb drivers to use usb_make_path(). It
> >> > > will be more transparent to userspace, if all drivers provide
> >> > > the bus_info using the same notation. Comments?
> >> >
> >> > I did this thing to dsbr100.c usb radio driver:
> >> >
> >> > diff -r de513684aca2 linux/drivers/media/radio/dsbr100.c
> >> > --- a/linux/drivers/media/radio/dsbr100.c Sun Jan 18 23:06:34
> >> > 2009 -0200 +++ b/linux/drivers/media/radio/dsbr100.cMon
> >> > Jan 19 05:18:36 2009 +0300 @@ -393,9 +393,12 @@
> >> > static int vidioc_querycap(struct file *file, void *priv,
> >> > struct v4l2_capability *v)
> >> > {
> >> > + struct dsbr100_device *radio = video_drvdata(file);
> >> > +
> >> > strlcpy(v->driver, "dsbr100", sizeof(v->driver));
> >> > strlcpy(v->card, "D-Link R-100 USB FM Radio",
> >> > sizeof(v->card));
> >> > - sprintf(v->bus_info, "USB");
> >> > + usb_make_path(radio->usbdev, v->bus_info,
> >> > sizeof(v->bus_info));
> >> > + printk(KERN_INFO "%s\n", v->bus_info);
> >> > v->version = RADIO_VERSION;
> >> > v->capabilities = V4L2_CAP_TUNER;
> >> > return 0;
> >> >
> >> > And get such dmesg messages for different usb ports:
> >> >
> >> > usb-:00:1d.2-2
> >> >
> >> > usb-:00:1d.0-1
> >> >
> >> > Looks okay to my eyes or may be i missed something. Anyway it's
> >> > more useful than just simple "USB" string.
> >> >
> >> Do you get the same string if you unplug and then replug the same
> >> device to the same port? If not, I have the same problems than
> >> before, otherwise I'll be happy with it.
> >
> > To be sure that i did that you want me to: i plug device in, run
> > application that use it, close application, unplug device, and then
> > plug device in (and i didn't reload the kernel module during this),
> > run app again..
> >
> > Yes, i have the same string in this case. btw, kernel 2.6.29-rc2.
>
> Oh, i forget to tell you that it was the same usb port and the same
> usb device. :)
>
>
Yes, that's what I meant. :) Userspace-land would be very happy if
usb_make_path() is used by all usb-device-drivers.
--
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Mon, 2009-01-19 at 03:55 +0100, Carsten Meier wrote:
> Am Mon, 19 Jan 2009 05:25:15 +0300
> schrieb Alexey Klimov :
>
> > On Sun, 2009-01-18 at 23:36 -0200, Mauro Carvalho Chehab wrote:
> > > On Sat, 17 Jan 2009 19:09:51 +0100
> > > Carsten Meier wrote:
> > >
> > > > Am Fri, 16 Jan 2009 02:47:50 -0200
> > > > schrieb Mauro Carvalho Chehab :
> > > >
> > > > > For usb devices, usb_make_path() provide a canonical name for
> > > > > the device. For PCI ones, we have pci_name() for the same
> > > > > function. in the case of pci devices, I suspect that all use
> > > > > pci_name(). We just need to use usb_make_path() at the usb
> > > > > ones.
> > > >
> > > > I looked at the sources for what string gets generated for
> > > > bus_info by usb_make_path(). If it gets used by pvrusb2, my
> > > > problems are solved, because it is constant across
> > > > standby-wake-up-cycles. The pvrusb2's implementation currently
> > > > delivers "usb 7-2 address 6" here. "address 6" corresponds to
> > > > devnum which gets constantly increased, which results in always
> > > > changing strings here. Sorry for my unneccessary complaints.
> > >
> > > Mike, Thierry, Jean-Francois, Laurent and others:
> > >
> > > IMO, we should patch all usb drivers to use usb_make_path(). It
> > > will be more transparent to userspace, if all drivers provide the
> > > bus_info using the same notation. Comments?
> >
> > I did this thing to dsbr100.c usb radio driver:
> >
> > diff -r de513684aca2 linux/drivers/media/radio/dsbr100.c
> > --- a/linux/drivers/media/radio/dsbr100.c Sun Jan 18 23:06:34
> > 2009 -0200 +++ b/linux/drivers/media/radio/dsbr100.cMon Jan
> > 19 05:18:36 2009 +0300 @@ -393,9 +393,12 @@
> > static int vidioc_querycap(struct file *file, void *priv,
> > struct v4l2_capability *v)
> > {
> > + struct dsbr100_device *radio = video_drvdata(file);
> > +
> > strlcpy(v->driver, "dsbr100", sizeof(v->driver));
> > strlcpy(v->card, "D-Link R-100 USB FM Radio",
> > sizeof(v->card));
> > - sprintf(v->bus_info, "USB");
> > + usb_make_path(radio->usbdev, v->bus_info,
> > sizeof(v->bus_info));
> > + printk(KERN_INFO "%s\n", v->bus_info);
> > v->version = RADIO_VERSION;
> > v->capabilities = V4L2_CAP_TUNER;
> > return 0;
> >
> > And get such dmesg messages for different usb ports:
> >
> > usb-:00:1d.2-2
> >
> > usb-:00:1d.0-1
> >
> > Looks okay to my eyes or may be i missed something. Anyway it's more
> > useful than just simple "USB" string.
> >
> Do you get the same string if you unplug and then replug the same
> device to the same port? If not, I have the same problems than before,
> otherwise I'll be happy with it.
To be sure that i did that you want me to: i plug device in, run
application that use it, close application, unplug device, and then plug
device in (and i didn't reload the kernel module during this), run app
again..
Yes, i have the same string in this case. btw, kernel 2.6.29-rc2.
--
Best regards, Klimov Alexey
--
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Mon, 19 Jan 2009 05:25:15 +0300
schrieb Alexey Klimov :
> On Sun, 2009-01-18 at 23:36 -0200, Mauro Carvalho Chehab wrote:
> > On Sat, 17 Jan 2009 19:09:51 +0100
> > Carsten Meier wrote:
> >
> > > Am Fri, 16 Jan 2009 02:47:50 -0200
> > > schrieb Mauro Carvalho Chehab :
> > >
> > > > For usb devices, usb_make_path() provide a canonical name for
> > > > the device. For PCI ones, we have pci_name() for the same
> > > > function. in the case of pci devices, I suspect that all use
> > > > pci_name(). We just need to use usb_make_path() at the usb
> > > > ones.
> > >
> > > I looked at the sources for what string gets generated for
> > > bus_info by usb_make_path(). If it gets used by pvrusb2, my
> > > problems are solved, because it is constant across
> > > standby-wake-up-cycles. The pvrusb2's implementation currently
> > > delivers "usb 7-2 address 6" here. "address 6" corresponds to
> > > devnum which gets constantly increased, which results in always
> > > changing strings here. Sorry for my unneccessary complaints.
> >
> > Mike, Thierry, Jean-Francois, Laurent and others:
> >
> > IMO, we should patch all usb drivers to use usb_make_path(). It
> > will be more transparent to userspace, if all drivers provide the
> > bus_info using the same notation. Comments?
>
> I did this thing to dsbr100.c usb radio driver:
>
> diff -r de513684aca2 linux/drivers/media/radio/dsbr100.c
> --- a/linux/drivers/media/radio/dsbr100.c Sun Jan 18 23:06:34
> 2009 -0200 +++ b/linux/drivers/media/radio/dsbr100.c Mon Jan
> 19 05:18:36 2009 +0300 @@ -393,9 +393,12 @@
> static int vidioc_querycap(struct file *file, void *priv,
> struct v4l2_capability *v)
> {
> + struct dsbr100_device *radio = video_drvdata(file);
> +
> strlcpy(v->driver, "dsbr100", sizeof(v->driver));
> strlcpy(v->card, "D-Link R-100 USB FM Radio",
> sizeof(v->card));
> - sprintf(v->bus_info, "USB");
> + usb_make_path(radio->usbdev, v->bus_info,
> sizeof(v->bus_info));
> + printk(KERN_INFO "%s\n", v->bus_info);
> v->version = RADIO_VERSION;
> v->capabilities = V4L2_CAP_TUNER;
> return 0;
>
> And get such dmesg messages for different usb ports:
>
> usb-:00:1d.2-2
>
> usb-:00:1d.0-1
>
> Looks okay to my eyes or may be i missed something. Anyway it's more
> useful than just simple "USB" string.
>
>
Do you get the same string if you unplug and then replug the same
device to the same port? If not, I have the same problems than before,
otherwise I'll be happy with it.
Regards,
Carsten
--
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Sun, 2009-01-18 at 23:36 -0200, Mauro Carvalho Chehab wrote:
> On Sat, 17 Jan 2009 19:09:51 +0100
> Carsten Meier wrote:
>
> > Am Fri, 16 Jan 2009 02:47:50 -0200
> > schrieb Mauro Carvalho Chehab :
> >
> > > For usb devices, usb_make_path() provide a canonical name for the
> > > device. For PCI ones, we have pci_name() for the same function. in
> > > the case of pci devices, I suspect that all use pci_name(). We just
> > > need to use usb_make_path() at the usb ones.
> > >
> >
> > I looked at the sources for what string gets generated for bus_info by
> > usb_make_path(). If it gets used by pvrusb2, my problems are solved,
> > because it is constant across standby-wake-up-cycles. The pvrusb2's
> > implementation currently delivers "usb 7-2 address 6" here. "address
> > 6" corresponds to devnum which gets constantly increased, which results
> > in always changing strings here. Sorry for my unneccessary complaints.
>
> Mike, Thierry, Jean-Francois, Laurent and others:
>
> IMO, we should patch all usb drivers to use usb_make_path(). It will be more
> transparent to userspace, if all drivers provide the bus_info using the same
> notation. Comments?
I did this thing to dsbr100.c usb radio driver:
diff -r de513684aca2 linux/drivers/media/radio/dsbr100.c
--- a/linux/drivers/media/radio/dsbr100.c Sun Jan 18 23:06:34 2009 -0200
+++ b/linux/drivers/media/radio/dsbr100.c Mon Jan 19 05:18:36 2009 +0300
@@ -393,9 +393,12 @@
static int vidioc_querycap(struct file *file, void *priv,
struct v4l2_capability *v)
{
+ struct dsbr100_device *radio = video_drvdata(file);
+
strlcpy(v->driver, "dsbr100", sizeof(v->driver));
strlcpy(v->card, "D-Link R-100 USB FM Radio", sizeof(v->card));
- sprintf(v->bus_info, "USB");
+ usb_make_path(radio->usbdev, v->bus_info, sizeof(v->bus_info));
+ printk(KERN_INFO "%s\n", v->bus_info);
v->version = RADIO_VERSION;
v->capabilities = V4L2_CAP_TUNER;
return 0;
And get such dmesg messages for different usb ports:
usb-:00:1d.2-2
usb-:00:1d.0-1
Looks okay to my eyes or may be i missed something. Anyway it's more useful
than just simple "USB" string.
--
Best regards, Klimov Alexey
--
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Sat, 17 Jan 2009 19:09:51 +0100 Carsten Meier wrote: > Am Fri, 16 Jan 2009 02:47:50 -0200 > schrieb Mauro Carvalho Chehab : > > > For usb devices, usb_make_path() provide a canonical name for the > > device. For PCI ones, we have pci_name() for the same function. in > > the case of pci devices, I suspect that all use pci_name(). We just > > need to use usb_make_path() at the usb ones. > > > > I looked at the sources for what string gets generated for bus_info by > usb_make_path(). If it gets used by pvrusb2, my problems are solved, > because it is constant across standby-wake-up-cycles. The pvrusb2's > implementation currently delivers "usb 7-2 address 6" here. "address > 6" corresponds to devnum which gets constantly increased, which results > in always changing strings here. Sorry for my unneccessary complaints. Mike, Thierry, Jean-Francois, Laurent and others: IMO, we should patch all usb drivers to use usb_make_path(). It will be more transparent to userspace, if all drivers provide the bus_info using the same notation. Comments? Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Fri, 16 Jan 2009 02:47:50 -0200 schrieb Mauro Carvalho Chehab : > For usb devices, usb_make_path() provide a canonical name for the > device. For PCI ones, we have pci_name() for the same function. in > the case of pci devices, I suspect that all use pci_name(). We just > need to use usb_make_path() at the usb ones. > I looked at the sources for what string gets generated for bus_info by usb_make_path(). If it gets used by pvrusb2, my problems are solved, because it is constant across standby-wake-up-cycles. The pvrusb2's implementation currently delivers "usb 7-2 address 6" here. "address 6" corresponds to devnum which gets constantly increased, which results in always changing strings here. Sorry for my unneccessary complaints. Regards, Carsten -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Fri, 16 Jan 2009 12:36:43 +0100 schrieb Carsten Meier : > To make this mechanism work reliably with USB, the meaning of the > field could be adapted to something like a binary identifier string. I really meant "opaque identifier string". -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Fri, 16 Jan 2009, Janne Grunau wrote:
> On Friday 16 January 2009 15:39:33 Mike Isely wrote:
> > In any case, right now the serial number in the pvrusb2 is not available
> > through that means because I haven't done anything to make it available
> > to udev. I'd like to do something, but so far I have found no
> > information on how to make that happen.
>
> You shouldn't need to do anything special. The serial number is available
> through the parent USB device. It can be used for udev rules through
> ATTRS{serial} and in sysfs entry of the video device through device/serial.
>
Ah yes!
What I said before was right in its own context, but now I see something
else.
The serial number that the pvrusb2 driver itself knows about is parsed
out of Hauppauge private data by the tveeprom module from the device's
internal I2C ROM. This data is formatted in a packetized way that is
specific to Hauppauge. What I was refering to was *that* serial number,
and since it's in the private ROM I saw no means for udev to know about
it.
However I just tested with two 24xxx devices using usbview, and the same
serial number is in fact visible in the USB configuration data.
There's simply no way for the USB core in Linux to be able to directly
know about, get at, or even understand that internal ROM. Yet there it
is. I'm thinking now that perhaps the FX2 microcontroller is either
parsing out the data itself during initialization and then writing out
the USB configuration accordingly, or the serial number is in fact
written in two places within the device! Up until now, I had not seen
any evidence to suggest that the FX2 firmware ever actually read the
nearby ROM on its own. But that could be what is happening here.
Thanks for pointing that out. These devices still surprise me.
For anyone looking at this, the serial number in the USB configuration
data for the device is just a hex-formatted version of the same value
that you can see via the pvrusb2 driver's sysfs interface.
-Mike
--
Mike Isely
isely @ pobox (dot) com
PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Friday 16 January 2009 15:39:33 Mike Isely wrote:
> In any case, right now the serial number in the pvrusb2 is not available
> through that means because I haven't done anything to make it available
> to udev. I'd like to do something, but so far I have found no
> information on how to make that happen.
You shouldn't need to do anything special. The serial number is available
through the parent USB device. It can be used for udev rules through
ATTRS{serial} and in sysfs entry of the video device through device/serial.
Janne
--
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Thu, 15 Jan 2009, Mauro Carvalho Chehab wrote: > > OK. Well, the usage he wants is something that is better fitted by using sysfs > info. There, you should have all information to uniquely identify a device: > driver, bus location (on PCI this can be relevant), MAC (for dvb devices), > etc. > IMO, the proper way is to add the serial number at sysfs, and let the bus_info > point to the proper bus location. Having the bus location, an userspace app > can > seek the sysfs and look for the udev info. > > For example, on an em28xx analog device I have here, bus_info returns > "1-3". Ok, this is a very bad way to specify the bus. IMO, we should use > usb_make_path() to generate the canonical name for USB buses. I will review what the pvrusb2 driver is doing and change it to use usb_make_path() if needed. However given all the other information about the device that querycap returns, a serial number in that batch would be right at home there. Requiring an app to go through everything you described is a pretty complex process for what should be very simple datum. In any case, right now the serial number in the pvrusb2 is not available through that means because I haven't done anything to make it available to udev. I'd like to do something, but so far I have found no information on how to make that happen. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Am Fri, 16 Jan 2009 02:47:50 -0200 schrieb Mauro Carvalho Chehab : > On Thu, 15 Jan 2009 18:41:33 +0100 > Carsten Meier wrote: > > > > OK. Well, the usage he wants is something that is better fitted by > > > using sysfs info. There, you should have all information to > > > uniquely identify a device: driver, bus location (on PCI this can > > > be relevant), MAC (for dvb devices), etc. IMO, the proper way is > > > to add the serial number at sysfs, and let the bus_info point to > > > the proper bus location. Having the bus location, an userspace > > > app can seek the sysfs and look for the udev info. > > > > > > For example, on an em28xx analog device I have here, bus_info > > > returns "1-3". Ok, this is a very bad way to specify the bus. > > > IMO, we should use usb_make_path() to generate the canonical name > > > for USB buses. > > > > > > Anyway, by getting this information, and looking at sysfs, we'll > > > see lots of very useful information [1]. > > > > > > By parsing the info, we know that: > > > the v4l driver is em28xx; > > > the audio driver is snd-usb-audio (card1); > > > the event interface is input10; > > > this device uses two i2c modules: saa7115 and tuner; > > > the device has no dvb interface. > > > > > > It should be easy to extend it to provide more info, like the > > > serial number. This will also help udev to create a permanent > > > name for the known devices, as it does with internet interfaces. > > > > > > Cheers, > > > Mauro > > > > Hi, > > > > I'm the originator of this topic. > > > > Sorry, but I don't agree here. If I start to parse the bus_info > > string, my app will only work with device-drivers which return a > > canonical sysfs-path here. > > Since some devices currently doesn't provide an unique name at > bus_info, any solution will be applied on kernel 2.6.30 anyway. The > window for such changes on 2.6.29 were already closed. > > > To achive this, actually a new field in the > > capability-struct must be defined, > > Adding an extra string field for VIDIOC_QUERYCAP will break binary > only compatibility. This is not allowed. > No, it won't. There are reserved fields (16 bytes) at the end of the struct which can be used. To retain source-compatibility every app is advised to use a private copy of the header-file. > > because the specs don't put any > > contrains on the bus_info-string. (some space is left there) > > Any solution will need to review the specs. > Yes, but newer apps which are parsing bus_info then won't work with older kernels. This won't happen if a new field gets defined. > > And even if I parse the string to somehow obtain a sysfs-path, then > > I have to distinguish devices which have a serial-no and which > > don't. This leads to a unmaintainable situation. > > Almost all devices don't provide a readable serial number, or we > don't know how to obtain it. We just know the serial numbers on the > devices for one manufacturer (and a few other USB ones that provide a > serial number at the usb serial id field). > bus_info is *only* intended for users to distinguish multiple device instances. That's clearly stated by the specs. It was designed with PCI-cards in mind which always return the same location (PCI-slot). Applications usually then do a string comparision on card and bus_info to associate the device with its internal configuration or data structures. To make this mechanism work reliably with USB, the meaning of the field could be adapted to something like a binary identifier string. It is only required that this identifier won't change if the device is connected to the same port through the same hub. If the device has a unique serial-no., it is a perfect candidate for this string (then even the connection-path may change), otherwise some constant USB-info could be used. (This is what I meant by best guess above) This approach won't break anything (only a small addition to the docs for USB-devices is required, in that a constant identifier is required) Drivers which currently return an always changing bus_info break the v4l2-specs because they make it impossible for apps (and users) to distinguish devices reliably (which is required by v4l2). OK, just my 2 dollars :) Regards, Carsten -- 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for: - pvrusb2: Issue VIDIOC_INT_INIT to v4l2 modules when they first attach - pvrusb2: Code module name directly in printk pvrusb2-i2c-chips-v4l2.c | 23 +-- pvrusb2-i2c-cmd-v4l2.c | 14 ++ pvrusb2-i2c-cmd-v4l2.h |1 + pvrusb2-i2c-core.c |2 +- pvrusb2-main.c |4 ++-- 5 files changed, 31 insertions(+), 13 deletions(-) Thanks. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Thu, 15 Jan 2009 18:41:33 +0100 Carsten Meier wrote: > > OK. Well, the usage he wants is something that is better fitted by > > using sysfs info. There, you should have all information to uniquely > > identify a device: driver, bus location (on PCI this can be relevant), > > MAC (for dvb devices), etc. IMO, the proper way is to add the serial > > number at sysfs, and let the bus_info point to the proper bus > > location. Having the bus location, an userspace app can seek the > > sysfs and look for the udev info. > > > > For example, on an em28xx analog device I have here, bus_info returns > > "1-3". Ok, this is a very bad way to specify the bus. IMO, we should > > use usb_make_path() to generate the canonical name for USB buses. > > > > Anyway, by getting this information, and looking at sysfs, we'll see > > lots of very useful information [1]. > > > > By parsing the info, we know that: > > the v4l driver is em28xx; > > the audio driver is snd-usb-audio (card1); > > the event interface is input10; > > this device uses two i2c modules: saa7115 and tuner; > > the device has no dvb interface. > > > > It should be easy to extend it to provide more info, like the serial > > number. This will also help udev to create a permanent name for the > > known devices, as it does with internet interfaces. > > > > Cheers, > > Mauro > > Hi, > > I'm the originator of this topic. > > Sorry, but I don't agree here. If I start to parse the bus_info string, > my app will only work with device-drivers which return a canonical > sysfs-path here. Since some devices currently doesn't provide an unique name at bus_info, any solution will be applied on kernel 2.6.30 anyway. The window for such changes on 2.6.29 were already closed. > To achive this, actually a new field in the > capability-struct must be defined, Adding an extra string field for VIDIOC_QUERYCAP will break binary only compatibility. This is not allowed. > because the specs don't put any > contrains on the bus_info-string. (some space is left there) Any solution will need to review the specs. > And even if I parse the string to somehow obtain a sysfs-path, then I > have to distinguish devices which have a serial-no and which don't. > This leads to a unmaintainable situation. Almost all devices don't provide a readable serial number, or we don't know how to obtain it. We just know the serial numbers on the devices for one manufacturer (and a few other USB ones that provide a serial number at the usb serial id field). > What bus_info must be to make it of any use is that it must contain a > string that won't change if the device reconnects (at least to the same > port; manually or due to system-wakeup) A driver can do its best guess > here (serial-no, slot or *constant* USB-port or whatever) I think this > is the *only* V4L2-conforming solution to the problem. For usb devices, usb_make_path() provide a canonical name for the device. For PCI ones, we have pci_name() for the same function. in the case of pci devices, I suspect that all use pci_name(). We just need to use usb_make_path() at the usb ones. Currently, only the IR part of most of the drivers use the usb canonical name. It is trivial to convert the remaining ones to use it. It is just a matter of replacing the string copy function to a usb_make_path() call. Something like replacing: strlcpy(cap->bus_info,pvr2_hdw_get_bus_info(hdw), sizeof(cap->bus_info)); by: usb_make_path(hdw->usb_dev, cap->bus_info, sizeof(cap->bus_info)); Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
> OK. Well, the usage he wants is something that is better fitted by > using sysfs info. There, you should have all information to uniquely > identify a device: driver, bus location (on PCI this can be relevant), > MAC (for dvb devices), etc. IMO, the proper way is to add the serial > number at sysfs, and let the bus_info point to the proper bus > location. Having the bus location, an userspace app can seek the > sysfs and look for the udev info. > > For example, on an em28xx analog device I have here, bus_info returns > "1-3". Ok, this is a very bad way to specify the bus. IMO, we should > use usb_make_path() to generate the canonical name for USB buses. > > Anyway, by getting this information, and looking at sysfs, we'll see > lots of very useful information [1]. > > By parsing the info, we know that: > the v4l driver is em28xx; > the audio driver is snd-usb-audio (card1); > the event interface is input10; > this device uses two i2c modules: saa7115 and tuner; > the device has no dvb interface. > > It should be easy to extend it to provide more info, like the serial > number. This will also help udev to create a permanent name for the > known devices, as it does with internet interfaces. > > Cheers, > Mauro Hi, I'm the originator of this topic. Sorry, but I don't agree here. If I start to parse the bus_info string, my app will only work with device-drivers which return a canonical sysfs-path here. To achive this, actually a new field in the capability-struct must be defined, because the specs don't put any contrains on the bus_info-string. (some space is left there) And even if I parse the string to somehow obtain a sysfs-path, then I have to distinguish devices which have a serial-no and which don't. This leads to a unmaintainable situation. What bus_info must be to make it of any use is that it must contain a string that won't change if the device reconnects (at least to the same port; manually or due to system-wakeup) A driver can do its best guess here (serial-no, slot or *constant* USB-port or whatever) I think this is the *only* V4L2-conforming solution to the problem. Regards, Carsten -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Wed, 14 Jan 2009 21:01:47 -0600 (CST) Mike Isely wrote: > On Thu, 15 Jan 2009, Mauro Carvalho Chehab wrote: > > > On Wed, 14 Jan 2009 10:44:38 -0600 (CST) > > Mike Isely wrote: > >[...] > > > > > > And I'd appreciate some suggestions from anyone about a means via the > > > V4L interface to make available a device-specific identifier, like a > > > serial number. Yes I know such a thing is not always available with all > > > devices, but it is available for Hauppauge devices in general and users > > > want to have access to that information for purposes of mapping behavior > > > in userspace. > > > > Sorry, but I didn't understand the need. What do you want to map in > > userspace? > > Except for debugging purposes, or when the user has more than one device > > connected, I can't see any other usage for such identifier. > > > > For debug, you can just print the serial number. For device unique > > identification, there are the sysfs stuff, used by udev. > > > > Am I missing something? > > > > Rather than paraphrase it all here, I invite you to look at the > discussion thread on the pvrusb2 list where the topic came up: > > http://www.isely.net/pipermail/pvrusb2/2009-January/002091.html > > After looking over that discussion, let's talk about it. I think I know > where you are heading with this, but I have other questions (if I'm > right about where you're heading) and I'd like you to see that thread > first. OK. Well, the usage he wants is something that is better fitted by using sysfs info. There, you should have all information to uniquely identify a device: driver, bus location (on PCI this can be relevant), MAC (for dvb devices), etc. IMO, the proper way is to add the serial number at sysfs, and let the bus_info point to the proper bus location. Having the bus location, an userspace app can seek the sysfs and look for the udev info. For example, on an em28xx analog device I have here, bus_info returns "1-3". Ok, this is a very bad way to specify the bus. IMO, we should use usb_make_path() to generate the canonical name for USB buses. Anyway, by getting this information, and looking at sysfs, we'll see lots of very useful information [1]. By parsing the info, we know that: the v4l driver is em28xx; the audio driver is snd-usb-audio (card1); the event interface is input10; this device uses two i2c modules: saa7115 and tuner; the device has no dvb interface. It should be easy to extend it to provide more info, like the serial number. This will also help udev to create a permanent name for the known devices, as it does with internet interfaces. Cheers, Mauro [1] This is the info generated by em28xx driver: /sys/bus/usb/devices/1-3 |-- 1-3:1.0 | |-- bAlternateSetting | |-- bInterfaceClass | |-- bInterfaceNumber | |-- bInterfaceProtocol | |-- bInterfaceSubClass | |-- bNumEndpoints | |-- driver -> ../../../../../../bus/usb/drivers/em28xx | |-- ep_81 -> usb_endpoint/usbdev1.2_ep81 | |-- ep_82 -> usb_endpoint/usbdev1.2_ep82 | |-- ep_84 -> usb_endpoint/usbdev1.2_ep84 | |-- modalias | |-- power | | `-- wakeup | |-- subsystem -> ../../../../../../bus/usb | |-- uevent | `-- usb_endpoint | |-- usbdev1.2_ep81 | | |-- bEndpointAddress | | |-- bInterval | | |-- bLength | | |-- bmAttributes | | |-- dev | | |-- device -> ../../../1-3:1.0 | | |-- direction | | |-- interval | | |-- power | | | `-- wakeup | | |-- subsystem -> ../../../../../../../../class/usb_endpoint | | |-- type | | |-- uevent | | `-- wMaxPacketSize | |-- usbdev1.2_ep82 | | |-- bEndpointAddress | | |-- bInterval | | |-- bLength | | |-- bmAttributes | | |-- dev | | |-- device -> ../../../1-3:1.0 | | |-- direction | | |-- interval | | |-- power | | | `-- wakeup | | |-- subsystem -> ../../../../../../../../class/usb_endpoint | | |-- type | | |-- uevent | | `-- wMaxPacketSize | `-- usbdev1.2_ep84 | |-- bEndpointAddress | |-- bInterval | |-- bLength | |-- bmAttributes | |-- dev | |-- device -> ../../../1-3:1.0 | |-- direction | |-- interval | |-- power | | `-- wakeup | |-- subsystem -> ../../../../../../../../class/usb_endpoint | |-- type | |-- uevent | `-- wMaxPacketSize |-- 1-3:1.1 | |-- bAlternateSetting | |-- bInterfaceClass | |-- bInterfaceNumber | |-- bInterfaceProtocol | |-- bInterfaceSubClass | |-- bNumEndpoints | |-- driver -> ../../../../../../bus/usb/drivers/snd-usb-audio | |-- modalias | |-- power | | `-- wakeup | |-- sound | | `-- card1 | | |-- audio1 | | | |-- dev | | | |-- device -> ../../ca
Re: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Thu, 15 Jan 2009, Mauro Carvalho Chehab wrote: > On Wed, 14 Jan 2009 10:44:38 -0600 (CST) > Mike Isely wrote: [...] > > > And I'd appreciate some suggestions from anyone about a means via the > > V4L interface to make available a device-specific identifier, like a > > serial number. Yes I know such a thing is not always available with all > > devices, but it is available for Hauppauge devices in general and users > > want to have access to that information for purposes of mapping behavior > > in userspace. > > Sorry, but I didn't understand the need. What do you want to map in userspace? > Except for debugging purposes, or when the user has more than one device > connected, I can't see any other usage for such identifier. > > For debug, you can just print the serial number. For device unique > identification, there are the sysfs stuff, used by udev. > > Am I missing something? > Rather than paraphrase it all here, I invite you to look at the discussion thread on the pvrusb2 list where the topic came up: http://www.isely.net/pipermail/pvrusb2/2009-January/002091.html After looking over that discussion, let's talk about it. I think I know where you are heading with this, but I have other questions (if I'm right about where you're heading) and I'd like you to see that thread first. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Wed, 14 Jan 2009 10:44:38 -0600 (CST) Mike Isely wrote: > Mauro: > > If you still don't like this specific changeset, then OK, but I'd still > appreciate it if you could pull the other changes (especially the > v4l2_subdev fix). While discussing the serial number patch, I've picked the other ones. > On Wed, 14 Jan 2009, Mike Isely wrote: > > > On Wed, 14 Jan 2009, Mauro Carvalho Chehab wrote: > > > >[...] > > > > > > > > I can see some troubles here: > > > > > > 1) The bus info helps to identify the place where you'll find the > > > device info at sysfs; > > > > > > 2) This is a V4L2 API non-compliance. All drivers should be compliant > > > with the API; > > > > > > 3) If we all agree that bus_info doesn't matter at all and decide to > > > change V4L2 API, we'll still have a big trouble: most devices don't > > > have a serial number. > > > > > > The other patches are ok. > > > > I was asked to make this change, because otherwise there's no means via > > the V4L interface to uniquely REPEATABLY be able to identify the same > > device each time it is plugged in. I have gotten complaints about this. > > I actually pointed out that bus_info was about the "where" not the > > "what" of the device, but I was convinced to change this - after being > > surprised that the V4L spec allowed for this. Look at the online v4l > > spec; the following description exists about bus_info (as part of the > > VIDIOC_QUERYCAP description): > > Interesting... Yet, IMO, we should be more strict about bus_info. This can be used as a way to associate a /dev/ with their sysfs entry. > And I'd appreciate some suggestions from anyone about a means via the > V4L interface to make available a device-specific identifier, like a > serial number. Yes I know such a thing is not always available with all > devices, but it is available for Hauppauge devices in general and users > want to have access to that information for purposes of mapping behavior > in userspace. Sorry, but I didn't understand the need. What do you want to map in userspace? Except for debugging purposes, or when the user has more than one device connected, I can't see any other usage for such identifier. For debug, you can just print the serial number. For device unique identification, there are the sysfs stuff, used by udev. Am I missing something? Cheers, 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Wednesday 14 January 2009 17:32:07 Mike Isely wrote:
> I was asked to make this change, because otherwise there's no means via
> the V4L interface to uniquely REPEATABLY be able to identify the same
> device each time it is plugged in. I have gotten complaints about this.
The serial number of the usb device should be available to udev through
ATTRS{serial}. That's how we solved it for owners of multilpe HD PVRs.
HTH Janne
--
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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Wednesday 14 January 2009 17:44:38 Mike Isely wrote: > On Wed, 14 Jan 2009, Mike Isely wrote: > > On Wed, 14 Jan 2009, Mauro Carvalho Chehab wrote: > > > >[...] > > > > > I can see some troubles here: > > > > > > 1) The bus info helps to identify the place where you'll find the > > > device info at sysfs; > > > > > > 2) This is a V4L2 API non-compliance. All drivers should be > > > compliant with the API; > > > > > > 3) If we all agree that bus_info doesn't matter at all and decide > > > to change V4L2 API, we'll still have a big trouble: most devices > > > don't have a serial number. > > > > > > The other patches are ok. > > > > I was asked to make this change, because otherwise there's no means > > via the V4L interface to uniquely REPEATABLY be able to identify > > the same device each time it is plugged in. I have gotten > > complaints about this. I actually pointed out that bus_info was > > about the "where" not the "what" of the device, but I was convinced > > to change this - after being surprised that the V4L spec allowed > > for this. Look at the online v4l spec; the following description > > exists about bus_info (as part of the VIDIOC_QUERYCAP description): > >[...] > > Mauro: > > If you still don't like this specific changeset, then OK, but I'd > still appreciate it if you could pull the other changes (especially > the v4l2_subdev fix). Mauro, I agree with Mike on this: please pull at least the v4l2_subdev fix which must go asap to 2.6.29 as well. It's a very embarrassing bug that I introduced and that Mike found (thanks Mike!). Regards, Hans > And I'd appreciate some suggestions from anyone about a means via the > V4L interface to make available a device-specific identifier, like a > serial number. Yes I know such a thing is not always available with > all devices, but it is available for Hauppauge devices in general and > users want to have access to that information for purposes of mapping > behavior in userspace. > > -Mike -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Wed, 14 Jan 2009, Mike Isely wrote: > On Wed, 14 Jan 2009, Mauro Carvalho Chehab wrote: > >[...] > > > > > I can see some troubles here: > > > > 1) The bus info helps to identify the place where you'll find the > > device info at sysfs; > > > > 2) This is a V4L2 API non-compliance. All drivers should be compliant > > with the API; > > > > 3) If we all agree that bus_info doesn't matter at all and decide to > > change V4L2 API, we'll still have a big trouble: most devices don't > > have a serial number. > > > > The other patches are ok. > > I was asked to make this change, because otherwise there's no means via > the V4L interface to uniquely REPEATABLY be able to identify the same > device each time it is plugged in. I have gotten complaints about this. > I actually pointed out that bus_info was about the "where" not the > "what" of the device, but I was convinced to change this - after being > surprised that the V4L spec allowed for this. Look at the online v4l > spec; the following description exists about bus_info (as part of the > VIDIOC_QUERYCAP description): > [...] Mauro: If you still don't like this specific changeset, then OK, but I'd still appreciate it if you could pull the other changes (especially the v4l2_subdev fix). And I'd appreciate some suggestions from anyone about a means via the V4L interface to make available a device-specific identifier, like a serial number. Yes I know such a thing is not always available with all devices, but it is available for Hauppauge devices in general and users want to have access to that information for purposes of mapping behavior in userspace. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Wed, 14 Jan 2009, Mauro Carvalho Chehab wrote: [...] > > I can see some troubles here: > > 1) The bus info helps to identify the place where you'll find the > device info at sysfs; > > 2) This is a V4L2 API non-compliance. All drivers should be compliant > with the API; > > 3) If we all agree that bus_info doesn't matter at all and decide to > change V4L2 API, we'll still have a big trouble: most devices don't > have a serial number. > > The other patches are ok. I was asked to make this change, because otherwise there's no means via the V4L interface to uniquely REPEATABLY be able to identify the same device each time it is plugged in. I have gotten complaints about this. I actually pointed out that bus_info was about the "where" not the "what" of the device, but I was convinced to change this - after being surprised that the V4L spec allowed for this. Look at the online v4l spec; the following description exists about bus_info (as part of the VIDIOC_QUERYCAP description): "Location of the device in the system, a NUL-terminated ASCII string. For example: "PCI Slot 4". This information is intended for users, to distinguish multiple identical devices. If no such information is available the field may simply count the devices controlled by the driver, or contain the empty string (bus_info[0] = 0)." Based on that description, the actual format of the string depends a lot on the type of the device and that the only real requirement is that it distinguish among multiple devices. The changeset I posted above still fulfills those requirements. How can my change be a non-compliance given the above actual specification in the V4L2 spec? There's absolutely nothing there stating that the information must be usable to map a sysfs path. Surely the published V4L2 spec is the last word on this question... -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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: [PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
On Wed, 14 Jan 2009 08:52:09 -0600 (CST) Mike Isely wrote: > > Mauro: > > Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for: > About this changeset: > - pvrusb2: Report device serial number in bus-info field Previously the V4L bus-info capability reported USB address, a fairly useless thing for anyone but a USB core maintainer (IMHO). This change replaces that information with the device's serial number if possible, or otherwise a unit number. Priority: normal Signed-off-by: Mike Isely --- a/linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c Wed Jan 14 01:24:20 2009 -0600 +++ b/linux/drivers/media/video/pvrusb2/pvrusb2-v4l2.c Wed Jan 14 01:30:53 2009 -0600 @@ -205,7 +205,7 @@ static long pvr2_v4l2_do_ioctl(struct fi struct v4l2_capability *cap = arg; memcpy(cap, &pvr_capability, sizeof(struct v4l2_capability)); - strlcpy(cap->bus_info,pvr2_hdw_get_bus_info(hdw), + strlcpy(cap->bus_info, pvr2_hdw_get_device_identifier(hdw), sizeof(cap->bus_info)); strlcpy(cap->card,pvr2_hdw_get_desc(hdw),sizeof(cap->card)); I can see some troubles here: 1) The bus info helps to identify the place where you'll find the device info at sysfs; 2) This is a V4L2 API non-compliance. All drivers should be compliant with the API; 3) If we all agree that bus_info doesn't matter at all and decide to change V4L2 API, we'll still have a big trouble: most devices don't have a serial number. The other patches are ok. Cheers, 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
[PULL] http://linuxtv.org/hg/~mcisely/pvrusb2
Mauro: Please pull from http://linuxtv.org/hg/~mcisely/pvrusb2 for: - pvrusb2: Stop advertising VBI capability - it isn't there - pvrusb2: Generate a device-unique identifier - pvrusb2: Change sysfs serial number handling - pvrusb2: Report device serial number in bus-info field - pvrusb2: Fix misleading comment caused by earlier commit - Fix obvious swapped names in v4l2_subdev logic pvrusb2/pvrusb2-hdw-internal.h | 12 pvrusb2/pvrusb2-hdw.c | 19 +++ pvrusb2/pvrusb2-hdw.h |6 -- pvrusb2/pvrusb2-sysfs.c| 12 ++-- pvrusb2/pvrusb2-v4l2.c |4 ++-- v4l2-subdev.c |4 ++-- 6 files changed, 41 insertions(+), 16 deletions(-) IMPORTANT NOTE: The v4l2-subdev.c is a fix for a regression that breaks most use of the pvrusb2 driver (or probably any other driver which relies on a v4l2_subdev-converted module and attempts a queryctrl or querymenu operation). The problem is generic to the v4l2_subdev support, and is a trivially obvious fix. That one should be fast-tracked into 2.6.29. -Mike -- Mike Isely isely @ pobox (dot) com PGP: 03 54 43 4D 75 E5 CC 92 71 16 01 E2 B5 F5 C1 E8 -- 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
