* em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) @ 2011-12-24 21:58 Dennis Sperlich 2011-12-25 10:52 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 15+ messages in thread From: Dennis Sperlich @ 2011-12-24 21:58 UTC (permalink / raw) To: linux-media Hi, I have a Terratec Cinergy HTC Stick an tried the new support for the DVB-C part. It works for SD material (at least for free receivable stations, I tried afair only QAM64), but did not for HD stations (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD (via KabelDeutschland). The HD material was just digital artefacts, as far as mplayer could decode it. When I did a dumpstream and looked at the resulting file size I got something about 1MB/s which seems a little too low, because SD was already about 870kB/s. After looking around I found a solution in increasing the isoc_dvb_max_packetsize from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s and looked good. I'm not sure, whether this is the correct fix, but it works for me. If you need more testing pleas tell. Regards, Dennis index 804a4ab..c518d13 100644 --- a/drivers/media/video/em28xx/em28xx-core.c +++ b/drivers/media/video/em28xx/em28xx-core.c @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T * but not enough for 44 Mbit DVB-C. */ - packet_size = 752; + packet_size = 940; } return packet_size; ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-24 21:58 em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) Dennis Sperlich @ 2011-12-25 10:52 ` Mauro Carvalho Chehab 2011-12-25 14:04 ` Dennis Sperlich 0 siblings, 1 reply; 15+ messages in thread From: Mauro Carvalho Chehab @ 2011-12-25 10:52 UTC (permalink / raw) To: Dennis Sperlich; +Cc: linux-media, Michael Krufky, Devin Heitmueller On 24-12-2011 19:58, Dennis Sperlich wrote: > Hi, > > I have a Terratec Cinergy HTC Stick an tried the new support for the DVB-C part. It works for SD material (at least for free receivable stations, I tried afair only QAM64), but did not for HD stations (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD (via KabelDeutschland). The HD material was just digital artefacts, as far as mplayer could decode it. When I did a dumpstream and looked at the resulting file size I got something about 1MB/s which seems a little too low, because SD was already about 870kB/s. After looking around I found a solution in increasing the isoc_dvb_max_packetsize from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s and looked good. I'm not sure, whether this is the correct fix, but it works for me. > > If you need more testing pleas tell. > > Regards, > Dennis > > > > index 804a4ab..c518d13 100644 > --- a/drivers/media/video/em28xx/em28xx-core.c > +++ b/drivers/media/video/em28xx/em28xx-core.c > @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) > * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T > * but not enough for 44 Mbit DVB-C. > */ > - packet_size = 752; > + packet_size = 940; > } > > return packet_size; As you can see there at the code, the packet size depends on the chipset, as not all will support 940 for packet size. Could you please provide us what was the chip detected id? It should be a message on your dmesg like: chip ID is em2870 or em28xx chip ID = 38 The patch should change it only for your specific chipset model, in order to avoid regressions to other supported chipsets, like: int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) { unsigned int chip_cfg2; unsigned int packet_size; switch (dev->chip_id) { case CHIP_ID_EM2710: case CHIP_ID_EM2750: case CHIP_ID_EM2800: case CHIP_ID_EM2820: ... case CHIP_ID_foo: packet_size = 940; ... } return packet_size; } The case you're touching seems to be for em2884, but the switch covers both em2884 and em28174 (plus the default for newer chipsets). Maybe Michael/Devin may have something to say, with regards to a way to detect the maximum supported packet size by each specific em28xx model. Regards, Mauro ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 10:52 ` Mauro Carvalho Chehab @ 2011-12-25 14:04 ` Dennis Sperlich 2011-12-25 14:11 ` Hans Petter Selasky 2011-12-25 18:13 ` Mauro Carvalho Chehab 0 siblings, 2 replies; 15+ messages in thread From: Dennis Sperlich @ 2011-12-25 14:04 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Michael Krufky, Devin Heitmueller On 25.12.2011 11:52, Mauro Carvalho Chehab wrote: > On 24-12-2011 19:58, Dennis Sperlich wrote: >> Hi, >> >> I have a Terratec Cinergy HTC Stick an tried the new support for the DVB-C part. It works for SD material (at least for free receivable stations, I tried afair only QAM64), but did not for HD stations (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD (via KabelDeutschland). The HD material was just digital artefacts, as far as mplayer could decode it. When I did a dumpstream and looked at the resulting file size I got something about 1MB/s which seems a little too low, because SD was already about 870kB/s. After looking around I found a solution in increasing the isoc_dvb_max_packetsize from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s and looked good. I'm not sure, whether this is the correct fix, but it works for me. >> >> If you need more testing pleas tell. >> >> Regards, >> Dennis >> >> >> >> index 804a4ab..c518d13 100644 >> --- a/drivers/media/video/em28xx/em28xx-core.c >> +++ b/drivers/media/video/em28xx/em28xx-core.c >> @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) >> * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T >> * but not enough for 44 Mbit DVB-C. >> */ >> - packet_size = 752; >> + packet_size = 940; >> } >> >> return packet_size; > As you can see there at the code, the packet size depends on the chipset, as > not all will support 940 for packet size. > > Could you please provide us what was the chip detected id? > > It should be a message on your dmesg like: > > chip ID is em2870 > or > em28xx chip ID = 38 > > The patch should change it only for your specific chipset model, in order to > avoid regressions to other supported chipsets, like: > > int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) > { > unsigned int chip_cfg2; > unsigned int packet_size; > > switch (dev->chip_id) { > case CHIP_ID_EM2710: > case CHIP_ID_EM2750: > case CHIP_ID_EM2800: > case CHIP_ID_EM2820: > ... > case CHIP_ID_foo: > packet_size = 940; > ... > } > > return packet_size; > } > > The case you're touching seems to be for em2884, but the switch covers both > em2884 and em28174 (plus the default for newer chipsets). dmesg says: em28xx #0: chip ID is em2884 so a patch would be more like: index 804a4ab..9280251 100644 --- a/drivers/media/video/em28xx/em28xx-core.c +++ b/drivers/media/video/em28xx/em28xx-core.c @@ -1151,6 +1151,8 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) packet_size = 564; break; case CHIP_ID_EM2884: + packet_size = 940; + break; case CHIP_ID_EM28174: default: /* > Maybe Michael/Devin may have something to say, with regards to a way to detect > the maximum supported packet size by each specific em28xx model. This would be fine, I tried also 1128 (6 times 188), but then mplayer did not play any more. I then tried 1034, but then I got the message " submit of urb 0 failed (error=-90)", so I guess there have to be integer multiples of 188. Regards, Dennis ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 14:04 ` Dennis Sperlich @ 2011-12-25 14:11 ` Hans Petter Selasky 2011-12-25 14:47 ` Devin Heitmueller ` (2 more replies) 2011-12-25 18:13 ` Mauro Carvalho Chehab 1 sibling, 3 replies; 15+ messages in thread From: Hans Petter Selasky @ 2011-12-25 14:11 UTC (permalink / raw) To: Dennis Sperlich Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Devin Heitmueller On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote: > On 25.12.2011 11:52, Mauro Carvalho Chehab wrote: > > On 24-12-2011 19:58, Dennis Sperlich wrote: > >> Hi, > >> > >> I have a Terratec Cinergy HTC Stick an tried the new support for the > >> DVB-C part. It works for SD material (at least for free receivable > >> stations, I tried afair only QAM64), but did not for HD stations > >> (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD > >> (via KabelDeutschland). The HD material was just digital artefacts, as > >> far as mplayer could decode it. When I did a dumpstream and looked at > >> the resulting file size I got something about 1MB/s which seems a > >> little too low, because SD was already about 870kB/s. After looking > >> around I found a solution in increasing the isoc_dvb_max_packetsize > >> from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s > >> and looked good. I'm not sure, whether this is the correct fix, but it > >> works for me. > >> > >> If you need more testing pleas tell. > >> > >> Regards, > >> Dennis > >> These numbers should not be hardcoded, but extracted from the USB endpoint descriptor! --HPS ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 14:11 ` Hans Petter Selasky @ 2011-12-25 14:47 ` Devin Heitmueller 2011-12-25 17:58 ` Mauro Carvalho Chehab 2011-12-25 19:42 ` Malcolm Priestley 2 siblings, 0 replies; 15+ messages in thread From: Devin Heitmueller @ 2011-12-25 14:47 UTC (permalink / raw) To: Hans Petter Selasky Cc: Dennis Sperlich, Mauro Carvalho Chehab, linux-media, Michael Krufky On Sun, Dec 25, 2011 at 9:11 AM, Hans Petter Selasky <hselasky@c2i.net> wrote: > These numbers should not be hardcoded, but extracted from the USB endpoint > descriptor! > > --HPS Hans is correct. I only hard-coded it at 564 as a quick hack when I was bootstrapping the em2784 support. The code really should be cleaned up to base it off of the endpoint descriptors. Patches certainly welcome as this is indeed a known issue. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 14:11 ` Hans Petter Selasky 2011-12-25 14:47 ` Devin Heitmueller @ 2011-12-25 17:58 ` Mauro Carvalho Chehab 2011-12-25 19:42 ` Malcolm Priestley 2 siblings, 0 replies; 15+ messages in thread From: Mauro Carvalho Chehab @ 2011-12-25 17:58 UTC (permalink / raw) To: Hans Petter Selasky Cc: Dennis Sperlich, linux-media, Michael Krufky, Devin Heitmueller On 25-12-2011 12:11, Hans Petter Selasky wrote: > On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote: >> On 25.12.2011 11:52, Mauro Carvalho Chehab wrote: >>> On 24-12-2011 19:58, Dennis Sperlich wrote: >>>> Hi, >>>> >>>> I have a Terratec Cinergy HTC Stick an tried the new support for the >>>> DVB-C part. It works for SD material (at least for free receivable >>>> stations, I tried afair only QAM64), but did not for HD stations >>>> (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD >>>> (via KabelDeutschland). The HD material was just digital artefacts, as >>>> far as mplayer could decode it. When I did a dumpstream and looked at >>>> the resulting file size I got something about 1MB/s which seems a >>>> little too low, because SD was already about 870kB/s. After looking >>>> around I found a solution in increasing the isoc_dvb_max_packetsize >>>> from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s >>>> and looked good. I'm not sure, whether this is the correct fix, but it >>>> works for me. >>>> >>>> If you need more testing pleas tell. >>>> >>>> Regards, >>>> Dennis >>>> > > These numbers should not be hardcoded, but extracted from the USB endpoint > descriptor! The driver retrieves the values from the USB endpoints during probe. This is used for analog mode. However, on DVB mode, the values are hardcoded by the ones that had/have access to Empiatech datasheets (Unfortunately, I don't have them). So, it is hard to know if the current limit is due to some chipset bug that doesn't report well the maximum packet size, or if it is just due to the lack of time for the ones that wrote the code to actually use the reported values. Fixing the driver to use the USB descriptors is easy. The hard part is to be sure that it won't break for the ones with older chipsets. Regards, Mauro. > > --HPS ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 14:11 ` Hans Petter Selasky 2011-12-25 14:47 ` Devin Heitmueller 2011-12-25 17:58 ` Mauro Carvalho Chehab @ 2011-12-25 19:42 ` Malcolm Priestley 2011-12-25 20:12 ` Dennis Sperlich 2 siblings, 1 reply; 15+ messages in thread From: Malcolm Priestley @ 2011-12-25 19:42 UTC (permalink / raw) To: Hans Petter Selasky Cc: Dennis Sperlich, Mauro Carvalho Chehab, linux-media, Michael Krufky, Devin Heitmueller On Sun, 2011-12-25 at 15:11 +0100, Hans Petter Selasky wrote: > On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote: > > On 25.12.2011 11:52, Mauro Carvalho Chehab wrote: > > > On 24-12-2011 19:58, Dennis Sperlich wrote: > > >> Hi, > > >> > > >> I have a Terratec Cinergy HTC Stick an tried the new support for the > > >> DVB-C part. It works for SD material (at least for free receivable > > >> stations, I tried afair only QAM64), but did not for HD stations > > >> (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD > > >> (via KabelDeutschland). The HD material was just digital artefacts, as > > >> far as mplayer could decode it. When I did a dumpstream and looked at > > >> the resulting file size I got something about 1MB/s which seems a > > >> little too low, because SD was already about 870kB/s. After looking > > >> around I found a solution in increasing the isoc_dvb_max_packetsize > > >> from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s > > >> and looked good. I'm not sure, whether this is the correct fix, but it > > >> works for me. > > >> Would not increasing EM28XX_DVB_NUM_BUFS currently set at 5 to say 10 have a better effect? Malcolm ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 19:42 ` Malcolm Priestley @ 2011-12-25 20:12 ` Dennis Sperlich 0 siblings, 0 replies; 15+ messages in thread From: Dennis Sperlich @ 2011-12-25 20:12 UTC (permalink / raw) To: Malcolm Priestley Cc: Hans Petter Selasky, Mauro Carvalho Chehab, linux-media, Michael Krufky, Devin Heitmueller On 25.12.2011 20:42, Malcolm Priestley wrote: > On Sun, 2011-12-25 at 15:11 +0100, Hans Petter Selasky wrote: >> On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote: >>> On 25.12.2011 11:52, Mauro Carvalho Chehab wrote: >>>> On 24-12-2011 19:58, Dennis Sperlich wrote: >>>>> Hi, >>>>> >>>>> I have a Terratec Cinergy HTC Stick an tried the new support for the >>>>> DVB-C part. It works for SD material (at least for free receivable >>>>> stations, I tried afair only QAM64), but did not for HD stations >>>>> (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD >>>>> (via KabelDeutschland). The HD material was just digital artefacts, as >>>>> far as mplayer could decode it. When I did a dumpstream and looked at >>>>> the resulting file size I got something about 1MB/s which seems a >>>>> little too low, because SD was already about 870kB/s. After looking >>>>> around I found a solution in increasing the isoc_dvb_max_packetsize >>>>> from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s >>>>> and looked good. I'm not sure, whether this is the correct fix, but it >>>>> works for me. >>>>> > Would not increasing EM28XX_DVB_NUM_BUFS currently set at 5 to say 10 > have a better effect? This does not work, I just tried it. Regards, Dennis ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 14:04 ` Dennis Sperlich 2011-12-25 14:11 ` Hans Petter Selasky @ 2011-12-25 18:13 ` Mauro Carvalho Chehab 2011-12-25 20:33 ` Dennis Sperlich 1 sibling, 1 reply; 15+ messages in thread From: Mauro Carvalho Chehab @ 2011-12-25 18:13 UTC (permalink / raw) To: Dennis Sperlich; +Cc: linux-media, Michael Krufky, Devin Heitmueller On 25-12-2011 12:04, Dennis Sperlich wrote: > On 25.12.2011 11:52, Mauro Carvalho Chehab wrote: >> On 24-12-2011 19:58, Dennis Sperlich wrote: >>> Hi, >>> >>> I have a Terratec Cinergy HTC Stick an tried the new support for the DVB-C part. It works for SD material (at least for free receivable stations, I tried afair only QAM64), but did not for HD stations (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD (via KabelDeutschland). The HD material was just digital artefacts, as far as mplayer could decode it. When I did a dumpstream and looked at the resulting file size I got something about 1MB/s which seems a little too low, because SD was already about 870kB/s. After looking around I found a solution in increasing the isoc_dvb_max_packetsize from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s and looked good. I'm not sure, whether this is the correct fix, but it works for me. >>> >>> If you need more testing pleas tell. >>> >>> Regards, >>> Dennis >>> >>> >>> >>> index 804a4ab..c518d13 100644 >>> --- a/drivers/media/video/em28xx/em28xx-core.c >>> +++ b/drivers/media/video/em28xx/em28xx-core.c >>> @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) >>> * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T >>> * but not enough for 44 Mbit DVB-C. >>> */ >>> - packet_size = 752; >>> + packet_size = 940; >>> } >>> >>> return packet_size; >> As you can see there at the code, the packet size depends on the chipset, as >> not all will support 940 for packet size. >> >> Could you please provide us what was the chip detected id? >> >> It should be a message on your dmesg like: >> >> chip ID is em2870 >> or >> em28xx chip ID = 38 >> >> The patch should change it only for your specific chipset model, in order to >> avoid regressions to other supported chipsets, like: >> >> int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) >> { >> unsigned int chip_cfg2; >> unsigned int packet_size; >> >> switch (dev->chip_id) { >> case CHIP_ID_EM2710: >> case CHIP_ID_EM2750: >> case CHIP_ID_EM2800: >> case CHIP_ID_EM2820: >> ... >> case CHIP_ID_foo: >> packet_size = 940; >> ... >> } >> >> return packet_size; >> } >> >> The case you're touching seems to be for em2884, but the switch covers both >> em2884 and em28174 (plus the default for newer chipsets). > > dmesg says: em28xx #0: chip ID is em2884 > > so a patch would be more like: > > index 804a4ab..9280251 100644 > --- a/drivers/media/video/em28xx/em28xx-core.c > +++ b/drivers/media/video/em28xx/em28xx-core.c > @@ -1151,6 +1151,8 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) > packet_size = 564; > break; > case CHIP_ID_EM2884: > + packet_size = 940; > + break; > case CHIP_ID_EM28174: > default: > /* > >> Maybe Michael/Devin may have something to say, with regards to a way to detect >> the maximum supported packet size by each specific em28xx model. > > This would be fine, I tried also 1128 (6 times 188), but then mplayer did not play any more. > I then tried 1034, but then I got the message " submit of urb 0 failed (error=-90)", so I guess > there have to be integer multiples of 188. The maximum packet size is described at the USB descriptors. The code at em28xx_set_alternate() seeks for the alternates and selects the one that will get the best alternate for the analog mode. There, you'll see a code that gets the best alternate for analog, and selects the proper packet size for it. For DVB, it should be a little different, as, on DVB, the driver doesn't know, in advance, what's the streaming rate. So, the driver needs to get a high enough packet size to fit for the bandwidth needs. Also, for DVB, the driver does: usb_set_interface(dev->udev, 0, 1); (e. g. it always use alternate 1) So, in thesis, that function could be as simple as: dev->alt_max_pkt_size[1]; (if the max packet size for alternate 1 would work for all chips) Or, eventually, the code might be using other alternates for HD, but I'm not sure if those chipsets support a different alternate for DVB. Regards, Mauro ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 18:13 ` Mauro Carvalho Chehab @ 2011-12-25 20:33 ` Dennis Sperlich 2011-12-26 5:55 ` Holger Nelson 0 siblings, 1 reply; 15+ messages in thread From: Dennis Sperlich @ 2011-12-25 20:33 UTC (permalink / raw) To: Mauro Carvalho Chehab; +Cc: linux-media, Michael Krufky, Devin Heitmueller [-- Attachment #1: Type: text/plain, Size: 5056 bytes --] On 25.12.2011 19:13, Mauro Carvalho Chehab wrote: > On 25-12-2011 12:04, Dennis Sperlich wrote: >> On 25.12.2011 11:52, Mauro Carvalho Chehab wrote: >>> On 24-12-2011 19:58, Dennis Sperlich wrote: >>>> Hi, >>>> >>>> I have a Terratec Cinergy HTC Stick an tried the new support for the DVB-C part. It works for SD material (at least for free receivable stations, I tried afair only QAM64), but did not for HD stations (QAM256). I have only access to unencrypted ARD HD, ZDF HD and arte HD (via KabelDeutschland). The HD material was just digital artefacts, as far as mplayer could decode it. When I did a dumpstream and looked at the resulting file size I got something about 1MB/s which seems a little too low, because SD was already about 870kB/s. After looking around I found a solution in increasing the isoc_dvb_max_packetsize from 752 to 940 (multiple of 188). Then an HD stream was about 1.4MB/s and looked good. I'm not sure, whether this is the correct fix, but it works for me. >>>> >>>> If you need more testing pleas tell. >>>> >>>> Regards, >>>> Dennis >>>> >>>> >>>> >>>> index 804a4ab..c518d13 100644 >>>> --- a/drivers/media/video/em28xx/em28xx-core.c >>>> +++ b/drivers/media/video/em28xx/em28xx-core.c >>>> @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) >>>> * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T >>>> * but not enough for 44 Mbit DVB-C. >>>> */ >>>> - packet_size = 752; >>>> + packet_size = 940; >>>> } >>>> >>>> return packet_size; >>> As you can see there at the code, the packet size depends on the chipset, as >>> not all will support 940 for packet size. >>> >>> Could you please provide us what was the chip detected id? >>> >>> It should be a message on your dmesg like: >>> >>> chip ID is em2870 >>> or >>> em28xx chip ID = 38 >>> >>> The patch should change it only for your specific chipset model, in order to >>> avoid regressions to other supported chipsets, like: >>> >>> int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) >>> { >>> unsigned int chip_cfg2; >>> unsigned int packet_size; >>> >>> switch (dev->chip_id) { >>> case CHIP_ID_EM2710: >>> case CHIP_ID_EM2750: >>> case CHIP_ID_EM2800: >>> case CHIP_ID_EM2820: >>> ... >>> case CHIP_ID_foo: >>> packet_size = 940; >>> ... >>> } >>> >>> return packet_size; >>> } >>> >>> The case you're touching seems to be for em2884, but the switch covers both >>> em2884 and em28174 (plus the default for newer chipsets). >> dmesg says: em28xx #0: chip ID is em2884 >> >> so a patch would be more like: >> >> index 804a4ab..9280251 100644 >> --- a/drivers/media/video/em28xx/em28xx-core.c >> +++ b/drivers/media/video/em28xx/em28xx-core.c >> @@ -1151,6 +1151,8 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) >> packet_size = 564; >> break; >> case CHIP_ID_EM2884: >> + packet_size = 940; >> + break; >> case CHIP_ID_EM28174: >> default: >> /* >> >>> Maybe Michael/Devin may have something to say, with regards to a way to detect >>> the maximum supported packet size by each specific em28xx model. >> This would be fine, I tried also 1128 (6 times 188), but then mplayer did not play any more. >> I then tried 1034, but then I got the message " submit of urb 0 failed (error=-90)", so I guess >> there have to be integer multiples of 188. > The maximum packet size is described at the USB descriptors. > > The code at em28xx_set_alternate() seeks for the alternates and selects > the one that will get the best alternate for the analog mode. > > There, you'll see a code that gets the best alternate for analog, > and selects the proper packet size for it. > > For DVB, it should be a little different, as, on DVB, the driver doesn't > know, in advance, what's the streaming rate. So, the driver needs to get > a high enough packet size to fit for the bandwidth needs. > > Also, for DVB, the driver does: > usb_set_interface(dev->udev, 0, 1); > > (e. g. it always use alternate 1) > > So, in thesis, that function could be as simple as: > dev->alt_max_pkt_size[1]; > > (if the max packet size for alternate 1 would work for all chips) > > Or, eventually, the code might be using other alternates for HD, > but I'm not sure if those chipsets support a different alternate for DVB. I just tried, replacing max_dvb_packet_size = em28xx_isoc_dvb_max_packetsize(dev); by max_dvb_packet_size = dev->alt_max_pkt_size[1]; but it did not work. Was this the correct replacement? printk(KERN_INFO "dev->alt_max_pkt_size[1] is %i\n",dev->alt_max_pkt_size[1]); then said, dev->alt_max_pkt_size[1] is 0. I also attachted a lsusb -v output for the Terratec Cinergy HTC Stick. I don't know, which of these endpoints the dvb-c part is, but it may be anyway usefull. Regard, Dennis [-- Attachment #2: lsusb --] [-- Type: text/plain, Size: 16063 bytes --] Bus 001 Device 009: ID 0ccd:00b2 TerraTec Electronic GmbH Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor 0x0ccd TerraTec Electronic GmbH idProduct 0x00b2 bcdDevice 1.00 iManufacturer 2 TERRATEC iProduct 1 Cinergy_HTC_Stick iSerial 3 02?TERRATE bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 305 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 1 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0000 1x 0 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 2 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0ad0 2x 720 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 3 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x0c00 2x 1024 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 4 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1300 3x 768 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 5 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1380 3x 896 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 6 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x13c0 3x 960 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 7 bNumEndpoints 4 bInterfaceClass 255 Vendor Specific Class bInterfaceSubClass 0 bInterfaceProtocol 255 iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0001 1x 1 bytes bInterval 11 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x1400 3x 1024 bytes bInterval 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x00c4 1x 196 bytes bInterval 4 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 1 Transfer Type Isochronous Synch Type None Usage Type Data wMaxPacketSize 0x03ac 1x 940 bytes bInterval 1 Device Qualifier (for other device speed): bLength 10 bDescriptorType 6 bcdUSB 2.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 bNumConfigurations 1 Device Status: 0x0000 (Bus Powered) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-25 20:33 ` Dennis Sperlich @ 2011-12-26 5:55 ` Holger Nelson 2011-12-26 12:18 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 15+ messages in thread From: Holger Nelson @ 2011-12-26 5:55 UTC (permalink / raw) To: Dennis Sperlich Cc: Mauro Carvalho Chehab, linux-media, Michael Krufky, Devin Heitmueller Hi! On Sun, 25 Dec 2011, Dennis Sperlich wrote: > I just tried, replacing > max_dvb_packet_size = em28xx_isoc_dvb_max_packetsize(dev); > by > max_dvb_packet_size = dev->alt_max_pkt_size[1]; > > but it did not work. Was this the correct replacement? > > printk(KERN_INFO "dev->alt_max_pkt_size[1] is > %i\n",dev->alt_max_pkt_size[1]); > > then said, dev->alt_max_pkt_size[1] is 0. > > I also attachted a lsusb -v output for the Terratec Cinergy HTC Stick. I > don't know, which of these endpoints the dvb-c part is, but it may be anyway > usefull. Is it possible, that dev->alt_max_pkt_size gets the maximum packet sizes for the analog video input endpoint (0x82 in lsusb output)? The patch below worked during a small test, but I doubt that it is the best way to do it. While still looking at it: Is there a reason to allocate 32 bytes per alternate interface configuration if we only store one unsigned int per configuration in it? - I just copied the allocation code from above. Holger diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c index 1704da0..70866c5 100644 --- a/drivers/media/video/em28xx/em28xx-cards.c +++ b/drivers/media/video/em28xx/em28xx-cards.c @@ -3269,6 +3273,30 @@ static int em28xx_usb_probe(struct usb_interface *interface, dev->alt_max_pkt_size[i] = size; } + dev->alt_dvb_max_pkt_size = kmalloc(32 * dev->num_alt, GFP_KERNEL); + + if (dev->alt_dvb_max_pkt_size == NULL) { + em28xx_errdev("out of memory!\n"); + kfree(dev); + retval = -ENOMEM; + goto err; + } + + for (i = 0; i < dev->num_alt ; i++) { + int ep; + for (ep = 0; ep < interface->altsetting[i].desc.bNumEndpoints; ep++) { + struct usb_host_endpoint *e = &interface->altsetting[i].endpoint[ep]; + if (e->desc.bEndpointAddress == 0x84) { + u16 tmp = le16_to_cpu(e->desc.wMaxPacketSize); + unsigned int size = tmp & 0x7ff; + if (udev->speed == USB_SPEED_HIGH) + size = size * hb_mult(tmp); + + dev->alt_dvb_max_pkt_size[i] = size; + } + } + } + if ((card[nr] >= 0) && (card[nr] < em28xx_bcount)) dev->model = card[nr]; @@ -3281,6 +3309,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, retval = em28xx_init_dev(&dev, udev, interface, nr); if (retval) { mutex_unlock(&dev->lock); + kfree(dev->alt_dvb_max_pkt_size); kfree(dev->alt_max_pkt_size); kfree(dev); goto err; @@ -3365,6 +3394,7 @@ static void em28xx_usb_disconnect(struct usb_interface *interface) em28xx_close_extension(dev); if (!dev->users) { + kfree(dev->alt_dvb_max_pkt_size); kfree(dev->alt_max_pkt_size); kfree(dev); } diff --git a/drivers/media/video/em28xx/em28xx-core.c b/drivers/media/video/em28xx/em28xx-core.c index 804a4ab..e7d3541 100644 --- a/drivers/media/video/em28xx/em28xx-core.c +++ b/drivers/media/video/em28xx/em28xx-core.c @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T * but not enough for 44 Mbit DVB-C. */ - packet_size = 752; + packet_size = dev->alt_dvb_max_pkt_size[1]; } return packet_size; diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c index 9b4557a..2491a2c 100644 --- a/drivers/media/video/em28xx/em28xx-video.c +++ b/drivers/media/video/em28xx/em28xx-video.c @@ -2254,6 +2254,7 @@ static int em28xx_v4l2_close(struct file *filp) free the remaining resources */ if (dev->state & DEV_DISCONNECTED) { em28xx_release_resources(dev); + kfree(dev->alt_dvb_max_pkt_size); kfree(dev->alt_max_pkt_size); kfree(dev); return 0; diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h index b1199ef..793c85a 100644 --- a/drivers/media/video/em28xx/em28xx.h +++ b/drivers/media/video/em28xx/em28xx.h @@ -597,6 +597,7 @@ struct em28xx { int max_pkt_size; /* max packet size of isoc transaction */ int num_alt; /* Number of alternative settings */ unsigned int *alt_max_pkt_size; /* array of wMaxPacketSize */ + unsigned int *alt_dvb_max_pkt_size; struct urb *urb[EM28XX_NUM_BUFS]; /* urb for isoc transfers */ char *transfer_buffer[EM28XX_NUM_BUFS]; /* transfer buffers for isoc transfer */ ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-26 5:55 ` Holger Nelson @ 2011-12-26 12:18 ` Mauro Carvalho Chehab 2011-12-28 3:50 ` Holger Nelson 0 siblings, 1 reply; 15+ messages in thread From: Mauro Carvalho Chehab @ 2011-12-26 12:18 UTC (permalink / raw) To: Holger Nelson Cc: Dennis Sperlich, linux-media, Michael Krufky, Devin Heitmueller On 26-12-2011 03:55, Holger Nelson wrote: > Hi! > > On Sun, 25 Dec 2011, Dennis Sperlich wrote: > >> I just tried, replacing >> max_dvb_packet_size = em28xx_isoc_dvb_max_packetsize(dev); >> by >> max_dvb_packet_size = dev->alt_max_pkt_size[1]; >> >> but it did not work. Was this the correct replacement? >> >> printk(KERN_INFO "dev->alt_max_pkt_size[1] is %i\n",dev->alt_max_pkt_size[1]); >> >> then said, dev->alt_max_pkt_size[1] is 0. >> >> I also attachted a lsusb -v output for the Terratec Cinergy HTC Stick. I don't know, which of these endpoints the dvb-c part is, but it may be anyway usefull. > > Is it possible, that dev->alt_max_pkt_size gets the maximum packet sizes for the analog video input endpoint (0x82 in lsusb output)? The patch below worked during a small test, but I doubt that it is the best way to do it. > > While still looking at it: Is there a reason to allocate 32 bytes per alternate interface configuration if we only store one unsigned int per configuration in it? - I just copied the allocation code from above. > > Holger > > diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c > index 1704da0..70866c5 100644 > --- a/drivers/media/video/em28xx/em28xx-cards.c > +++ b/drivers/media/video/em28xx/em28xx-cards.c > @@ -3269,6 +3273,30 @@ static int em28xx_usb_probe(struct usb_interface *interface, > dev->alt_max_pkt_size[i] = size; > } > > + dev->alt_dvb_max_pkt_size = kmalloc(32 * dev->num_alt, GFP_KERNEL); It is likely over-allocating memory. The proper way would be to replace 32 by sizeof(dev->alt_dvb_max_pkt_size[0]). > + > + if (dev->alt_dvb_max_pkt_size == NULL) { > + em28xx_errdev("out of memory!\n"); > + kfree(dev); > + retval = -ENOMEM; > + goto err; > + } > + > + for (i = 0; i < dev->num_alt ; i++) { > + int ep; > + for (ep = 0; ep < interface->altsetting[i].desc.bNumEndpoints; ep++) { > + struct usb_host_endpoint *e = &interface->altsetting[i].endpoint[ep]; > + if (e->desc.bEndpointAddress == 0x84) { Yeah, this looks correct. Yet, I would avoid create another loop here. Also, there are too many hacks at the em28xx probe utility. Take a look at the tm6000-cards.c. It used to be similar to em28xx probe, but the code now looks better. IMO, we should add, at em28xx-regs: #define EM28XX_EP_ANALOG 0x82 #define EM28XX_EP_AUDIO 0x83 #define EM28XX_EP_DIGITAL 0x84 and use those macros a > + u16 tmp = le16_to_cpu(e->desc.wMaxPacketSize); > + unsigned int size = tmp & 0x7ff; > + if (udev->speed == USB_SPEED_HIGH) > + size = size * hb_mult(tmp); > + > + dev->alt_dvb_max_pkt_size[i] = size; > + } > + } > + } > + > if ((card[nr] >= 0) && (card[nr] < em28xx_bcount)) > dev->model = card[nr]; > > @@ -3281,6 +3309,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, > retval = em28xx_init_dev(&dev, udev, interface, nr); > if (retval) { > mutex_unlock(&dev->lock); > + kfree(dev->alt_dvb_max_pkt_size); > kfree(dev->alt_max_pkt_size); > kfree(dev); > goto err; > @@ -3365,6 +3394,7 @@ static void em28xx_usb_disconnect(struct usb_interface *interface) > em28xx_close_extension(dev); > > if (!dev->users) { > + kfree(dev->alt_dvb_max_pkt_size); > kfree(dev->alt_max_pkt_size); > kfree(dev); > } > diff --git a/drivers/media/video/em28xx/em28xx-core.c b/drivers/media/video/em28xx/em28xx-core.c > index 804a4ab..e7d3541 100644 > --- a/drivers/media/video/em28xx/em28xx-core.c > +++ b/drivers/media/video/em28xx/em28xx-core.c > @@ -1157,7 +1157,7 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) > * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T > * but not enough for 44 Mbit DVB-C. > */ > - packet_size = 752; > + packet_size = dev->alt_dvb_max_pkt_size[1]; > } > > return packet_size; > diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c > index 9b4557a..2491a2c 100644 > --- a/drivers/media/video/em28xx/em28xx-video.c > +++ b/drivers/media/video/em28xx/em28xx-video.c > @@ -2254,6 +2254,7 @@ static int em28xx_v4l2_close(struct file *filp) > free the remaining resources */ > if (dev->state & DEV_DISCONNECTED) { > em28xx_release_resources(dev); > + kfree(dev->alt_dvb_max_pkt_size); > kfree(dev->alt_max_pkt_size); > kfree(dev); > return 0; > diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h > index b1199ef..793c85a 100644 > --- a/drivers/media/video/em28xx/em28xx.h > +++ b/drivers/media/video/em28xx/em28xx.h > @@ -597,6 +597,7 @@ struct em28xx { > int max_pkt_size; /* max packet size of isoc transaction */ > int num_alt; /* Number of alternative settings */ > unsigned int *alt_max_pkt_size; /* array of wMaxPacketSize */ > + unsigned int *alt_dvb_max_pkt_size; > struct urb *urb[EM28XX_NUM_BUFS]; /* urb for isoc transfers */ > char *transfer_buffer[EM28XX_NUM_BUFS]; /* transfer buffers for isoc > transfer */ The above code looks promising. I'm currently without time right now to work on a patch, but I think that several hacks inside the em28xx probe should be removed, including the one that detects the endpoint based on the packet size. As it is easier to code than to explain in words, the code below could be a start (ok, it doesn't compile, doesn't remove all hacks, doesn't free memory, etc...) Feel free to use it as a start for a real patch, if you wish. diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c index 1704da0..1bd4ef5 100644 --- a/drivers/media/video/em28xx/em28xx-cards.c +++ b/drivers/media/video/em28xx/em28xx-cards.c @@ -3087,8 +3087,8 @@ static int em28xx_usb_probe(struct usb_interface *interface, struct usb_device *udev; struct em28xx *dev = NULL; int retval; - bool is_audio_only = false, has_audio = false; - int i, nr, isoc_pipe; + bool has_audio = false, has_video = false, has_dvb = false; + int i, nr, sizedescr, size; const int ifnum = interface->altsetting[0].desc.bInterfaceNumber; char *speed; char descr[255] = ""; @@ -3120,6 +3120,32 @@ static int em28xx_usb_probe(struct usb_interface *interface, goto err; } + /* allocate memory for our device state and initialize it */ + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (dev == NULL) { + em28xx_err(DRIVER_NAME ": out of memory!\n"); + retval = -ENOMEM; + goto err; + } + + /* compute alternate max packet sizes */ + dev->alt_max_pkt_size = kmalloc(sizeof(dev->alt_max_pkt_size[0] * interface->num_altsetting, GFP_KERNEL); + if (dev->alt_max_pkt_size == NULL) { + em28xx_errdev("out of memory!\n"); + kfree(dev); + retval = -ENOMEM; + goto err; + } + + dev->alt_dvb_pkt_size = kmalloc(sizeof(dev->alt_max_pkt_size[0] * interface->num_altsetting, GFP_KERNEL); + if (dev->alt_dvb_pkt_size == NULL) { + em28xx_errdev("out of memory!\n"); + kfree(); + kfree(dev); + retval = -ENOMEM; + goto err; + } + /* Get endpoints */ for (i = 0; i < interface->num_altsetting; i++) { int ep; @@ -3128,8 +3154,23 @@ static int em28xx_usb_probe(struct usb_interface *interface, struct usb_host_endpoint *e; e = &interface->altsetting[i].endpoint[ep]; - if (e->desc.bEndpointAddress == 0x83) + sizedescr = le16_to_cpu(interface->altsetting[i].endpoint[ep].desc.wMaxPacketSize); + size = sizedescr & 0x7fff; + if (udev->speed == USB_SPEED_HIGH) + size = size * hb_mult(sizedescr); + + switch (e->desc.bEndpointAddress) { + case EM28XX_EP_AUDIO: has_audio = true; + break; + case EM28XX_EP_ANALOG: + has_video = true; + dev->alt_max_pkt_size[i] = size; + break; + case EM28XX_EP_DIGITAL: + has_dvb = true; + dev->alt_dvb_pkt_size[i] = size; + break; } } @@ -3223,14 +3264,6 @@ static int em28xx_usb_probe(struct usb_interface *interface, goto err; } - /* allocate memory for our device state and initialize it */ - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (dev == NULL) { - em28xx_err(DRIVER_NAME ": out of memory!\n"); - retval = -ENOMEM; - goto err; - } - snprintf(dev->name, sizeof(dev->name), "em28xx #%d", nr); dev->devno = nr; dev->model = id->driver_info; diff --git a/drivers/media/video/em28xx/em28xx-core.c b/drivers/media/video/em28xx/em28xx-core.c index 804a4ab..b73aaa5 100644 --- a/drivers/media/video/em28xx/em28xx-core.c +++ b/drivers/media/video/em28xx/em28xx-core.c @@ -1070,7 +1070,7 @@ int em28xx_init_isoc(struct em28xx *dev, int max_packets, should also be using 'desc.bInterval' */ pipe = usb_rcvisocpipe(dev->udev, - dev->mode == EM28XX_ANALOG_MODE ? 0x82 : 0x84); + dev->mode == EM28XX_ANALOG_MODE ? EM28XX_EP_ANALOG : EM28XX_EP_DIGITAL); usb_fill_int_urb(urb, dev->udev, pipe, dev->isoc_ctl.transfer_buffer[i], sb_size, diff --git a/drivers/media/video/em28xx/em28xx-reg.h b/drivers/media/video/em28xx/em28xx-reg.h index 66f7923..2f62685 100644 --- a/drivers/media/video/em28xx/em28xx-reg.h +++ b/drivers/media/video/em28xx/em28xx-reg.h @@ -12,6 +12,11 @@ #define EM_GPO_2 (1 << 2) #define EM_GPO_3 (1 << 3) +/* em28xx endpoints */ +#define EM28XX_EP_ANALOG 0x82 +#define EM28XX_EP_AUDIO 0x83 +#define EM28XX_EP_DIGITAL 0x84 + /* em2800 registers */ #define EM2800_R08_AUDIOSRC 0x08 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-26 12:18 ` Mauro Carvalho Chehab @ 2011-12-28 3:50 ` Holger Nelson 2011-12-28 12:09 ` Mauro Carvalho Chehab 0 siblings, 1 reply; 15+ messages in thread From: Holger Nelson @ 2011-12-28 3:50 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Dennis Sperlich, linux-media, Michael Krufky, Devin Heitmueller On Mon, 26 Dec 2011, Mauro Carvalho Chehab wrote: > I'm currently without time right now to work on a patch, but I think that several hacks > inside the em28xx probe should be removed, including the one that detects the endpoint > based on the packet size. > > As it is easier to code than to explain in words, the code below could be > a start (ok, it doesn't compile, doesn't remove all hacks, doesn't free memory, etc...) > Feel free to use it as a start for a real patch, if you wish. I think, I filled the missing parts and removed most of the hacks in the probe code. The code works with my Cinergy HTC USB XS. Holger diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c index cff0768..e2a7b77 100644 --- a/drivers/media/video/em28xx/em28xx-audio.c +++ b/drivers/media/video/em28xx/em28xx-audio.c @@ -193,7 +193,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev) urb->dev = dev->udev; urb->context = dev; - urb->pipe = usb_rcvisocpipe(dev->udev, 0x83); + urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO); urb->transfer_flags = URB_ISO_ASAP; urb->transfer_buffer = dev->adev.transfer_buffer[i]; urb->interval = 1; diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c index a7cfded..8082914 100644 --- a/drivers/media/video/em28xx/em28xx-cards.c +++ b/drivers/media/video/em28xx/em28xx-cards.c @@ -3087,12 +3087,11 @@ unregister_dev: static int em28xx_usb_probe(struct usb_interface *interface, const struct usb_device_id *id) { - const struct usb_endpoint_descriptor *endpoint; struct usb_device *udev; struct em28xx *dev = NULL; int retval; - bool is_audio_only = false, has_audio = false; - int i, nr, isoc_pipe; + bool has_audio = false, has_video = false, has_dvb = false; + int i, nr, sizedescr, size; const int ifnum = interface->altsetting[0].desc.bInterfaceNumber; char *speed; char descr[255] = ""; @@ -3124,56 +3123,69 @@ static int em28xx_usb_probe(struct usb_interface *interface, goto err; } - /* Get endpoints */ - for (i = 0; i < interface->num_altsetting; i++) { - int ep; + /* allocate memory for our device state and initialize it */ + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (dev == NULL) { + em28xx_err(DRIVER_NAME ": out of memory!\n"); + retval = -ENOMEM; + goto err; + } - for (ep = 0; ep < interface->altsetting[i].desc.bNumEndpoints; ep++) { - struct usb_host_endpoint *e; - e = &interface->altsetting[i].endpoint[ep]; + /* compute alternate max packet sizes */ + dev->alt_video_max_pkt_size = kmalloc(sizeof(dev->alt_video_max_pkt_size[0]) * interface->num_altsetting, GFP_KERNEL); + if (dev->alt_video_max_pkt_size == NULL) { + em28xx_errdev("out of memory!\n"); + kfree(dev); + retval = -ENOMEM; + goto err; + } - if (e->desc.bEndpointAddress == 0x83) - has_audio = true; - } + dev->alt_dvb_max_pkt_size = kmalloc(sizeof(dev->alt_dvb_max_pkt_size[0]) * interface->num_altsetting, GFP_KERNEL); + if (dev->alt_dvb_max_pkt_size == NULL) { + em28xx_errdev("out of memory!\n"); + kfree(dev->alt_video_max_pkt_size); + kfree(dev); + retval = -ENOMEM; + goto err; } - endpoint = &interface->cur_altsetting->endpoint[0].desc; + /* Get endpoints */ + for (i = 0; i < interface->num_altsetting; i++) { + int ep; - /* check if the device has the iso in endpoint at the correct place */ - if (usb_endpoint_xfer_isoc(endpoint) - && - (interface->altsetting[1].endpoint[0].desc.wMaxPacketSize == 940)) { - /* It's a newer em2874/em2875 device */ - isoc_pipe = 0; - } else { - int check_interface = 1; - isoc_pipe = 1; - endpoint = &interface->cur_altsetting->endpoint[1].desc; - if (!usb_endpoint_xfer_isoc(endpoint)) - check_interface = 0; - - if (usb_endpoint_dir_out(endpoint)) - check_interface = 0; - - if (!check_interface) { - if (has_audio) { - is_audio_only = true; - } else { - em28xx_err(DRIVER_NAME " video device (%04x:%04x): " - "interface %i, class %i found.\n", - le16_to_cpu(udev->descriptor.idVendor), - le16_to_cpu(udev->descriptor.idProduct), - ifnum, - interface->altsetting[0].desc.bInterfaceClass); - em28xx_err(DRIVER_NAME " This is an anciliary " - "interface not used by the driver\n"); - - retval = -ENODEV; - goto err; + for (ep = 0; ep < interface->altsetting[i].desc.bNumEndpoints; ep++) { + const struct usb_endpoint_descriptor *e; + + e = &interface->altsetting[i].endpoint[ep].desc; + + sizedescr = le16_to_cpu(e->wMaxPacketSize); + size = sizedescr & 0x7fff; + if (udev->speed == USB_SPEED_HIGH) + size = size * hb_mult(sizedescr); + + if (usb_endpoint_xfer_isoc(e) && usb_endpoint_dir_in(e)) { + switch (e->bEndpointAddress) { + case EM28XX_EP_AUDIO: + has_audio = true; + break; + case EM28XX_EP_ANALOG: + has_video = true; + dev->alt_video_max_pkt_size[i] = size; + break; + case EM28XX_EP_DIGITAL: + has_dvb = true; + dev->alt_dvb_max_pkt_size[i] = size; + break; + } } } } - + + if (!(has_audio||has_video||has_dvb)) { + retval=-ENODEV; + goto err_free_all; + } + switch (udev->speed) { case USB_SPEED_LOW: speed = "1.5"; @@ -3197,6 +3209,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, strlcat(descr, " ", sizeof(descr)); strlcat(descr, udev->product, sizeof(descr)); } + if (*descr) strlcat(descr, " ", sizeof(descr)); @@ -3213,6 +3226,14 @@ static int em28xx_usb_probe(struct usb_interface *interface, printk(KERN_INFO DRIVER_NAME ": Audio Vendor Class interface %i found\n", ifnum); + if (has_video) + printk(KERN_INFO DRIVER_NAME + ": Video interface %i found\n", + ifnum); + if (has_dvb) + printk(KERN_INFO DRIVER_NAME + ": DVB interface %i found\n", + ifnum); /* * Make sure we have 480 Mbps of bandwidth, otherwise things like @@ -3224,22 +3245,14 @@ static int em28xx_usb_probe(struct usb_interface *interface, printk(DRIVER_NAME ": Device must be connected to a high-speed" " USB 2.0 port.\n"); retval = -ENODEV; - goto err; - } - - /* allocate memory for our device state and initialize it */ - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (dev == NULL) { - em28xx_err(DRIVER_NAME ": out of memory!\n"); - retval = -ENOMEM; - goto err; + goto err_free_all; } snprintf(dev->name, sizeof(dev->name), "em28xx #%d", nr); dev->devno = nr; dev->model = id->driver_info; dev->alt = -1; - dev->is_audio_only = is_audio_only; + dev->is_audio_only = has_audio&&!(has_video||has_dvb); dev->has_alsa_audio = has_audio; dev->audio_ifnum = ifnum; @@ -3252,26 +3265,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, } } - /* compute alternate max packet sizes */ dev->num_alt = interface->num_altsetting; - dev->alt_max_pkt_size = kmalloc(32 * dev->num_alt, GFP_KERNEL); - - if (dev->alt_max_pkt_size == NULL) { - em28xx_errdev("out of memory!\n"); - kfree(dev); - retval = -ENOMEM; - goto err; - } - - for (i = 0; i < dev->num_alt ; i++) { - u16 tmp = le16_to_cpu(interface->altsetting[i].endpoint[isoc_pipe].desc.wMaxPacketSize); - unsigned int size = tmp & 0x7ff; - - if (udev->speed == USB_SPEED_HIGH) - size = size * hb_mult(tmp); - - dev->alt_max_pkt_size[i] = size; - } if ((card[nr] >= 0) && (card[nr] < em28xx_bcount)) dev->model = card[nr]; @@ -3285,9 +3279,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, retval = em28xx_init_dev(&dev, udev, interface, nr); if (retval) { mutex_unlock(&dev->lock); - kfree(dev->alt_max_pkt_size); - kfree(dev); - goto err; + goto err_free_all; } request_modules(dev); @@ -3306,6 +3298,11 @@ static int em28xx_usb_probe(struct usb_interface *interface, return 0; +err_free_all: + kfree(dev->alt_dvb_max_pkt_size); + kfree(dev->alt_video_max_pkt_size); + kfree(dev); + err: clear_bit(nr, &em28xx_devused); @@ -3369,7 +3366,8 @@ static void em28xx_usb_disconnect(struct usb_interface *interface) em28xx_close_extension(dev); if (!dev->users) { - kfree(dev->alt_max_pkt_size); + kfree(dev->alt_dvb_max_pkt_size); + kfree(dev->alt_video_max_pkt_size); kfree(dev); } } diff --git a/drivers/media/video/em28xx/em28xx-core.c b/drivers/media/video/em28xx/em28xx-core.c index 804a4ab..74608f9 100644 --- a/drivers/media/video/em28xx/em28xx-core.c +++ b/drivers/media/video/em28xx/em28xx-core.c @@ -829,14 +829,14 @@ int em28xx_set_alternate(struct em28xx *dev) for (i = 0; i < dev->num_alt; i++) { /* stop when the selected alt setting offers enough bandwidth */ - if (dev->alt_max_pkt_size[i] >= min_pkt_size) { + if (dev->alt_video_max_pkt_size[i] >= min_pkt_size) { dev->alt = i; break; /* otherwise make sure that we end up with the maximum bandwidth because the min_pkt_size equation might be wrong... */ - } else if (dev->alt_max_pkt_size[i] > - dev->alt_max_pkt_size[dev->alt]) + } else if (dev->alt_video_max_pkt_size[i] > + dev->alt_video_max_pkt_size[dev->alt]) dev->alt = i; } @@ -844,7 +844,7 @@ set_alt: if (dev->alt != prev_alt) { em28xx_coredbg("minimum isoc packet size: %u (alt=%d)\n", min_pkt_size, dev->alt); - dev->max_pkt_size = dev->alt_max_pkt_size[dev->alt]; + dev->max_pkt_size = dev->alt_video_max_pkt_size[dev->alt]; em28xx_coredbg("setting alternate %d with wMaxPacketSize=%u\n", dev->alt, dev->max_pkt_size); errCode = usb_set_interface(dev->udev, 0, dev->alt); @@ -1070,7 +1070,7 @@ int em28xx_init_isoc(struct em28xx *dev, int max_packets, should also be using 'desc.bInterval' */ pipe = usb_rcvisocpipe(dev->udev, - dev->mode == EM28XX_ANALOG_MODE ? 0x82 : 0x84); + dev->mode == EM28XX_ANALOG_MODE ? EM28XX_EP_ANALOG : EM28XX_EP_DIGITAL); usb_fill_int_urb(urb, dev->udev, pipe, dev->isoc_ctl.transfer_buffer[i], sb_size, @@ -1144,20 +1144,10 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) } break; case CHIP_ID_EM2874: - /* - * FIXME: for now assumes 564 like it was before, but the - * em2874 code should be added to return the proper value - */ - packet_size = 564; - break; case CHIP_ID_EM2884: case CHIP_ID_EM28174: default: - /* - * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T - * but not enough for 44 Mbit DVB-C. - */ - packet_size = 752; + packet_size = dev->alt_dvb_max_pkt_size[1];; } return packet_size; diff --git a/drivers/media/video/em28xx/em28xx-reg.h b/drivers/media/video/em28xx/em28xx-reg.h index 66f7923..2f62685 100644 --- a/drivers/media/video/em28xx/em28xx-reg.h +++ b/drivers/media/video/em28xx/em28xx-reg.h @@ -12,6 +12,11 @@ #define EM_GPO_2 (1 << 2) #define EM_GPO_3 (1 << 3) +/* em28xx endpoints */ +#define EM28XX_EP_ANALOG 0x82 +#define EM28XX_EP_AUDIO 0x83 +#define EM28XX_EP_DIGITAL 0x84 + /* em2800 registers */ #define EM2800_R08_AUDIOSRC 0x08 diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c index 9b4557a..af9f0b7 100644 --- a/drivers/media/video/em28xx/em28xx-video.c +++ b/drivers/media/video/em28xx/em28xx-video.c @@ -2254,7 +2254,8 @@ static int em28xx_v4l2_close(struct file *filp) free the remaining resources */ if (dev->state & DEV_DISCONNECTED) { em28xx_release_resources(dev); - kfree(dev->alt_max_pkt_size); + kfree(dev->alt_dvb_max_pkt_size); + kfree(dev->alt_video_max_pkt_size); kfree(dev); return 0; } diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h index b1199ef..f73f028 100644 --- a/drivers/media/video/em28xx/em28xx.h +++ b/drivers/media/video/em28xx/em28xx.h @@ -596,7 +596,8 @@ struct em28xx { int alt; /* alternate */ int max_pkt_size; /* max packet size of isoc transaction */ int num_alt; /* Number of alternative settings */ - unsigned int *alt_max_pkt_size; /* array of wMaxPacketSize */ + unsigned int *alt_video_max_pkt_size; /* array of wMaxPacketSize */ + unsigned int *alt_dvb_max_pkt_size; /* array of wMaxPacketSize */ struct urb *urb[EM28XX_NUM_BUFS]; /* urb for isoc transfers */ char *transfer_buffer[EM28XX_NUM_BUFS]; /* transfer buffers for isoc transfer */ ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) 2011-12-28 3:50 ` Holger Nelson @ 2011-12-28 12:09 ` Mauro Carvalho Chehab 2011-12-28 22:55 ` [PATCH] em28xx: Reworked probe code to get rid of some hacks (was: Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick)) Holger Nelson 0 siblings, 1 reply; 15+ messages in thread From: Mauro Carvalho Chehab @ 2011-12-28 12:09 UTC (permalink / raw) To: Holger Nelson Cc: Dennis Sperlich, linux-media, Michael Krufky, Devin Heitmueller On 28-12-2011 01:50, Holger Nelson wrote: > On Mon, 26 Dec 2011, Mauro Carvalho Chehab wrote: > >> I'm currently without time right now to work on a patch, but I think that several hacks >> inside the em28xx probe should be removed, including the one that detects the endpoint >> based on the packet size. >> >> As it is easier to code than to explain in words, the code below could be >> a start (ok, it doesn't compile, doesn't remove all hacks, doesn't free memory, etc...) >> Feel free to use it as a start for a real patch, if you wish. > > I think, I filled the missing parts and removed most of the hacks in the probe code. The code works with my Cinergy HTC USB XS. The code looks sane on my eyes. Didn't test it through. On the final version, please include a proper description for the patch and your Signed-off-by:, according with: http://linuxtv.org/wiki/index.php/Development:_How_to_submit_patches > > Holger > > diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c > index cff0768..e2a7b77 100644 > --- a/drivers/media/video/em28xx/em28xx-audio.c > +++ b/drivers/media/video/em28xx/em28xx-audio.c > @@ -193,7 +193,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev) > > urb->dev = dev->udev; > urb->context = dev; > - urb->pipe = usb_rcvisocpipe(dev->udev, 0x83); > + urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO); > urb->transfer_flags = URB_ISO_ASAP; > urb->transfer_buffer = dev->adev.transfer_buffer[i]; > urb->interval = 1; > diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c > index a7cfded..8082914 100644 > --- a/drivers/media/video/em28xx/em28xx-cards.c > +++ b/drivers/media/video/em28xx/em28xx-cards.c > @@ -3087,12 +3087,11 @@ unregister_dev: > static int em28xx_usb_probe(struct usb_interface *interface, > const struct usb_device_id *id) > { > - const struct usb_endpoint_descriptor *endpoint; > struct usb_device *udev; > struct em28xx *dev = NULL; > int retval; > - bool is_audio_only = false, has_audio = false; > - int i, nr, isoc_pipe; > + bool has_audio = false, has_video = false, has_dvb = false; > + int i, nr, sizedescr, size; > const int ifnum = interface->altsetting[0].desc.bInterfaceNumber; > char *speed; > char descr[255] = ""; > @@ -3124,56 +3123,69 @@ static int em28xx_usb_probe(struct usb_interface *interface, > goto err; > } > > - /* Get endpoints */ > - for (i = 0; i < interface->num_altsetting; i++) { > - int ep; > + /* allocate memory for our device state and initialize it */ > + dev = kzalloc(sizeof(*dev), GFP_KERNEL); > + if (dev == NULL) { > + em28xx_err(DRIVER_NAME ": out of memory!\n"); > + retval = -ENOMEM; > + goto err; > + } > > - for (ep = 0; ep < interface->altsetting[i].desc.bNumEndpoints; ep++) { > - struct usb_host_endpoint *e; > - e = &interface->altsetting[i].endpoint[ep]; > + /* compute alternate max packet sizes */ > + dev->alt_video_max_pkt_size = kmalloc(sizeof(dev->alt_video_max_pkt_size[0]) * interface->num_altsetting, GFP_KERNEL); > + if (dev->alt_video_max_pkt_size == NULL) { > + em28xx_errdev("out of memory!\n"); > + kfree(dev); > + retval = -ENOMEM; > + goto err; > + } > > - if (e->desc.bEndpointAddress == 0x83) > - has_audio = true; > - } > + dev->alt_dvb_max_pkt_size = kmalloc(sizeof(dev->alt_dvb_max_pkt_size[0]) * interface->num_altsetting, GFP_KERNEL); > + if (dev->alt_dvb_max_pkt_size == NULL) { > + em28xx_errdev("out of memory!\n"); > + kfree(dev->alt_video_max_pkt_size); > + kfree(dev); > + retval = -ENOMEM; > + goto err; > } > > - endpoint = &interface->cur_altsetting->endpoint[0].desc; > + /* Get endpoints */ > + for (i = 0; i < interface->num_altsetting; i++) { > + int ep; > > - /* check if the device has the iso in endpoint at the correct place */ > - if (usb_endpoint_xfer_isoc(endpoint) > - && > - (interface->altsetting[1].endpoint[0].desc.wMaxPacketSize == 940)) { > - /* It's a newer em2874/em2875 device */ > - isoc_pipe = 0; > - } else { > - int check_interface = 1; > - isoc_pipe = 1; > - endpoint = &interface->cur_altsetting->endpoint[1].desc; > - if (!usb_endpoint_xfer_isoc(endpoint)) > - check_interface = 0; > - > - if (usb_endpoint_dir_out(endpoint)) > - check_interface = 0; > - > - if (!check_interface) { > - if (has_audio) { > - is_audio_only = true; > - } else { > - em28xx_err(DRIVER_NAME " video device (%04x:%04x): " > - "interface %i, class %i found.\n", > - le16_to_cpu(udev->descriptor.idVendor), > - le16_to_cpu(udev->descriptor.idProduct), > - ifnum, > - interface->altsetting[0].desc.bInterfaceClass); > - em28xx_err(DRIVER_NAME " This is an anciliary " > - "interface not used by the driver\n"); > - > - retval = -ENODEV; > - goto err; > + for (ep = 0; ep < interface->altsetting[i].desc.bNumEndpoints; ep++) { > + const struct usb_endpoint_descriptor *e; > + + e = &interface->altsetting[i].endpoint[ep].desc; There is an extra "+" here. > + > + sizedescr = le16_to_cpu(e->wMaxPacketSize); > + size = sizedescr & 0x7fff; > + if (udev->speed == USB_SPEED_HIGH) > + size = size * hb_mult(sizedescr); > + > + if (usb_endpoint_xfer_isoc(e) && usb_endpoint_dir_in(e)) { > + switch (e->bEndpointAddress) { > + case EM28XX_EP_AUDIO: > + has_audio = true; > + break; > + case EM28XX_EP_ANALOG: > + has_video = true; > + dev->alt_video_max_pkt_size[i] = size; > + break; > + case EM28XX_EP_DIGITAL: > + has_dvb = true; > + dev->alt_dvb_max_pkt_size[i] = size; > + break; > + } > } > } > } > - > + + if (!(has_audio||has_video||has_dvb)) { There is an extra "+" here. Also Linux CodingStyle requires spaces before and after operators: if (!(has_audio || has_video || has_dvb)) { > + retval=-ENODEV; > + goto err_free_all; > + } > + > switch (udev->speed) { > case USB_SPEED_LOW: > speed = "1.5"; > @@ -3197,6 +3209,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, > strlcat(descr, " ", sizeof(descr)); > strlcat(descr, udev->product, sizeof(descr)); > } > + > if (*descr) > strlcat(descr, " ", sizeof(descr)); > > @@ -3213,6 +3226,14 @@ static int em28xx_usb_probe(struct usb_interface *interface, > printk(KERN_INFO DRIVER_NAME > ": Audio Vendor Class interface %i found\n", > ifnum); > + if (has_video) > + printk(KERN_INFO DRIVER_NAME > + ": Video interface %i found\n", > + ifnum); > + if (has_dvb) > + printk(KERN_INFO DRIVER_NAME > + ": DVB interface %i found\n", > + ifnum); > > /* > * Make sure we have 480 Mbps of bandwidth, otherwise things like > @@ -3224,22 +3245,14 @@ static int em28xx_usb_probe(struct usb_interface *interface, > printk(DRIVER_NAME ": Device must be connected to a high-speed" > " USB 2.0 port.\n"); > retval = -ENODEV; > - goto err; > - } > - > - /* allocate memory for our device state and initialize it */ > - dev = kzalloc(sizeof(*dev), GFP_KERNEL); > - if (dev == NULL) { > - em28xx_err(DRIVER_NAME ": out of memory!\n"); > - retval = -ENOMEM; > - goto err; > + goto err_free_all; > } > > snprintf(dev->name, sizeof(dev->name), "em28xx #%d", nr); > dev->devno = nr; > dev->model = id->driver_info; > dev->alt = -1; > - dev->is_audio_only = is_audio_only; > + dev->is_audio_only = has_audio&&!(has_video||has_dvb); CodingStyle: it should be, instead: dev->is_audio_only = has_audio && !(has_video || has_dvb); > dev->has_alsa_audio = has_audio; > dev->audio_ifnum = ifnum; > > @@ -3252,26 +3265,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, > } > } > > - /* compute alternate max packet sizes */ > dev->num_alt = interface->num_altsetting; > - dev->alt_max_pkt_size = kmalloc(32 * dev->num_alt, GFP_KERNEL); > - > - if (dev->alt_max_pkt_size == NULL) { > - em28xx_errdev("out of memory!\n"); > - kfree(dev); > - retval = -ENOMEM; > - goto err; > - } > - > - for (i = 0; i < dev->num_alt ; i++) { > - u16 tmp = le16_to_cpu(interface->altsetting[i].endpoint[isoc_pipe].desc.wMaxPacketSize); > - unsigned int size = tmp & 0x7ff; > - > - if (udev->speed == USB_SPEED_HIGH) > - size = size * hb_mult(tmp); > - > - dev->alt_max_pkt_size[i] = size; > - } > > if ((card[nr] >= 0) && (card[nr] < em28xx_bcount)) > dev->model = card[nr]; > @@ -3285,9 +3279,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, > retval = em28xx_init_dev(&dev, udev, interface, nr); > if (retval) { > mutex_unlock(&dev->lock); > - kfree(dev->alt_max_pkt_size); > - kfree(dev); > - goto err; > + goto err_free_all; Maybe you could also move the mutex_unlock to the error path, like: goto unlock_and_free; > } > > request_modules(dev); > @@ -3306,6 +3298,11 @@ static int em28xx_usb_probe(struct usb_interface *interface, > > return 0; > > +err_free_all: > + kfree(dev->alt_dvb_max_pkt_size); > + kfree(dev->alt_video_max_pkt_size); > + kfree(dev); > + > err: > clear_bit(nr, &em28xx_devused); > > @@ -3369,7 +3366,8 @@ static void em28xx_usb_disconnect(struct usb_interface *interface) > em28xx_close_extension(dev); > > if (!dev->users) { > - kfree(dev->alt_max_pkt_size); > + kfree(dev->alt_dvb_max_pkt_size); > + kfree(dev->alt_video_max_pkt_size); > kfree(dev); > } > } > diff --git a/drivers/media/video/em28xx/em28xx-core.c b/drivers/media/video/em28xx/em28xx-core.c > index 804a4ab..74608f9 100644 > --- a/drivers/media/video/em28xx/em28xx-core.c > +++ b/drivers/media/video/em28xx/em28xx-core.c > @@ -829,14 +829,14 @@ int em28xx_set_alternate(struct em28xx *dev) > > for (i = 0; i < dev->num_alt; i++) { > /* stop when the selected alt setting offers enough bandwidth */ > - if (dev->alt_max_pkt_size[i] >= min_pkt_size) { > + if (dev->alt_video_max_pkt_size[i] >= min_pkt_size) { > dev->alt = i; > break; > /* otherwise make sure that we end up with the maximum bandwidth > because the min_pkt_size equation might be wrong... > */ > - } else if (dev->alt_max_pkt_size[i] > > - dev->alt_max_pkt_size[dev->alt]) > + } else if (dev->alt_video_max_pkt_size[i] > > + dev->alt_video_max_pkt_size[dev->alt]) > dev->alt = i; > } > > @@ -844,7 +844,7 @@ set_alt: > if (dev->alt != prev_alt) { > em28xx_coredbg("minimum isoc packet size: %u (alt=%d)\n", > min_pkt_size, dev->alt); > - dev->max_pkt_size = dev->alt_max_pkt_size[dev->alt]; > + dev->max_pkt_size = dev->alt_video_max_pkt_size[dev->alt]; > em28xx_coredbg("setting alternate %d with wMaxPacketSize=%u\n", > dev->alt, dev->max_pkt_size); > errCode = usb_set_interface(dev->udev, 0, dev->alt); > @@ -1070,7 +1070,7 @@ int em28xx_init_isoc(struct em28xx *dev, int max_packets, > should also be using 'desc.bInterval' > */ > pipe = usb_rcvisocpipe(dev->udev, > - dev->mode == EM28XX_ANALOG_MODE ? 0x82 : 0x84); > + dev->mode == EM28XX_ANALOG_MODE ? EM28XX_EP_ANALOG : EM28XX_EP_DIGITAL); > > usb_fill_int_urb(urb, dev->udev, pipe, > dev->isoc_ctl.transfer_buffer[i], sb_size, > @@ -1144,20 +1144,10 @@ int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) > } > break; > case CHIP_ID_EM2874: > - /* > - * FIXME: for now assumes 564 like it was before, but the > - * em2874 code should be added to return the proper value > - */ > - packet_size = 564; > - break; > case CHIP_ID_EM2884: > case CHIP_ID_EM28174: > default: > - /* > - * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T > - * but not enough for 44 Mbit DVB-C. > - */ > - packet_size = 752; > + packet_size = dev->alt_dvb_max_pkt_size[1];; There are two ";;" above. It could make sense to do a loop here and get the biggest max_pkt_size. Of course, this would mean that, at em28xx_start_streaming, the code should be changed from: usb_set_interface(dev->udev, 0, 1); to: usb_set_interface(dev->udev, 0, dev->dvb_alternate); and that the loop would fill dvb_alternate with the alternate associated with the selected packet size. Now that the packet_size is coming from the endpoint, I think that the chipset-specific code there could be removed completely. > } > > return packet_size; > diff --git a/drivers/media/video/em28xx/em28xx-reg.h b/drivers/media/video/em28xx/em28xx-reg.h > index 66f7923..2f62685 100644 > --- a/drivers/media/video/em28xx/em28xx-reg.h > +++ b/drivers/media/video/em28xx/em28xx-reg.h > @@ -12,6 +12,11 @@ > #define EM_GPO_2 (1 << 2) > #define EM_GPO_3 (1 << 3) > > +/* em28xx endpoints */ > +#define EM28XX_EP_ANALOG 0x82 > +#define EM28XX_EP_AUDIO 0x83 > +#define EM28XX_EP_DIGITAL 0x84 > + > /* em2800 registers */ > #define EM2800_R08_AUDIOSRC 0x08 > > diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c > index 9b4557a..af9f0b7 100644 > --- a/drivers/media/video/em28xx/em28xx-video.c > +++ b/drivers/media/video/em28xx/em28xx-video.c > @@ -2254,7 +2254,8 @@ static int em28xx_v4l2_close(struct file *filp) > free the remaining resources */ > if (dev->state & DEV_DISCONNECTED) { > em28xx_release_resources(dev); > - kfree(dev->alt_max_pkt_size); > + kfree(dev->alt_dvb_max_pkt_size); > + kfree(dev->alt_video_max_pkt_size); > kfree(dev); > return 0; > } > diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h > index b1199ef..f73f028 100644 > --- a/drivers/media/video/em28xx/em28xx.h > +++ b/drivers/media/video/em28xx/em28xx.h > @@ -596,7 +596,8 @@ struct em28xx { > int alt; /* alternate */ > int max_pkt_size; /* max packet size of isoc transaction */ > int num_alt; /* Number of alternative settings */ > - unsigned int *alt_max_pkt_size; /* array of wMaxPacketSize */ > + unsigned int *alt_video_max_pkt_size; /* array of wMaxPacketSize */ > + unsigned int *alt_dvb_max_pkt_size; /* array of wMaxPacketSize */ > struct urb *urb[EM28XX_NUM_BUFS]; /* urb for isoc transfers */ > char *transfer_buffer[EM28XX_NUM_BUFS]; /* transfer buffers for isoc > transfer */ > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] em28xx: Reworked probe code to get rid of some hacks (was: Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick)) 2011-12-28 12:09 ` Mauro Carvalho Chehab @ 2011-12-28 22:55 ` Holger Nelson 0 siblings, 0 replies; 15+ messages in thread From: Holger Nelson @ 2011-12-28 22:55 UTC (permalink / raw) To: Mauro Carvalho Chehab Cc: Dennis Sperlich, linux-media, Michael Krufky, Devin Heitmueller Reworked device probing to get rid of hacks to guess the maximum size of dvb iso transfer packets. The new code also selects the first alternate config which supports the largest possible iso transfers for dvb. Signed-off-by: Holger Nelson <hnelson@hnelson.de> diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c index cff0768..e2a7b77 100644 --- a/drivers/media/video/em28xx/em28xx-audio.c +++ b/drivers/media/video/em28xx/em28xx-audio.c @@ -193,7 +193,7 @@ static int em28xx_init_audio_isoc(struct em28xx *dev) urb->dev = dev->udev; urb->context = dev; - urb->pipe = usb_rcvisocpipe(dev->udev, 0x83); + urb->pipe = usb_rcvisocpipe(dev->udev, EM28XX_EP_AUDIO); urb->transfer_flags = URB_ISO_ASAP; urb->transfer_buffer = dev->adev.transfer_buffer[i]; urb->interval = 1; diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c index a7cfded..027f769 100644 --- a/drivers/media/video/em28xx/em28xx-cards.c +++ b/drivers/media/video/em28xx/em28xx-cards.c @@ -3087,12 +3087,11 @@ unregister_dev: static int em28xx_usb_probe(struct usb_interface *interface, const struct usb_device_id *id) { - const struct usb_endpoint_descriptor *endpoint; struct usb_device *udev; struct em28xx *dev = NULL; int retval; - bool is_audio_only = false, has_audio = false; - int i, nr, isoc_pipe; + bool has_audio = false, has_video = false, has_dvb = false; + int i, nr; const int ifnum = interface->altsetting[0].desc.bInterfaceNumber; char *speed; char descr[255] = ""; @@ -3124,54 +3123,63 @@ static int em28xx_usb_probe(struct usb_interface *interface, goto err; } + /* allocate memory for our device state and initialize it */ + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (dev == NULL) { + em28xx_err(DRIVER_NAME ": out of memory!\n"); + retval = -ENOMEM; + goto err; + } + + /* compute alternate max packet sizes */ + dev->alt_max_pkt_size = kmalloc(sizeof(dev->alt_max_pkt_size[0]) * interface->num_altsetting, GFP_KERNEL); + if (dev->alt_max_pkt_size == NULL) { + em28xx_errdev("out of memory!\n"); + kfree(dev); + retval = -ENOMEM; + goto err; + } + /* Get endpoints */ for (i = 0; i < interface->num_altsetting; i++) { int ep; for (ep = 0; ep < interface->altsetting[i].desc.bNumEndpoints; ep++) { - struct usb_host_endpoint *e; - e = &interface->altsetting[i].endpoint[ep]; - - if (e->desc.bEndpointAddress == 0x83) - has_audio = true; + const struct usb_endpoint_descriptor *e; + int sizedescr, size; + + e = &interface->altsetting[i].endpoint[ep].desc; + + sizedescr = le16_to_cpu(e->wMaxPacketSize); + size = sizedescr & 0x7ff; + + if (udev->speed == USB_SPEED_HIGH) + size = size * hb_mult(sizedescr); + + if (usb_endpoint_xfer_isoc(e) && usb_endpoint_dir_in(e)) { + switch (e->bEndpointAddress) { + case EM28XX_EP_AUDIO: + has_audio = true; + break; + case EM28XX_EP_ANALOG: + has_video = true; + dev->alt_max_pkt_size[i] = size; + break; + case EM28XX_EP_DIGITAL: + has_dvb = true; + if (size > dev->dvb_max_pkt_size) { + dev->dvb_max_pkt_size = size; + dev->dvb_alt = i; + } + break; + } + } } } - endpoint = &interface->cur_altsetting->endpoint[0].desc; - - /* check if the device has the iso in endpoint at the correct place */ - if (usb_endpoint_xfer_isoc(endpoint) - && - (interface->altsetting[1].endpoint[0].desc.wMaxPacketSize == 940)) { - /* It's a newer em2874/em2875 device */ - isoc_pipe = 0; - } else { - int check_interface = 1; - isoc_pipe = 1; - endpoint = &interface->cur_altsetting->endpoint[1].desc; - if (!usb_endpoint_xfer_isoc(endpoint)) - check_interface = 0; - - if (usb_endpoint_dir_out(endpoint)) - check_interface = 0; - - if (!check_interface) { - if (has_audio) { - is_audio_only = true; - } else { - em28xx_err(DRIVER_NAME " video device (%04x:%04x): " - "interface %i, class %i found.\n", - le16_to_cpu(udev->descriptor.idVendor), - le16_to_cpu(udev->descriptor.idProduct), - ifnum, - interface->altsetting[0].desc.bInterfaceClass); - em28xx_err(DRIVER_NAME " This is an anciliary " - "interface not used by the driver\n"); - - retval = -ENODEV; - goto err; - } - } + if (!(has_audio || has_video || has_dvb)) { + retval=-ENODEV; + goto err_free; } switch (udev->speed) { @@ -3197,6 +3205,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, strlcat(descr, " ", sizeof(descr)); strlcat(descr, udev->product, sizeof(descr)); } + if (*descr) strlcat(descr, " ", sizeof(descr)); @@ -3213,6 +3222,14 @@ static int em28xx_usb_probe(struct usb_interface *interface, printk(KERN_INFO DRIVER_NAME ": Audio Vendor Class interface %i found\n", ifnum); + if (has_video) + printk(KERN_INFO DRIVER_NAME + ": Video interface %i found\n", + ifnum); + if (has_dvb) + printk(KERN_INFO DRIVER_NAME + ": DVB interface %i found\n", + ifnum); /* * Make sure we have 480 Mbps of bandwidth, otherwise things like @@ -3224,22 +3241,14 @@ static int em28xx_usb_probe(struct usb_interface *interface, printk(DRIVER_NAME ": Device must be connected to a high-speed" " USB 2.0 port.\n"); retval = -ENODEV; - goto err; - } - - /* allocate memory for our device state and initialize it */ - dev = kzalloc(sizeof(*dev), GFP_KERNEL); - if (dev == NULL) { - em28xx_err(DRIVER_NAME ": out of memory!\n"); - retval = -ENOMEM; - goto err; + goto err_free; } snprintf(dev->name, sizeof(dev->name), "em28xx #%d", nr); dev->devno = nr; dev->model = id->driver_info; dev->alt = -1; - dev->is_audio_only = is_audio_only; + dev->is_audio_only = has_audio && !(has_video || has_dvb); dev->has_alsa_audio = has_audio; dev->audio_ifnum = ifnum; @@ -3252,26 +3261,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, } } - /* compute alternate max packet sizes */ dev->num_alt = interface->num_altsetting; - dev->alt_max_pkt_size = kmalloc(32 * dev->num_alt, GFP_KERNEL); - - if (dev->alt_max_pkt_size == NULL) { - em28xx_errdev("out of memory!\n"); - kfree(dev); - retval = -ENOMEM; - goto err; - } - - for (i = 0; i < dev->num_alt ; i++) { - u16 tmp = le16_to_cpu(interface->altsetting[i].endpoint[isoc_pipe].desc.wMaxPacketSize); - unsigned int size = tmp & 0x7ff; - - if (udev->speed == USB_SPEED_HIGH) - size = size * hb_mult(tmp); - - dev->alt_max_pkt_size[i] = size; - } if ((card[nr] >= 0) && (card[nr] < em28xx_bcount)) dev->model = card[nr]; @@ -3284,10 +3274,7 @@ static int em28xx_usb_probe(struct usb_interface *interface, mutex_lock(&dev->lock); retval = em28xx_init_dev(&dev, udev, interface, nr); if (retval) { - mutex_unlock(&dev->lock); - kfree(dev->alt_max_pkt_size); - kfree(dev); - goto err; + goto unlock_and_free; } request_modules(dev); @@ -3306,6 +3293,13 @@ static int em28xx_usb_probe(struct usb_interface *interface, return 0; +unlock_and_free: + mutex_unlock(&dev->lock); + +err_free: + kfree(dev->alt_max_pkt_size); + kfree(dev); + err: clear_bit(nr, &em28xx_devused); diff --git a/drivers/media/video/em28xx/em28xx-core.c b/drivers/media/video/em28xx/em28xx-core.c index 804a4ab..bc1d5dd 100644 --- a/drivers/media/video/em28xx/em28xx-core.c +++ b/drivers/media/video/em28xx/em28xx-core.c @@ -1070,7 +1070,7 @@ int em28xx_init_isoc(struct em28xx *dev, int max_packets, should also be using 'desc.bInterval' */ pipe = usb_rcvisocpipe(dev->udev, - dev->mode == EM28XX_ANALOG_MODE ? 0x82 : 0x84); + dev->mode == EM28XX_ANALOG_MODE ? EM28XX_EP_ANALOG : EM28XX_EP_DIGITAL); usb_fill_int_urb(urb, dev->udev, pipe, dev->isoc_ctl.transfer_buffer[i], sb_size, @@ -1108,62 +1108,6 @@ int em28xx_init_isoc(struct em28xx *dev, int max_packets, } EXPORT_SYMBOL_GPL(em28xx_init_isoc); -/* Determine the packet size for the DVB stream for the given device - (underlying value programmed into the eeprom) */ -int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) -{ - unsigned int chip_cfg2; - unsigned int packet_size; - - switch (dev->chip_id) { - case CHIP_ID_EM2710: - case CHIP_ID_EM2750: - case CHIP_ID_EM2800: - case CHIP_ID_EM2820: - case CHIP_ID_EM2840: - case CHIP_ID_EM2860: - /* No DVB support */ - return -EINVAL; - case CHIP_ID_EM2870: - case CHIP_ID_EM2883: - /* TS max packet size stored in bits 1-0 of R01 */ - chip_cfg2 = em28xx_read_reg(dev, EM28XX_R01_CHIPCFG2); - switch (chip_cfg2 & EM28XX_CHIPCFG2_TS_PACKETSIZE_MASK) { - case EM28XX_CHIPCFG2_TS_PACKETSIZE_188: - packet_size = 188; - break; - case EM28XX_CHIPCFG2_TS_PACKETSIZE_376: - packet_size = 376; - break; - case EM28XX_CHIPCFG2_TS_PACKETSIZE_564: - packet_size = 564; - break; - case EM28XX_CHIPCFG2_TS_PACKETSIZE_752: - packet_size = 752; - break; - } - break; - case CHIP_ID_EM2874: - /* - * FIXME: for now assumes 564 like it was before, but the - * em2874 code should be added to return the proper value - */ - packet_size = 564; - break; - case CHIP_ID_EM2884: - case CHIP_ID_EM28174: - default: - /* - * FIXME: same as em2874. 564 was enough for 22 Mbit DVB-T - * but not enough for 44 Mbit DVB-C. - */ - packet_size = 752; - } - - return packet_size; -} -EXPORT_SYMBOL_GPL(em28xx_isoc_dvb_max_packetsize); - /* * em28xx_wake_i2c() * configure i2c attached devices diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c index 3868c1e..52ff128 100644 --- a/drivers/media/video/em28xx/em28xx-dvb.c +++ b/drivers/media/video/em28xx/em28xx-dvb.c @@ -163,12 +163,12 @@ static int em28xx_start_streaming(struct em28xx_dvb *dvb) struct em28xx *dev = dvb->adapter.priv; int max_dvb_packet_size; - usb_set_interface(dev->udev, 0, 1); + usb_set_interface(dev->udev, 0, dev->dvb_alt); rc = em28xx_set_mode(dev, EM28XX_DIGITAL_MODE); if (rc < 0) return rc; - max_dvb_packet_size = em28xx_isoc_dvb_max_packetsize(dev); + max_dvb_packet_size = dev->dvb_max_pkt_size; if (max_dvb_packet_size < 0) return max_dvb_packet_size; dprintk(1, "Using %d buffers each with %d bytes\n", diff --git a/drivers/media/video/em28xx/em28xx-reg.h b/drivers/media/video/em28xx/em28xx-reg.h index 66f7923..2f62685 100644 --- a/drivers/media/video/em28xx/em28xx-reg.h +++ b/drivers/media/video/em28xx/em28xx-reg.h @@ -12,6 +12,11 @@ #define EM_GPO_2 (1 << 2) #define EM_GPO_3 (1 << 3) +/* em28xx endpoints */ +#define EM28XX_EP_ANALOG 0x82 +#define EM28XX_EP_AUDIO 0x83 +#define EM28XX_EP_DIGITAL 0x84 + /* em2800 registers */ #define EM2800_R08_AUDIOSRC 0x08 diff --git a/drivers/media/video/em28xx/em28xx.h b/drivers/media/video/em28xx/em28xx.h index b1199ef..85394cb 100644 --- a/drivers/media/video/em28xx/em28xx.h +++ b/drivers/media/video/em28xx/em28xx.h @@ -597,6 +597,8 @@ struct em28xx { int max_pkt_size; /* max packet size of isoc transaction */ int num_alt; /* Number of alternative settings */ unsigned int *alt_max_pkt_size; /* array of wMaxPacketSize */ + int dvb_alt; /* alternate for DVB */ + unsigned int dvb_max_pkt_size; /* wMaxPacketSize for DVB */ struct urb *urb[EM28XX_NUM_BUFS]; /* urb for isoc transfers */ char *transfer_buffer[EM28XX_NUM_BUFS]; /* transfer buffers for isoc transfer */ ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-12-28 22:55 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-24 21:58 em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick) Dennis Sperlich 2011-12-25 10:52 ` Mauro Carvalho Chehab 2011-12-25 14:04 ` Dennis Sperlich 2011-12-25 14:11 ` Hans Petter Selasky 2011-12-25 14:47 ` Devin Heitmueller 2011-12-25 17:58 ` Mauro Carvalho Chehab 2011-12-25 19:42 ` Malcolm Priestley 2011-12-25 20:12 ` Dennis Sperlich 2011-12-25 18:13 ` Mauro Carvalho Chehab 2011-12-25 20:33 ` Dennis Sperlich 2011-12-26 5:55 ` Holger Nelson 2011-12-26 12:18 ` Mauro Carvalho Chehab 2011-12-28 3:50 ` Holger Nelson 2011-12-28 12:09 ` Mauro Carvalho Chehab 2011-12-28 22:55 ` [PATCH] em28xx: Reworked probe code to get rid of some hacks (was: Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick)) Holger Nelson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.