From: "Frank Schäfer" <fschaefer.oss@googlemail.com>
To: dheitmueller@kernellabs.com
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH 00/23] em28xx: add support fur USB bulk transfers
Date: Tue, 23 Oct 2012 22:02:24 +0300 [thread overview]
Message-ID: <5086E9C0.5060701@googlemail.com> (raw)
In-Reply-To: <CAGoCfiw-nL03s=JSc_MVzR0+hQEfHV5i+FMf41EbEME8jw3wQg@mail.gmail.com>
Hi,
Am 21.10.2012 21:13, schrieb Devin Heitmueller:
> Hi Frank,
>
> On Sun, Oct 21, 2012 at 12:52 PM, Frank Schäfer
> <fschaefer.oss@googlemail.com> wrote:
>> This patch series adds support for USB bulk transfers to the em28xx driver.
> This is a welcome change that some users have been asking about for a while.
Yes, I know...
>
>> Patch 1 is a bugfix for the image data processing with non-interlaced devices (webcams)
>> that should be considered for stable (see commit message).
>>
>> Patches 2-21 extend the driver to support USB bulk transfers.
>> USB endpoint mapping had to be extended and is a bit tricky.
>> It might still not be sufficient to handle ALL isoc/bulk endpoints of ALL existing devices,
>> but it should work with the devices we have seen so far and (most important !)
>> preserves backwards compatibility to the current driver behavior.
>> Isoc endpoints/transfers are preffered by default, patch 21 adds a module parameter to change this behavior.
>>
>> The last two patches are follow-up patches not really related to USB tranfers.
>> Patch 22 reduces the code size in em28xx-video by merging the two URB data processing functions
> This is generally good stuff. When I originally added the VBI
> support, I kept the URB handlers separate initially to reduce the risk
> of breaking existing devices, and always assumed that at some point
> the two routines would be merged. You did regression test without VBI
> support enabled though, right?
Yes, but when you take a look at the code, you will see that this patch
nothing really changes for VBI devices.
The problem / regression potential is the non-VBI-devices as they are
now using the VBI-version, too, but they have been tested.
Btw, why didn't you test this function with VBI disabled when you added
it ? ;)
>
>> and patch 23 enables VBI-support for em2840-devices.
> Patch 23 shouldn't be applied unless somebody has an em2840 device to
> test with first. Nobody has complained about this so far, and it's
> better to not support VBI than to possibly break existing support.
Btw, what about em2874 / em2884 / em28174 ?
We should really sort these kind of things out when adding new devices...
>
>> Please note that I could test the changes with an analog non-interlaced non-VBI device only !
>> So further tests with DVB/interlaced/VBI devices are strongly recommended !
> So here's the problem: I don't have the cycles to test this, and all
> the refactoring presents a very real risk that breakage of existing
> support could occur. You've basically got three options if you want
> to see this merged upstream:
>
> 1. Wait for me to eventually do the testing.
> 2. Plead for users to do testing, in particular of the VBI support
> for interlaced devices (which is 99% of devices out there)
> 3. See if Mauro has time to do the testing.
I would say 1 + 2 + 3 ;)
And maybe it's a good chance for the people who were asking for this
feature in the past.
I know there are lots of other people on this list having such a device.
> 4. Spend $30 and buy one of the dozens of em28xx based analog capture
> devices out there and do the validation yourself (a huge percentage of
> the "Video tape capture devices" are em28xx based. For example, when
> I did the original VBI work, I used the following:
>
> KWorld DVD Maker USB 2.0 VS- USB2800 USB 2.0 Interface
> http://www.newegg.com/Product/Product.aspx?Item=N82E16815100112
>
> If you're in the United States, I can mail you a device for testing.
> But given how dirt-cheap they are, buying one yourself might be easier
> (and if the money is really the issue, send me your paypal details
> offline and I'll give you the $30.00).
No, thank you. I already have too many devices which I actually don't need.
I'm doing this as a hobby and at the moment, I'm focussed on getting the
devices I already have working properly (which isn't a small task).
I personally don't need this feature uptsream at the moment.
The device I used for testing supports ISOC as well and the em25xx
webcam I'm currently working on will likely use gspca at the end.
> Thanks for you hard work on this, and it will be great to get this
> stuff into the mainline.
I did what I could do. Now its up to others ;)
Regards,
Frank
> Devin
>
next prev parent reply other threads:[~2012-10-23 20:02 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-21 16:52 [PATCH 00/23] em28xx: add support fur USB bulk transfers Frank Schäfer
2012-10-21 16:52 ` [PATCH 01/23] em28xx: fix wrong data offset for non-interlaced mode in em28xx_copy_video Frank Schäfer
2012-10-21 16:52 ` [PATCH 02/23] em28xx: clarify meaning of field 'progressive' in struct em28xx Frank Schäfer
2012-10-21 16:52 ` [PATCH 03/23] em28xx: rename isoc packet number constants and parameters Frank Schäfer
2012-10-21 16:52 ` [PATCH 04/23] em28xx: rename struct em28xx_usb_isoc_bufs to em28xx_usb_bufs Frank Schäfer
2012-10-21 16:52 ` [PATCH 05/23] em28xx: rename struct em28xx_usb_isoc_ctl to em28xx_usb_ctl Frank Schäfer
2012-10-21 16:52 ` [PATCH 06/23] em28xx: remove obsolete #define EM28XX_URB_TIMEOUT Frank Schäfer
2012-10-21 16:52 ` [PATCH 07/23] em28xx: update description of em28xx_irq_callback Frank Schäfer
2012-10-21 16:52 ` [PATCH 08/23] em28xx: rename function em28xx_uninit_isoc to em28xx_uninit_usb_xfer Frank Schäfer
2012-10-21 16:52 ` [PATCH 09/23] em28xx: create a common function for isoc and bulk URB allocation and setup Frank Schäfer
2012-10-21 16:52 ` [PATCH 10/23] em28xx: create a common function for isoc and bulk USB transfer initialization Frank Schäfer
2012-10-21 16:52 ` Frank Schäfer
2012-10-21 16:52 ` [PATCH 11/23] em28xx: clear USB halt/stall condition in em28xx_init_usb_xfer when using bulk transfers Frank Schäfer
2012-10-21 16:52 ` [PATCH 12/23] em28xx: remove double checks for urb->status == -ENOENT in urb_data_copy functions Frank Schäfer
2012-10-21 16:52 ` [PATCH 13/23] em28xx: rename function em28xx_isoc_copy and extend for USB bulk transfers Frank Schäfer
2012-10-21 16:52 ` [PATCH 14/23] em28xx: rename function em28xx_isoc_copy_vbi " Frank Schäfer
2012-10-21 16:52 ` [PATCH 15/23] em28xx: rename function em28xx_dvb_isoc_copy " Frank Schäfer
2012-10-21 16:52 ` [PATCH 16/23] em28xx: rename usb debugging module parameter and macro Frank Schäfer
2012-10-21 16:52 ` [PATCH 17/23] em28xx: rename some USB parameter fields in struct em28xx to clarify their role Frank Schäfer
2012-10-21 16:52 ` [PATCH 18/23] em28xx: add fields for analog and DVB USB transfer type selection to struct em28xx Frank Schäfer
2012-10-21 16:52 ` [PATCH 19/23] em28xx: set USB alternate settings for analog video bulk transfers properly Frank Schäfer
2012-10-21 16:52 ` [PATCH 20/23] em28xx: improve USB endpoint logic, also use bulk transfers Frank Schäfer
2012-10-21 16:52 ` [PATCH 21/23] em28xx: add module parameter for selection of the preferred USB transfer type Frank Schäfer
2012-10-21 16:52 ` [PATCH 22/23] em28xx: use common urb data copying function for vbi and non-vbi devices Frank Schäfer
2012-10-21 16:52 ` [PATCH 23/23] em28xx: enable VBI-support for em2840 devices Frank Schäfer
2012-10-21 18:13 ` [PATCH 00/23] em28xx: add support fur USB bulk transfers Devin Heitmueller
2012-10-23 19:02 ` Frank Schäfer [this message]
[not found] ` <20121028175752.447c39d5@redhat.com>
2012-10-29 15:33 ` Frank Schäfer
2012-10-29 20:03 ` Mauro Carvalho Chehab
2012-10-29 21:14 ` Frank Schäfer
2012-10-30 3:00 ` Mauro Carvalho Chehab
2012-10-30 4:06 ` Mauro Carvalho Chehab
2012-10-30 13:08 ` Devin Heitmueller
2012-10-30 14:03 ` Mauro Carvalho Chehab
2012-10-30 17:29 ` Frank Schäfer
2012-10-30 17:18 ` Frank Schäfer
2012-11-08 18:03 ` Frank Schäfer
2012-11-09 15:02 ` Mauro Carvalho Chehab
2012-11-09 16:03 ` Frank Schäfer
2012-10-31 1:39 ` Benny Amorsen
2012-10-31 10:57 ` Ezequiel Garcia
2012-10-31 12:03 ` Frank Schäfer
2012-10-31 20:24 ` Benny Amorsen
2012-10-31 20:22 ` Benny Amorsen
2012-10-31 11:50 ` Frank Schäfer
2012-10-31 19:58 ` Benny Amorsen
2012-10-31 20:25 ` Ezequiel Garcia
2012-10-31 22:26 ` Benny Amorsen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5086E9C0.5060701@googlemail.com \
--to=fschaefer.oss@googlemail.com \
--cc=dheitmueller@kernellabs.com \
--cc=linux-media@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).