Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Fri, Dec 08, 2017 at 03:57:30PM +0100, Greg KH wrote: > On Fri, Dec 08, 2017 at 02:44:12PM +, Michael Drake wrote: > > On 08/12/17 14:27, Greg KH wrote: > > > On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote: > > > > On 07/12/17 20:04, Greg KH wrote: > > > > > > > --- lsusb-v.orig 2017-12-07 21:01:26.153185002 +0100 > > > > > +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100 > > > > > @@ -1110,9 +1110,9 @@ > > > > >bDescriptorType36 > > > > >bDescriptorSubtype 1 (HEADER) > > > > >bcdADC 1.00 > > > > > -wTotalLength 0x0028 > > > > > +wTotalLength 40 > > > > [snip] > > > > > > Anyway, since it's a 2 byte field the hex representation > > > > is expected here, and the actual value is the same. > > > > > > Yes, the value is the same, but all other wTotalLength fields exported > > > by lsusb are in decimal. Changing just this one feels odd to me. > > > > Yes, that's a good reason for wanting to change the other ones. > > > > [snip] > > > > > I'll look at moving those other wFields to be also in 0x mode to > > > match up here. > > > > If you like I can work up a patch for that. I'd just be tweaking > > the old printfs to dump wFields with "0x%04x". > > > > I'm also thinking of porting more of the existing lsusb dumping > > to use the desc_dump() function, but that would have to be piece > > by piece, as I get time. > > I think moving more over to use desc_dump() is a better idea, let me see > if I can do one now to see how hard/easy it is... I've just fixed up the wTotalLength fields for now, I've run out of time to work on this. If you want to convert more of the descriptor fields, that would be wonderful. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Fri, Dec 08, 2017 at 02:44:12PM +, Michael Drake wrote: > On 08/12/17 14:27, Greg KH wrote: > > On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote: > > > On 07/12/17 20:04, Greg KH wrote: > > > > > --- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100 > > > > +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100 > > > > @@ -1110,9 +1110,9 @@ > > > >bDescriptorType36 > > > >bDescriptorSubtype 1 (HEADER) > > > >bcdADC 1.00 > > > > -wTotalLength 0x0028 > > > > +wTotalLength 40 > > [snip] > > > > Anyway, since it's a 2 byte field the hex representation > > > is expected here, and the actual value is the same. > > > > Yes, the value is the same, but all other wTotalLength fields exported > > by lsusb are in decimal. Changing just this one feels odd to me. > > Yes, that's a good reason for wanting to change the other ones. > > [snip] > > > I'll look at moving those other wFields to be also in 0x mode to > > match up here. > > If you like I can work up a patch for that. I'd just be tweaking > the old printfs to dump wFields with "0x%04x". > > I'm also thinking of porting more of the existing lsusb dumping > to use the desc_dump() function, but that would have to be piece > by piece, as I get time. I think moving more over to use desc_dump() is a better idea, let me see if I can do one now to see how hard/easy it is... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On 08/12/17 14:27, Greg KH wrote: On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote: On 07/12/17 20:04, Greg KH wrote: --- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100 +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100 @@ -1110,9 +1110,9 @@ bDescriptorType36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 -wTotalLength 0x0028 +wTotalLength 40 [snip] Anyway, since it's a 2 byte field the hex representation is expected here, and the actual value is the same. Yes, the value is the same, but all other wTotalLength fields exported by lsusb are in decimal. Changing just this one feels odd to me. Yes, that's a good reason for wanting to change the other ones. [snip] I'll look at moving those other wFields to be also in 0x mode to match up here. If you like I can work up a patch for that. I'd just be tweaking the old printfs to dump wFields with "0x%04x". I'm also thinking of porting more of the existing lsusb dumping to use the desc_dump() function, but that would have to be piece by piece, as I get time. Cheers, -- Michael Drake http://www.codethink.co.uk/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Fri, Dec 08, 2017 at 10:30:43AM +, Michael Drake wrote: > On 07/12/17 20:04, Greg KH wrote: > > On Thu, Dec 07, 2017 at 07:24:35PM +, Michael Drake wrote: > > > On 07/12/17 17:26, Greg KH wrote: > > > > On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote: > > > > > On 07/12/17 15:16, Greg KH wrote: > > > > > > On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: > > > > > > > Oops, I should have tested the code, it now crashes for me with > > > > > > > the > > > > > > > following error: > > > > > > > Floating point exception (core dumped) > > > > > > > > > > > > > > Do you see this as well? > > > > > > > > > > No, I don't see that. > > > > > > > > > > > And it's crashing on my USB audio device. Here's the output of it > > > > > > from > > > > > > the "old" lsusb output. > > > > > > > > > > [snip] > > > > > > > > > > Does it still crash with the warning fixes I posted? If so I'll > > > > > look in detail tomorrow. > > > > > > > > I will check when I get home tonight, I don't have the USB device on me > > > > at the moment. > > > > > > Thank you. I believe I've guessed the problem and sent a patch. > > > > Ok, that now works. > > > > But there is a difference in the "old" and "new" outputs, here's a diff > > of a full -v output of my laptop and a bunch of USB devices plugged in, > > showing the fields that now look different. > > > > The big one is the change from "streaming" to "control", is that > > correct? > > Replied inline below. > > > thanks, > > > > greg k-h > > > > > > --- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100 > > +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100 > > @@ -1110,9 +1110,9 @@ > > bDescriptorType36 > > bDescriptorSubtype 1 (HEADER) > > bcdADC 1.00 > > -wTotalLength 0x0028 > > +wTotalLength 40 > > I was a bit confused by this diff at first, because the > "-" lines are my version and the "+" lines are the old > version. Oops, you are right, sorry for getting them backwards. > Anyway, this is expected. For NUMBER type fields, my > code uses the heuristic that 1 byte fields are shown as > decimal and >1 byte is shown as full hexadecimal, with > leading zeros shown. This is consistent with the way > the old lsusb behaved in most cases, although it was > slightly inconsistent about it, and this is an example > of that inconsistency. > > Anyway, since it's a 2 byte field the hex representation > is expected here, and the actual value is the same. Yes, the value is the same, but all other wTotalLength fields exported by lsusb are in decimal. Changing just this one feels odd to me. > > bInCollection 1 > > -baInterfaceNr(0)1 > > +baInterfaceNr( 0) 1 > > Just a whitespace change. My version's not using > a fixed width for the array index, and only uses what's > needed for the largest index to be represented. Fair enough, I was worried things looked worse now :) > > AudioControl Interface Descriptor: > > bLength12 > > bDescriptorType36 > > @@ -1142,10 +1142,10 @@ > > bUnitID13 > > bSourceID 1 > > bControlSize1 > > -bmaControls(0) 0x01 > > +bmaControls( 0) 0x01 > > Mute Control > > -bmaControls(1) 0x00 > > -bmaControls(2) 0x00 > > +bmaControls( 1) 0x00 > > +bmaControls( 2) 0x00 > > Ditto for these. > > > iFeature0 > > Interface Descriptor: > > bLength 9 > > @@ -1173,7 +1173,7 @@ > > bDescriptorSubtype 1 (AS_GENERAL) > > bTerminalLink 1 > > bDelay 1 frames > > -wFormatTag 0x0001 PCM > > +wFormatTag 1 PCM > > This is the >1 byte shown as hexadecimal thing. Ok, consistancy is good, I can see that. > > AudioStreaming Interface Descriptor: > > bLength20 > > bDescriptorType36 > > @@ -1199,14 +1199,14 @@ > > bInterval 4 > > bRefresh0 > > bSynchAddress 133 > > -AudioStreaming Endpoint Descriptor: > > +AudioControl Endpoint Descriptor: > > This is from the dump_audiostreaming_endpoint() function. > The "AudioStreaming" string is correct. Looks like a typo > in the old lsusb output. Hey, bugfixes, nice! Best reason yet to take your patches :) I'll look at moving those other wFields to be also in 0x mode to match up here. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On 07/12/17 20:04, Greg KH wrote: On Thu, Dec 07, 2017 at 07:24:35PM +, Michael Drake wrote: On 07/12/17 17:26, Greg KH wrote: On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote: On 07/12/17 15:16, Greg KH wrote: On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: Oops, I should have tested the code, it now crashes for me with the following error: Floating point exception (core dumped) Do you see this as well? No, I don't see that. And it's crashing on my USB audio device. Here's the output of it from the "old" lsusb output. [snip] Does it still crash with the warning fixes I posted? If so I'll look in detail tomorrow. I will check when I get home tonight, I don't have the USB device on me at the moment. Thank you. I believe I've guessed the problem and sent a patch. Ok, that now works. But there is a difference in the "old" and "new" outputs, here's a diff of a full -v output of my laptop and a bunch of USB devices plugged in, showing the fields that now look different. The big one is the change from "streaming" to "control", is that correct? Replied inline below. thanks, greg k-h --- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100 +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100 @@ -1110,9 +1110,9 @@ bDescriptorType36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 -wTotalLength 0x0028 +wTotalLength 40 I was a bit confused by this diff at first, because the "-" lines are my version and the "+" lines are the old version. Anyway, this is expected. For NUMBER type fields, my code uses the heuristic that 1 byte fields are shown as decimal and >1 byte is shown as full hexadecimal, with leading zeros shown. This is consistent with the way the old lsusb behaved in most cases, although it was slightly inconsistent about it, and this is an example of that inconsistency. Anyway, since it's a 2 byte field the hex representation is expected here, and the actual value is the same. bInCollection 1 -baInterfaceNr(0)1 +baInterfaceNr( 0) 1 Just a whitespace change. My version's not using a fixed width for the array index, and only uses what's needed for the largest index to be represented. AudioControl Interface Descriptor: bLength12 bDescriptorType36 @@ -1142,10 +1142,10 @@ bUnitID13 bSourceID 1 bControlSize1 -bmaControls(0) 0x01 +bmaControls( 0) 0x01 Mute Control -bmaControls(1) 0x00 -bmaControls(2) 0x00 +bmaControls( 1) 0x00 +bmaControls( 2) 0x00 Ditto for these. iFeature0 Interface Descriptor: bLength 9 @@ -1173,7 +1173,7 @@ bDescriptorSubtype 1 (AS_GENERAL) bTerminalLink 1 bDelay 1 frames -wFormatTag 0x0001 PCM +wFormatTag 1 PCM This is the >1 byte shown as hexadecimal thing. AudioStreaming Interface Descriptor: bLength20 bDescriptorType36 @@ -1199,14 +1199,14 @@ bInterval 4 bRefresh0 bSynchAddress 133 -AudioStreaming Endpoint Descriptor: +AudioControl Endpoint Descriptor: This is from the dump_audiostreaming_endpoint() function. The "AudioStreaming" string is correct. Looks like a typo in the old lsusb output. bLength 7 bDescriptorType37 bDescriptorSubtype 1 (EP_GENERAL) bmAttributes 0x01 Sampling Frequency bLockDelayUnits 0 Undefined - wLockDelay 0x + wLockDelay 0 Undefined This and the remaining entries are similar to the above ones. Endpoint Descriptor: bLength 9 bDescriptorType 5 @@ -1235,7 +1235,7 @@ bDescriptorSubtype 1 (AS_GENERAL) bTerminalLink 1 bDelay 1 frames -wFormatTag 0x0001 PCM +wFormatTag 1 PCM AudioStreaming Interface Descriptor: bLength20 bDescriptorType36 @@ -1261,14 +1261,14 @@ bInterval 4 bRefresh0 bSynchAddress 133 -AudioStreaming Endpoint Descriptor: +AudioControl Endpoint Descriptor: bLength 7 bDescriptorType37 bDescriptorSubtype 1 (EP_GENERAL) bmAttributes 0x01 Sampling Frequency bLockDelayUnits 0
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Thu, Dec 07, 2017 at 07:24:35PM +, Michael Drake wrote: > On 07/12/17 17:26, Greg KH wrote: > > On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote: > > > On 07/12/17 15:16, Greg KH wrote: > > > > On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: > > > > > Oops, I should have tested the code, it now crashes for me with the > > > > > following error: > > > > > Floating point exception (core dumped) > > > > > > > > > > Do you see this as well? > > > > > > No, I don't see that. > > > > > > > And it's crashing on my USB audio device. Here's the output of it from > > > > the "old" lsusb output. > > > > > > [snip] > > > > > > Does it still crash with the warning fixes I posted? If so I'll > > > look in detail tomorrow. > > > > I will check when I get home tonight, I don't have the USB device on me > > at the moment. > > Thank you. I believe I've guessed the problem and sent a patch. Ok, that now works. But there is a difference in the "old" and "new" outputs, here's a diff of a full -v output of my laptop and a bunch of USB devices plugged in, showing the fields that now look different. The big one is the change from "streaming" to "control", is that correct? thanks, greg k-h --- lsusb-v.orig2017-12-07 21:01:26.153185002 +0100 +++ lsusb-v.new 2017-12-07 21:01:32.806517978 +0100 @@ -1110,9 +1110,9 @@ bDescriptorType36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 -wTotalLength 0x0028 +wTotalLength 40 bInCollection 1 -baInterfaceNr(0)1 +baInterfaceNr( 0) 1 AudioControl Interface Descriptor: bLength12 bDescriptorType36 @@ -1142,10 +1142,10 @@ bUnitID13 bSourceID 1 bControlSize1 -bmaControls(0) 0x01 +bmaControls( 0) 0x01 Mute Control -bmaControls(1) 0x00 -bmaControls(2) 0x00 +bmaControls( 1) 0x00 +bmaControls( 2) 0x00 iFeature0 Interface Descriptor: bLength 9 @@ -1173,7 +1173,7 @@ bDescriptorSubtype 1 (AS_GENERAL) bTerminalLink 1 bDelay 1 frames -wFormatTag 0x0001 PCM +wFormatTag 1 PCM AudioStreaming Interface Descriptor: bLength20 bDescriptorType36 @@ -1199,14 +1199,14 @@ bInterval 4 bRefresh0 bSynchAddress 133 -AudioStreaming Endpoint Descriptor: +AudioControl Endpoint Descriptor: bLength 7 bDescriptorType37 bDescriptorSubtype 1 (EP_GENERAL) bmAttributes 0x01 Sampling Frequency bLockDelayUnits 0 Undefined - wLockDelay 0x + wLockDelay 0 Undefined Endpoint Descriptor: bLength 9 bDescriptorType 5 @@ -1235,7 +1235,7 @@ bDescriptorSubtype 1 (AS_GENERAL) bTerminalLink 1 bDelay 1 frames -wFormatTag 0x0001 PCM +wFormatTag 1 PCM AudioStreaming Interface Descriptor: bLength20 bDescriptorType36 @@ -1261,14 +1261,14 @@ bInterval 4 bRefresh0 bSynchAddress 133 -AudioStreaming Endpoint Descriptor: +AudioControl Endpoint Descriptor: bLength 7 bDescriptorType37 bDescriptorSubtype 1 (EP_GENERAL) bmAttributes 0x01 Sampling Frequency bLockDelayUnits 0 Undefined - wLockDelay 0x + wLockDelay 0 Undefined Endpoint Descriptor: bLength 9 bDescriptorType 5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On 07/12/17 17:26, Greg KH wrote: On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote: On 07/12/17 15:16, Greg KH wrote: On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: Oops, I should have tested the code, it now crashes for me with the following error: Floating point exception (core dumped) Do you see this as well? No, I don't see that. And it's crashing on my USB audio device. Here's the output of it from the "old" lsusb output. [snip] Does it still crash with the warning fixes I posted? If so I'll look in detail tomorrow. I will check when I get home tonight, I don't have the USB device on me at the moment. Thank you. I believe I've guessed the problem and sent a patch. Cheers, -- Michael Drake http://www.codethink.co.uk/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Thu, Dec 07, 2017 at 06:26:30PM +0100, Greg KH wrote: > On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote: > > > > > > On 07/12/17 15:16, Greg KH wrote: > > > On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: > > > > On Thu, Dec 07, 2017 at 04:00:36PM +0100, Greg KH wrote: > > > > > On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: > > > > > > This adds a new way of dumping descriptors, which splits the > > > > > > knowledge > > > > > > of how to interpret descriptor data from the actual dumping. This > > > > > > has > > > > > > two advantages: > > > > > > > > > > > > 1. It is easy to add support for new descriptors, since they are now > > > > > > simple definitions that resemble the tables in the USB > > > > > > specifications. > > > > > > > > > > > > 2. The code for dumping descriptors is common, so the output is > > > > > > easy to > > > > > > keep consistent. It is also consistent and thorough in its > > > > > > handling > > > > > > of insufficient descriptor data buffer, and junk data at the > > > > > > end of > > > > > > a descriptor. > > > > > > > > > > > > UAC1 and UAC2 are converted to use the new mechanism, initial > > > > > > support > > > > > > for UAC3 is added. Finally, support for the USB3 BOS Configuration > > > > > > Summary Descriptor is added. > > > > > > > > > > > > This was previously opened as a github pull request here: > > > > > > > > > > > > https://github.com/gregkh/usbutils/pull/61 > > > > > > > > > > Thanks for this, all of the patches are now applied. > > > > > > > > > > There were some intermediate build warnings, but future patches in the > > > > > series fixed that up, next time be more careful, each patch should be > > > > > "clean". > > > > > > > > > > However the build now gets the following warnings: > > > > > > > > > >CC lsusb-lsusb.o > > > > > lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not > > > > > used [-Wunused-function] > > > > > static int get_audioterminal_string(char *buf, size_t size, > > > > > u_int16_t termt) > > > > > ^~~~ > > > > >CC lsusb-lsusb-t.o > > > > >CC lsusb-desc-defs.o > > > > >CC lsusb-desc-dump.o > > > > > desc-dump.c: In function ‘desc_bmcontrol_dump’: > > > > > desc-dump.c:67:18: warning: comparison between pointer and zero > > > > > character constant [-Wpointer-compare] > > > > > if (strings[0] != '\0') { > > > > >^~ > > > > > desc-dump.c:67:7: note: did you mean to dereference the pointer? > > > > > if (strings[0] != '\0') { > > > > > ^ > > > > >CC lsusb-names.o > > > > > > > > > > > > > > > Can you fix this up and send a patch for them? > > > > > > > > Oops, I should have tested the code, it now crashes for me with the > > > > following error: > > > > Floating point exception (core dumped) > > > > > > > > Do you see this as well? > > > > No, I don't see that. > > > > > And it's crashing on my USB audio device. Here's the output of it from > > > the "old" lsusb output. > > > > [snip] > > > > Does it still crash with the warning fixes I posted? If so I'll > > look in detail tomorrow. > > I will check when I get home tonight, I don't have the USB device on me > at the moment. Nope, same failure :( I'll try to debug it tomorrow if you can't think of anything... greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Thu, Dec 07, 2017 at 05:14:10PM +, Michael Drake wrote: > > > On 07/12/17 15:16, Greg KH wrote: > > On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: > > > On Thu, Dec 07, 2017 at 04:00:36PM +0100, Greg KH wrote: > > > > On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: > > > > > This adds a new way of dumping descriptors, which splits the knowledge > > > > > of how to interpret descriptor data from the actual dumping. This has > > > > > two advantages: > > > > > > > > > > 1. It is easy to add support for new descriptors, since they are now > > > > > simple definitions that resemble the tables in the USB > > > > > specifications. > > > > > > > > > > 2. The code for dumping descriptors is common, so the output is easy > > > > > to > > > > > keep consistent. It is also consistent and thorough in its > > > > > handling > > > > > of insufficient descriptor data buffer, and junk data at the end > > > > > of > > > > > a descriptor. > > > > > > > > > > UAC1 and UAC2 are converted to use the new mechanism, initial support > > > > > for UAC3 is added. Finally, support for the USB3 BOS Configuration > > > > > Summary Descriptor is added. > > > > > > > > > > This was previously opened as a github pull request here: > > > > > > > > > > https://github.com/gregkh/usbutils/pull/61 > > > > > > > > Thanks for this, all of the patches are now applied. > > > > > > > > There were some intermediate build warnings, but future patches in the > > > > series fixed that up, next time be more careful, each patch should be > > > > "clean". > > > > > > > > However the build now gets the following warnings: > > > > > > > >CC lsusb-lsusb.o > > > > lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not > > > > used [-Wunused-function] > > > > static int get_audioterminal_string(char *buf, size_t size, u_int16_t > > > > termt) > > > > ^~~~ > > > >CC lsusb-lsusb-t.o > > > >CC lsusb-desc-defs.o > > > >CC lsusb-desc-dump.o > > > > desc-dump.c: In function ‘desc_bmcontrol_dump’: > > > > desc-dump.c:67:18: warning: comparison between pointer and zero > > > > character constant [-Wpointer-compare] > > > > if (strings[0] != '\0') { > > > >^~ > > > > desc-dump.c:67:7: note: did you mean to dereference the pointer? > > > > if (strings[0] != '\0') { > > > > ^ > > > >CC lsusb-names.o > > > > > > > > > > > > Can you fix this up and send a patch for them? > > > > > > Oops, I should have tested the code, it now crashes for me with the > > > following error: > > > Floating point exception (core dumped) > > > > > > Do you see this as well? > > No, I don't see that. > > > And it's crashing on my USB audio device. Here's the output of it from > > the "old" lsusb output. > > [snip] > > Does it still crash with the warning fixes I posted? If so I'll > look in detail tomorrow. I will check when I get home tonight, I don't have the USB device on me at the moment. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Thu, Dec 07, 2017 at 05:11:58PM +, Michael Drake wrote: > > > On 07/12/17 15:00, Greg KH wrote: > > On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: > > > This adds a new way of dumping descriptors, which splits the knowledge > > > of how to interpret descriptor data from the actual dumping. This has > > > two advantages: > > > > > > 1. It is easy to add support for new descriptors, since they are now > > > simple definitions that resemble the tables in the USB specifications. > > > > > > 2. The code for dumping descriptors is common, so the output is easy to > > > keep consistent. It is also consistent and thorough in its handling > > > of insufficient descriptor data buffer, and junk data at the end of > > > a descriptor. > > > > > > UAC1 and UAC2 are converted to use the new mechanism, initial support > > > for UAC3 is added. Finally, support for the USB3 BOS Configuration > > > Summary Descriptor is added. > > > > > > This was previously opened as a github pull request here: > > > > > > https://github.com/gregkh/usbutils/pull/61 > > > > Thanks for this, all of the patches are now applied. > > > > There were some intermediate build warnings, but future patches in the > > series fixed that up, next time be more careful, each patch should be > > "clean". > > > > However the build now gets the following warnings: > > > >CC lsusb-lsusb.o > > lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not used > > [-Wunused-function] > > static int get_audioterminal_string(char *buf, size_t size, u_int16_t > > termt) > > ^~~~ > >CC lsusb-lsusb-t.o > >CC lsusb-desc-defs.o > >CC lsusb-desc-dump.o > > desc-dump.c: In function ‘desc_bmcontrol_dump’: > > desc-dump.c:67:18: warning: comparison between pointer and zero character > > constant [-Wpointer-compare] > > if (strings[0] != '\0') { > >^~ > > desc-dump.c:67:7: note: did you mean to dereference the pointer? > > if (strings[0] != '\0') { > > ^ > >CC lsusb-names.o > > > > > > Can you fix this up and send a patch for them? > > Done. > > Oddly I didn't see the warnings with the default `make`. I hacked > "Makefile.am" locally to add "-Wall -Wextra -pedantic -O2" to the > CCFLAGS, and I saw the one about the unused get_audioterminal_string() > function. However I still didn't see the other. > > (Using gcc (Debian 6.3.0-18) 6.3.0 20170516) It's your version of gcc, you need a more modern one: $ gcc --version gcc (GCC) 7.2.0 thanks for the fixups. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On 07/12/17 15:16, Greg KH wrote: On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: On Thu, Dec 07, 2017 at 04:00:36PM +0100, Greg KH wrote: On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: This adds a new way of dumping descriptors, which splits the knowledge of how to interpret descriptor data from the actual dumping. This has two advantages: 1. It is easy to add support for new descriptors, since they are now simple definitions that resemble the tables in the USB specifications. 2. The code for dumping descriptors is common, so the output is easy to keep consistent. It is also consistent and thorough in its handling of insufficient descriptor data buffer, and junk data at the end of a descriptor. UAC1 and UAC2 are converted to use the new mechanism, initial support for UAC3 is added. Finally, support for the USB3 BOS Configuration Summary Descriptor is added. This was previously opened as a github pull request here: https://github.com/gregkh/usbutils/pull/61 Thanks for this, all of the patches are now applied. There were some intermediate build warnings, but future patches in the series fixed that up, next time be more careful, each patch should be "clean". However the build now gets the following warnings: CC lsusb-lsusb.o lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not used [-Wunused-function] static int get_audioterminal_string(char *buf, size_t size, u_int16_t termt) ^~~~ CC lsusb-lsusb-t.o CC lsusb-desc-defs.o CC lsusb-desc-dump.o desc-dump.c: In function ‘desc_bmcontrol_dump’: desc-dump.c:67:18: warning: comparison between pointer and zero character constant [-Wpointer-compare] if (strings[0] != '\0') { ^~ desc-dump.c:67:7: note: did you mean to dereference the pointer? if (strings[0] != '\0') { ^ CC lsusb-names.o Can you fix this up and send a patch for them? Oops, I should have tested the code, it now crashes for me with the following error: Floating point exception (core dumped) Do you see this as well? No, I don't see that. And it's crashing on my USB audio device. Here's the output of it from the "old" lsusb output. [snip] Does it still crash with the warning fixes I posted? If so I'll look in detail tomorrow. Cheers, -- Michael Drake http://www.codethink.co.uk/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On 07/12/17 15:00, Greg KH wrote: On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: This adds a new way of dumping descriptors, which splits the knowledge of how to interpret descriptor data from the actual dumping. This has two advantages: 1. It is easy to add support for new descriptors, since they are now simple definitions that resemble the tables in the USB specifications. 2. The code for dumping descriptors is common, so the output is easy to keep consistent. It is also consistent and thorough in its handling of insufficient descriptor data buffer, and junk data at the end of a descriptor. UAC1 and UAC2 are converted to use the new mechanism, initial support for UAC3 is added. Finally, support for the USB3 BOS Configuration Summary Descriptor is added. This was previously opened as a github pull request here: https://github.com/gregkh/usbutils/pull/61 Thanks for this, all of the patches are now applied. There were some intermediate build warnings, but future patches in the series fixed that up, next time be more careful, each patch should be "clean". However the build now gets the following warnings: CC lsusb-lsusb.o lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not used [-Wunused-function] static int get_audioterminal_string(char *buf, size_t size, u_int16_t termt) ^~~~ CC lsusb-lsusb-t.o CC lsusb-desc-defs.o CC lsusb-desc-dump.o desc-dump.c: In function ‘desc_bmcontrol_dump’: desc-dump.c:67:18: warning: comparison between pointer and zero character constant [-Wpointer-compare] if (strings[0] != '\0') { ^~ desc-dump.c:67:7: note: did you mean to dereference the pointer? if (strings[0] != '\0') { ^ CC lsusb-names.o Can you fix this up and send a patch for them? Done. Oddly I didn't see the warnings with the default `make`. I hacked "Makefile.am" locally to add "-Wall -Wextra -pedantic -O2" to the CCFLAGS, and I saw the one about the unused get_audioterminal_string() function. However I still didn't see the other. (Using gcc (Debian 6.3.0-18) 6.3.0 20170516) Cheers, -- Michael Drake http://www.codethink.co.uk/ -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Thu, Dec 07, 2017 at 04:01:23PM +0100, Greg KH wrote: > On Thu, Dec 07, 2017 at 04:00:36PM +0100, Greg KH wrote: > > On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: > > > This adds a new way of dumping descriptors, which splits the knowledge > > > of how to interpret descriptor data from the actual dumping. This has > > > two advantages: > > > > > > 1. It is easy to add support for new descriptors, since they are now > > >simple definitions that resemble the tables in the USB specifications. > > > > > > 2. The code for dumping descriptors is common, so the output is easy to > > >keep consistent. It is also consistent and thorough in its handling > > >of insufficient descriptor data buffer, and junk data at the end of > > >a descriptor. > > > > > > UAC1 and UAC2 are converted to use the new mechanism, initial support > > > for UAC3 is added. Finally, support for the USB3 BOS Configuration > > > Summary Descriptor is added. > > > > > > This was previously opened as a github pull request here: > > > > > > https://github.com/gregkh/usbutils/pull/61 > > > > Thanks for this, all of the patches are now applied. > > > > There were some intermediate build warnings, but future patches in the > > series fixed that up, next time be more careful, each patch should be > > "clean". > > > > However the build now gets the following warnings: > > > > CC lsusb-lsusb.o > > lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not used > > [-Wunused-function] > > static int get_audioterminal_string(char *buf, size_t size, u_int16_t > > termt) > > ^~~~ > > CC lsusb-lsusb-t.o > > CC lsusb-desc-defs.o > > CC lsusb-desc-dump.o > > desc-dump.c: In function ‘desc_bmcontrol_dump’: > > desc-dump.c:67:18: warning: comparison between pointer and zero character > > constant [-Wpointer-compare] > >if (strings[0] != '\0') { > > ^~ > > desc-dump.c:67:7: note: did you mean to dereference the pointer? > >if (strings[0] != '\0') { > >^ > > CC lsusb-names.o > > > > > > Can you fix this up and send a patch for them? > > Oops, I should have tested the code, it now crashes for me with the > following error: > Floating point exception (core dumped) > > Do you see this as well? And it's crashing on my USB audio device. Here's the output of it from the "old" lsusb output. Bus 001 Device 018: ID 0d8c:1066 C-Media Electronics, Inc. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x0d8c C-Media Electronics, Inc. idProduct 0x1066 bcdDevice1.03 iManufacturer 1 iProduct2 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 214 bNumInterfaces 3 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xc0 Self Powered MaxPower0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 0 bInterfaceClass 1 Audio bInterfaceSubClass 1 Control Device bInterfaceProtocol 0 iInterface 2 AudioControl Interface Descriptor: bLength 9 bDescriptorType36 bDescriptorSubtype 1 (HEADER) bcdADC 1.00 wTotalLength 40 bInCollection 1 baInterfaceNr( 0) 1 AudioControl Interface Descriptor: bLength12 bDescriptorType36 bDescriptorSubtype 2 (INPUT_TERMINAL) bTerminalID 1 wTerminalType 0x0101 USB Streaming bAssocTerminal 0 bNrChannels 2 wChannelConfig 0x0003 Left Front (L) Right Front (R) iChannelNames 0 iTerminal 0 AudioControl Interface Descriptor: bLength 9 bDescriptorType36 bDescriptorSubtype 3 (OUTPUT_TERMINAL) bTerminalID 7 wTerminalType 0x0301 Speaker bAssocTerminal 0 bSourceID 13 iTerminal 0 AudioControl Interface Descriptor: bLength10 bDescriptorType36 bDescriptorSubtype 6 (FEATURE_UNIT) bUnitID13 bSourceID 1 bControlSize1 bmaControls( 0) 0x01 Mute Control
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Thu, Dec 07, 2017 at 04:00:36PM +0100, Greg KH wrote: > On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: > > This adds a new way of dumping descriptors, which splits the knowledge > > of how to interpret descriptor data from the actual dumping. This has > > two advantages: > > > > 1. It is easy to add support for new descriptors, since they are now > >simple definitions that resemble the tables in the USB specifications. > > > > 2. The code for dumping descriptors is common, so the output is easy to > >keep consistent. It is also consistent and thorough in its handling > >of insufficient descriptor data buffer, and junk data at the end of > >a descriptor. > > > > UAC1 and UAC2 are converted to use the new mechanism, initial support > > for UAC3 is added. Finally, support for the USB3 BOS Configuration > > Summary Descriptor is added. > > > > This was previously opened as a github pull request here: > > > > https://github.com/gregkh/usbutils/pull/61 > > Thanks for this, all of the patches are now applied. > > There were some intermediate build warnings, but future patches in the > series fixed that up, next time be more careful, each patch should be > "clean". > > However the build now gets the following warnings: > > CC lsusb-lsusb.o > lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not used > [-Wunused-function] > static int get_audioterminal_string(char *buf, size_t size, u_int16_t termt) > ^~~~ > CC lsusb-lsusb-t.o > CC lsusb-desc-defs.o > CC lsusb-desc-dump.o > desc-dump.c: In function ‘desc_bmcontrol_dump’: > desc-dump.c:67:18: warning: comparison between pointer and zero character > constant [-Wpointer-compare] >if (strings[0] != '\0') { > ^~ > desc-dump.c:67:7: note: did you mean to dereference the pointer? >if (strings[0] != '\0') { >^ > CC lsusb-names.o > > > Can you fix this up and send a patch for them? Oops, I should have tested the code, it now crashes for me with the following error: Floating point exception (core dumped) Do you see this as well? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3
On Thu, Dec 07, 2017 at 10:26:21AM +, Michael Drake wrote: > This adds a new way of dumping descriptors, which splits the knowledge > of how to interpret descriptor data from the actual dumping. This has > two advantages: > > 1. It is easy to add support for new descriptors, since they are now >simple definitions that resemble the tables in the USB specifications. > > 2. The code for dumping descriptors is common, so the output is easy to >keep consistent. It is also consistent and thorough in its handling >of insufficient descriptor data buffer, and junk data at the end of >a descriptor. > > UAC1 and UAC2 are converted to use the new mechanism, initial support > for UAC3 is added. Finally, support for the USB3 BOS Configuration > Summary Descriptor is added. > > This was previously opened as a github pull request here: > > https://github.com/gregkh/usbutils/pull/61 Thanks for this, all of the patches are now applied. There were some intermediate build warnings, but future patches in the series fixed that up, next time be more careful, each patch should be "clean". However the build now gets the following warnings: CC lsusb-lsusb.o lsusb.c:220:12: warning: ‘get_audioterminal_string’ defined but not used [-Wunused-function] static int get_audioterminal_string(char *buf, size_t size, u_int16_t termt) ^~~~ CC lsusb-lsusb-t.o CC lsusb-desc-defs.o CC lsusb-desc-dump.o desc-dump.c: In function ‘desc_bmcontrol_dump’: desc-dump.c:67:18: warning: comparison between pointer and zero character constant [-Wpointer-compare] if (strings[0] != '\0') { ^~ desc-dump.c:67:7: note: did you mean to dereference the pointer? if (strings[0] != '\0') { ^ CC lsusb-names.o Can you fix this up and send a patch for them? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html