Re: [PATCH v2 0/8] lsusb: Add initial support for USB Audio Class 3

2017-12-08 Thread Greg KH
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

2017-12-08 Thread Greg KH
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

2017-12-08 Thread Michael Drake

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

2017-12-08 Thread Greg KH
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

2017-12-08 Thread Michael Drake

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

2017-12-07 Thread Greg KH
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

2017-12-07 Thread Michael Drake

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

2017-12-07 Thread Greg KH
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

2017-12-07 Thread Greg KH
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

2017-12-07 Thread Greg KH
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

2017-12-07 Thread Michael Drake



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

2017-12-07 Thread Michael Drake



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

2017-12-07 Thread Greg KH
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

2017-12-07 Thread Greg KH
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

2017-12-07 Thread Greg KH
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