em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick)
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
drivers/media/video/em28xx/em28xx-core.c +1 -1
@@ -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;
Quoted 15 lines · Dennis Sperlich
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_packetsizefrom 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,
Dennisindex 804a4ab..c518d13 100644
drivers/media/video/em28xx/em28xx-core.c int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) · line 1163
Quoted 8 lines
drivers/media/video/em28xx/em28xx-core.c +1
-1@@ -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
Quoted 57 lines · Mauro Carvalho Chehab
Quoted 15 lines · Dennis Sperlich
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_packetsizefrom 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,
Dennisindex 804a4ab..c518d13 100644
drivers/media/video/em28xx/em28xx-core.c int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) · line 1163
Quoted 8 lines
drivers/media/video/em28xx/em28xx-core.c +1
-1@@ -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
orem28xx chip ID = 38The 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; }
Quoted 2 lines
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
drivers/media/video/em28xx/em28xx-core.c +2 -0
@@ -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 :/*
Quoted 2 lines
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
Quoted 17 lines · On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote:
Quoted 16 lines · Mauro Carvalho Chehab
Quoted 15 lines · Dennis Sperlich
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_packetsizefrom 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.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
If you need more testing pleas tell.
Regards,
Dennis
These numbers should not be hardcoded, but extracted from the USB endpoint descriptor!
--HPS
Quoted 4 lines · Hans Petter Selasky
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
Signature · 2 lines
Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com
Quoted 18 lines · Hans Petter Selasky
Quoted 17 lines · On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote:
Quoted 16 lines · Mauro Carvalho Chehab
Quoted 15 lines · Dennis Sperlich
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_packetsizefrom 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.
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
If you need more testing pleas tell.
Regards,
DennisThese 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.
Quoted 2 lines
--HPS
Quoted 78 lines · Dennis Sperlich
Quoted 57 lines · Mauro Carvalho Chehab
Quoted 15 lines · Dennis Sperlich
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_packetsizefrom 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,
Dennisindex 804a4ab..c518d13 100644
drivers/media/video/em28xx/em28xx-core.c int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) · line 1163
Quoted 8 lines
drivers/media/video/em28xx/em28xx-core.c +1
-1@@ -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
orem28xx chip ID = 38The 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; }Quoted 2 lines
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
drivers/media/video/em28xx/em28xx-core.c +2
-0@@ -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 :/*
Quoted 6 lines
Quoted 2 lines
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
Quoted 5 lines · Hans Petter Selasky
Quoted 4 lines · On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote:
Quoted 3 lines · Mauro Carvalho Chehab
Quoted 2 lines · Dennis Sperlich
Hi,
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
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_packetsizefrom 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
Quoted 19 lines · Malcolm Priestley
Quoted 5 lines · Hans Petter Selasky
Quoted 4 lines · On Sunday 25 December 2011 15:04:17 Dennis Sperlich wrote:
Quoted 3 lines · Mauro Carvalho Chehab
Quoted 2 lines · Dennis Sperlich
Hi,
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
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_packetsizefrom 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.
Quoted 2 lines
Would not increasing
EM28XX_DVB_NUM_BUFScurrently set at 5 to say 10 have a better effect?
This does not work, I just tried it.
Regards,
Dennis
Quoted 100 lines · Mauro Carvalho Chehab
Quoted 77 lines · Dennis Sperlich
Quoted 57 lines · Mauro Carvalho Chehab
Quoted 15 lines · Dennis Sperlich
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_packetsizefrom 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,
Dennisindex 804a4ab..c518d13 100644
drivers/media/video/em28xx/em28xx-core.c int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) · line 1163
Quoted 8 lines
drivers/media/video/em28xx/em28xx-core.c +1
-1@@ -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
orem28xx chip ID = 38The 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; }Quoted 2 lines
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
drivers/media/video/em28xx/em28xx-core.c +2
-0@@ -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 :/* Quoted 5 lines
Quoted 2 lines
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)
Quoted 7 lines
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
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)
Hi!
Quoted 5 lines · Dennis Sperlich
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];
Quoted 8 lines
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
drivers/media/video/em28xx/em28xx-cards.c +26 -0
@@ -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); }
drivers/media/video/em28xx/em28xx-core.c +1 -1
@@ -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;
drivers/media/video/em28xx/em28xx-video.c +1 -0
@@ -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 ;
drivers/media/video/em28xx/em28xx.h +1 -0
@@ -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 */
Quoted 28 lines · Holger Nelson
Hi!
Quoted 5 lines · Dennis Sperlich
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];Quoted 7 lines
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_sizegets the maximum packet sizes for the analog video input endpoint (0x82in 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
drivers/media/video/em28xx/em28xx-cards.c +0
-0
drivers/media/video/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3276
Quoted 4 lines
drivers/media/video/em28xx/em28xx-cards.c +1
-0@@ -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]).
drivers/media/video/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3289
Quoted 13 lines
drivers/media/video/em28xx/em28xx-cards.c +13
-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
Quoted 59 lines
drivers/media/video/em28xx/em28xx-cards.c +12
-0+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); }drivers/media/video/em28xx/em28xx-core.c +1
-1@@ -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;drivers/media/video/em28xx/em28xx-video.c +1
-0@@ -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 ;drivers/media/video/em28xx/em28xx.h +0
-0
drivers/media/video/em28xx/em28xx.h struct em28xx { · line 603
Quoted 7 lines
drivers/media/video/em28xx/em28xx.h +1
-0@@ -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.
drivers/media/video/em28xx/em28xx-cards.c +44 -11
@@ -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;
drivers/media/video/em28xx/em28xx-core.c +1 -1
@@ -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,
drivers/media/video/em28xx/em28xx-reg.h +5 -0
@@ -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
Quoted 7 lines · Mauro Carvalho Chehab
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
drivers/media/video/em28xx/em28xx-audio.c +1 -1
@@ -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 ;
drivers/media/video/em28xx/em28xx-cards.c +74 -78
224 lines not on this page · raw message
drivers/media/video/em28xx/em28xx-core.c +6 -16
@@ -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;
drivers/media/video/em28xx/em28xx-reg.h +5 -0
@@ -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
drivers/media/video/em28xx/em28xx-video.c +2 -1
@@ -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 ; }
drivers/media/video/em28xx/em28xx.h +2 -1
@@ -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 */
Quoted 11 lines · Holger Nelson
Quoted 7 lines · Mauro Carvalho Chehab
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
Quoted 103 lines
Holger
drivers/media/video/em28xx/em28xx-audio.c +1
-1@@ -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 ;drivers/media/video/em28xx/em28xx-cards.c +27
-37@@ -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),
drivers/media/video/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3158
Quoted 10 lines
drivers/media/video/em28xx/em28xx-cards.c +3
-7- 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.
Quoted 15 lines
drivers/media/video/em28xx/em28xx-cards.c +15
-0+ + 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 ;
drivers/media/video/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3182
Quoted 10 lines
drivers/media/video/em28xx/em28xx-cards.c +6
-1+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)) {Quoted 42 lines
drivers/media/video/em28xx/em28xx-cards.c +13
-8+ 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 ;
drivers/media/video/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3255
Quoted 10 lines
drivers/media/video/em28xx/em28xx-cards.c +2
-2- 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);
Quoted 31 lines
drivers/media/video/em28xx/em28xx-cards.c +0
-19dev->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];
drivers/media/video/em28xx/em28xx-cards.c static int em28xx_usb_probe(struct usb_interface *interface, · line 3282
Quoted 7 lines
drivers/media/video/em28xx/em28xx-cards.c +1
-3@@ -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;
Quoted 74 lines
drivers/media/video/em28xx/em28xx-cards.c +7
-1}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); } }drivers/media/video/em28xx/em28xx-core.c +5
-10@@ -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 ;
drivers/media/video/em28xx/em28xx-core.c int em28xx_isoc_dvb_max_packetsize(struct em28xx *dev) · line 1150
Quoted 10 lines
drivers/media/video/em28xx/em28xx-core.c +1
-6-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.
Quoted 51 lines
drivers/media/video/em28xx/em28xx-core.c +0
-0}return packet_size;drivers/media/video/em28xx/em28xx-reg.h +5
-0@@ -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 0x08drivers/media/video/em28xx/em28xx-video.c +2
-1@@ -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 ; }drivers/media/video/em28xx/em28xx.h +2
-1@@ -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 */ Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 1/1 em28xx: Reworked probe code to get rid of some hacks (was: Re: em28xx_isoc_dvb_max_packetsize for EM2884 (Terratec Cinergy HTC Stick))
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.
drivers/media/video/em28xx/em28xx-audio.c +1 -1
@@ -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 ;
drivers/media/video/em28xx/em28xx-cards.c +70 -76
212 lines not on this page · raw message
drivers/media/video/em28xx/em28xx-core.c +1 -57
70 lines not on this page · raw message
drivers/media/video/em28xx/em28xx-dvb.c +2 -2
@@ -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",
drivers/media/video/em28xx/em28xx-reg.h +5 -0
@@ -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
drivers/media/video/em28xx/em28xx.h +2 -0
@@ -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 */