Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* Unable to set "iInterface" in usb gadget via configfs
@ 2020-01-14 23:58 Andrew P. Lentvorski
  2020-01-15 15:14 ` Alan Stern
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew P. Lentvorski @ 2020-01-14 23:58 UTC (permalink / raw)
  To: linux-usb

I've been trying to report what I think is a bug, but I can't seem to
get through to the mailing list.  If these are coming through
duplicated, please let me know so I can quit sending them.

Thanks,
-a


> I've been trying to set "iInterface" in my usb gadget to a specific string, but I simply can't find a way to make configfs accept this.
> 
> When I set my gadget up on my Beaglebone Black (uname -a: Linux beaglebone 4.14.108-ti-r113 #1 SMP PREEMPT Wed Jul 31 00:01:10 UTC 2019 armv7l GNU/Linux).
> 
> I get (output from lsusb):
> 
> iInterface 5 HID Interface
> 
> 
> But I want it to be something like:
> 
> iInterface 4 LPC-LINK2 CMSIS-DAP V5.224
> 
> 
> This seems to be wired up as a fixed value in f_hid.c and doesn't seem to have a corresponding way to actually change it via configfs.
> 
> 
> #define CT_FUNC_HID_IDX 0
> 
> static struct usb_string ct_func_string_defs[] = {
>         [CT_FUNC_HID_IDX].s     = "HID Interface",
>         {},                     /* end of list */
> };
>  
> 
> Thanks,
> 
> -a
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Unable to set "iInterface" in usb gadget via configfs
  2020-01-14 23:58 Unable to set "iInterface" in usb gadget via configfs Andrew P. Lentvorski
@ 2020-01-15 15:14 ` Alan Stern
  2020-01-16  2:23   ` Andrew P. Lentvorski
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Stern @ 2020-01-15 15:14 UTC (permalink / raw)
  To: Andrew P. Lentvorski; +Cc: linux-usb

On Tue, 14 Jan 2020, Andrew P. Lentvorski wrote:

> I've been trying to report what I think is a bug, but I can't seem to
> get through to the mailing list.  If these are coming through
> duplicated, please let me know so I can quit sending them.

I don't think any earlier messages in this thread made it through the 
mailing list, but this one definitely did.

> Thanks,
> -a
> 
> 
> > I've been trying to set "iInterface" in my usb gadget to a specific string, but I simply can't find a way to make configfs accept this.
> > 
> > When I set my gadget up on my Beaglebone Black (uname -a: Linux beaglebone 4.14.108-ti-r113 #1 SMP PREEMPT Wed Jul 31 00:01:10 UTC 2019 armv7l GNU/Linux).
> > 
> > I get (output from lsusb):
> > 
> > iInterface 5 HID Interface
> > 
> > 
> > But I want it to be something like:
> > 
> > iInterface 4 LPC-LINK2 CMSIS-DAP V5.224
> > 
> > 
> > This seems to be wired up as a fixed value in f_hid.c and doesn't seem to have a corresponding way to actually change it via configfs.
> > 
> > 
> > #define CT_FUNC_HID_IDX 0
> > 
> > static struct usb_string ct_func_string_defs[] = {
> >         [CT_FUNC_HID_IDX].s     = "HID Interface",
> >         {},                     /* end of list */
> > };

Then maybe you need to fix f_hid.c.  Or maybe configfs isn't meant to
allow the user to specify these string index values (I don't know any
of the configfs details).

Alan Stern


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Unable to set "iInterface" in usb gadget via configfs
  2020-01-15 15:14 ` Alan Stern
@ 2020-01-16  2:23   ` Andrew P. Lentvorski
  2020-01-16 13:02     ` Felipe Balbi
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew P. Lentvorski @ 2020-01-16  2:23 UTC (permalink / raw)
  To: Alan Stern; +Cc: linux-usb

On 1/15/20 7:14 AM, Alan Stern wrote:

> I don't think any earlier messages in this thread made it through the 
> mailing list, but this one definitely did.

Yay!  I also saw this one in the archives so I was hopeful.

>>> I've been trying to set "iInterface" in my usb gadget to a specific string, but I simply can't find a way to make configfs accept this.
>>>
>>> When I set my gadget up on my Beaglebone Black (uname -a: Linux beaglebone 4.14.108-ti-r113 #1 SMP PREEMPT Wed Jul 31 00:01:10 UTC 2019 armv7l GNU/Linux).
>>>
>>> I get (output from lsusb):
>>>
>>> iInterface 5 HID Interface
>>>
>>>
>>> But I want it to be something like:
>>>
>>> iInterface 4 LPC-LINK2 CMSIS-DAP V5.224
>>>
>>>
>>> This seems to be wired up as a fixed value in f_hid.c and doesn't seem to have a corresponding way to actually change it via configfs.
>>>
>>>
>>> #define CT_FUNC_HID_IDX 0
>>>
>>> static struct usb_string ct_func_string_defs[] = {
>>>         [CT_FUNC_HID_IDX].s     = "HID Interface",
>>>         {},                     /* end of list */
>>> };
> 
> Then maybe you need to fix f_hid.c.  Or maybe configfs isn't meant to
> allow the user to specify these string index values (I don't know any
> of the configfs details).

That's kind of my problem in that I was hoping to get someone far more
knowledegable than me to at least flag these before attacking it:

A) I didn't overlook something stupid and this really is hardwired with
no way to change it (either in configfs or ... some other? kernel mechanism)

B) This is an *actual* bug.

C) This is an actual bug *that should be fixed* and isn't that way
intentionally for some Linux reason.

D) This is actually the right place to fix it.  Obviously there is going
to be something at the configfs level, too, and I have *zero* idea where
to start looking for that.

Thanks,
-a


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Unable to set "iInterface" in usb gadget via configfs
  2020-01-16  2:23   ` Andrew P. Lentvorski
@ 2020-01-16 13:02     ` Felipe Balbi
  2020-01-17  0:39       ` Andrew P. Lentvorski
  0 siblings, 1 reply; 8+ messages in thread
From: Felipe Balbi @ 2020-01-16 13:02 UTC (permalink / raw)
  To: Andrew P. Lentvorski, Alan Stern; +Cc: linux-usb

[-- Attachment #1: Type: text/plain, Size: 2405 bytes --]


hi,

"Andrew P. Lentvorski" <bsder@allcaps.org> writes:
> On 1/15/20 7:14 AM, Alan Stern wrote:
>
>> I don't think any earlier messages in this thread made it through the 
>> mailing list, but this one definitely did.
>
> Yay!  I also saw this one in the archives so I was hopeful.
>
>>>> I've been trying to set "iInterface" in my usb gadget to a specific
>>>> string, but I simply can't find a way to make configfs accept this.

iInterface is not a string. It's the index to a string descriptor table.

>>>> When I set my gadget up on my Beaglebone Black (uname -a: Linux
>>>> beaglebone 4.14.108-ti-r113 #1 SMP PREEMPT Wed Jul 31 00:01:10 UTC
>>>> 2019 armv7l GNU/Linux).
>>>>
>>>> I get (output from lsusb):
>>>>
>>>> iInterface 5 HID Interface
>>>>
>>>>
>>>> But I want it to be something like:
>>>>
>>>> iInterface 4 LPC-LINK2 CMSIS-DAP V5.224

Why? Oh, you want your beaglebone to behave as a CMSIS-DAP to trick some
other SW?

Do we need to support that upstream, though? It seems like this is a
one-off thing. Does anybody else need to configure interface string
descriptor?

>> Then maybe you need to fix f_hid.c.  Or maybe configfs isn't meant to
>> allow the user to specify these string index values (I don't know any
>> of the configfs details).
>
> That's kind of my problem in that I was hoping to get someone far more
> knowledegable than me to at least flag these before attacking it:
>
> A) I didn't overlook something stupid and this really is hardwired with
> no way to change it (either in configfs or ... some other? kernel mechanism)

yes, it really is hardwired

> B) This is an *actual* bug.

Not a bug, just was never a requirement.

> C) This is an actual bug *that should be fixed* and isn't that way
> intentionally for some Linux reason.

Up until now, it has been intentional. Currently, I don't see a need to
change it. I may be persuaded otherwise, but I need to see arguments
other than "I want to trick some SW into thinking I'm something else".

> D) This is actually the right place to fix it.  Obviously there is going
> to be something at the configfs level, too, and I have *zero* idea where
> to start looking for that.

drivers/usb/gadget/configfs.c, look for GS_STRINGS_RW(). Note, also,
that you should *not* allow for strings to be changed after the device
has been enumerated.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Unable to set "iInterface" in usb gadget via configfs
  2020-01-16 13:02     ` Felipe Balbi
@ 2020-01-17  0:39       ` Andrew P. Lentvorski
  2020-01-17  9:25         ` Felipe Balbi
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew P. Lentvorski @ 2020-01-17  0:39 UTC (permalink / raw)
  To: Felipe Balbi, Alan Stern; +Cc: linux-usb

On 1/16/20 5:02 AM, Felipe Balbi wrote:

>>>>> When I set my gadget up on my Beaglebone Black (uname -a: Linux
>>>>> beaglebone 4.14.108-ti-r113 #1 SMP PREEMPT Wed Jul 31 00:01:10 UTC
>>>>> 2019 armv7l GNU/Linux).
>>>>>
>>>>> I get (output from lsusb):
>>>>>
>>>>> iInterface 5 HID Interface
>>>>>
>>>>>
>>>>> But I want it to be something like:
>>>>>
>>>>> iInterface 4 LPC-LINK2 CMSIS-DAP V5.224
> 
> Why? Oh, you want your beaglebone to behave as a CMSIS-DAP to trick some
> other SW?
> 
> Do we need to support that upstream, though? It seems like this is a
> one-off thing. Does anybody else need to configure interface string
> descriptor?

I ... don't know, actually.  That's probably a good question.

What I *can* tell you is that a quick survey of the various USB devices
in my office with lsusb shows the majority of iInterface are 0x00, but
that everything else is a smattering of all manner of stuff: "Integrated
Camera", "Bulk Control Interface", "Volume Control", "Firmware Upgrade
Port", etc.

Microchip's PIC32 USB HID sample code sets the value to 0x00.

USB Complete (5th Edition) also shows this as 0x00 on page 267 for a
configuration_descriptor for a vendor-specific HID.


>> C) This is an actual bug *that should be fixed* and isn't that way
>> intentionally for some Linux reason.
> 
> Up until now, it has been intentional. Currently, I don't see a need to
> change it. 

Clearly, some HID devices *are* using this field.  So, this probably
should be made changeable if we want the HID gadget to be maximally useful.

As for the technical details I can see from the code:


A bit of code archaeology shows that .iInterface was expected to be
dynamic as of the origin commit for f_hid.c (commit
71adf118946957839a13aa4d1094183e05c6c094).  This still persists in the
file today.


The allocation of usb_gstrings_attach comes in at Nov 6, 2014 as of
(commit 5ca8d3ec9970f4798e68bd21a9d44db3d0ff4da7) with the message:

"Before configfs is integrated the usb_gstrings_attach() interface
must be used."

This looks like a case of something that simply got lost in the shuffle
in the upgrade path to configfs, and I'm just the first poor slob who
tripped over the oversight.


However, I'm a little concerned as to why this was set *at all* (which
seems to be what is actually tripping me up), and I'd be interested as
to what drove the specific choice of 0x05 "HID Interface".  I don't see
this in any spec anywhere obvious and this seems to be a deliberate
change from previous behavior.

If this isn't user configurable, then it's probably a highly
questionable choice to give this any default value other than 0x00.
This would have been quite a bit easier to code, too, so I'm *really*
scratching my head about this.


> I may be persuaded otherwise, but I need to see arguments
> other than "I want to trick some SW into thinking I'm something else".

Obviously I'm biased and scratching my own itch, but why would you
consider that not to be an important argument?

Being able to fake being a particular piece of USB hardware that's
currently tied to some ancient Windows binary driver is a great way to
insert Linux into industrial and lab control pipelines.  Giving a
control board the ability to now also be accessed via ethernet or
wireless (or even a better USB protocol) and thus now has an upgrade or
higher performance path is a *really* useful thing.  And the Beaglebone
Black is a really good "protocol engine".  Finally, after making the usb
gadget emulation work, I can probably blow a bunch of Windows machines
away completely since something like a Beaglebone Black is more than
sufficient to handle the control without any outside intervention.

My end target isn't "CMSIS-DAP"--there are a zillion really cheap
CMSIS-DAP devices so emulating CMSIS-DAP would be mostly pointless
pedantry (although perhaps a good usb gadget tutorial).  CMSIS-DAP just
happens to be a really good test case as it is an extremely well
documented HID-based protocol and has extensive tests to validate it.
Consequently, I can be sure that most bugs and difficulties are
localized to my gadget implementation code or the gadget driver.



Anyhow, let me know whether I should attack the problem or not.  I
suspect my biggest issue is simply that I will have to do all the work
on a Beaglebone Black as I really don't know of any other way to test
that gadget subsystem.  This will probably be okay if I can build it as
a module--if I have to do a full kernel build it will probably demand
that I set up a cross-compiling environment (and that isn't trivial).


Thanks,
-a

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Unable to set "iInterface" in usb gadget via configfs
  2020-01-17  0:39       ` Andrew P. Lentvorski
@ 2020-01-17  9:25         ` Felipe Balbi
  2020-01-18  0:58           ` Andrew P. Lentvorski
  0 siblings, 1 reply; 8+ messages in thread
From: Felipe Balbi @ 2020-01-17  9:25 UTC (permalink / raw)
  To: Andrew P. Lentvorski, Alan Stern; +Cc: linux-usb

[-- Attachment #1: Type: text/plain, Size: 7353 bytes --]


Hi,

"Andrew P. Lentvorski" <bsder@allcaps.org> writes:
>>>>>> When I set my gadget up on my Beaglebone Black (uname -a: Linux
>>>>>> beaglebone 4.14.108-ti-r113 #1 SMP PREEMPT Wed Jul 31 00:01:10 UTC
>>>>>> 2019 armv7l GNU/Linux).
>>>>>>
>>>>>> I get (output from lsusb):
>>>>>>
>>>>>> iInterface 5 HID Interface
>>>>>>
>>>>>>
>>>>>> But I want it to be something like:
>>>>>>
>>>>>> iInterface 4 LPC-LINK2 CMSIS-DAP V5.224
>> 
>> Why? Oh, you want your beaglebone to behave as a CMSIS-DAP to trick some
>> other SW?
>> 
>> Do we need to support that upstream, though? It seems like this is a
>> one-off thing. Does anybody else need to configure interface string
>> descriptor?
>
> I ... don't know, actually.  That's probably a good question.
>
> What I *can* tell you is that a quick survey of the various USB devices
> in my office with lsusb shows the majority of iInterface are 0x00, but
> that everything else is a smattering of all manner of stuff: "Integrated
> Camera", "Bulk Control Interface", "Volume Control", "Firmware Upgrade
> Port", etc.
>
> Microchip's PIC32 USB HID sample code sets the value to 0x00.
>
> USB Complete (5th Edition) also shows this as 0x00 on page 267 for a
> configuration_descriptor for a vendor-specific HID.
>
>
>>> C) This is an actual bug *that should be fixed* and isn't that way
>>> intentionally for some Linux reason.
>> 
>> Up until now, it has been intentional. Currently, I don't see a need to
>> change it. 
>
> Clearly, some HID devices *are* using this field.  So, this probably
> should be made changeable if we want the HID gadget to be maximally useful.
>
> As for the technical details I can see from the code:
>
>
> A bit of code archaeology shows that .iInterface was expected to be
> dynamic as of the origin commit for f_hid.c (commit
> 71adf118946957839a13aa4d1094183e05c6c094).  This still persists in the
> file today.

What you're missing, though, is that iInterface is merely an *index*
into an array of strings. What's dynamic is the index, not the string
itself.

The reason for this is that we want to build different gadgets by
combining the different f_* functions in different ways which will, in
turn, change the location of $this function's strings in that array.

> The allocation of usb_gstrings_attach comes in at Nov 6, 2014 as of
> (commit 5ca8d3ec9970f4798e68bd21a9d44db3d0ff4da7) with the message:
>
> "Before configfs is integrated the usb_gstrings_attach() interface
> must be used."
>
> This looks like a case of something that simply got lost in the shuffle
> in the upgrade path to configfs, and I'm just the first poor slob who
> tripped over the oversight.
>
>
> However, I'm a little concerned as to why this was set *at all* (which
> seems to be what is actually tripping me up), and I'd be interested as
> to what drove the specific choice of 0x05 "HID Interface".  I don't see
> this in any spec anywhere obvious and this seems to be a deliberate
> change from previous behavior.

why? No behavior changed. Look at the very first commit on
f_hid.c. commit 71adf118946957839a13aa4d1094183e05c6c094 contains the
following:

+static struct usb_string ct_func_string_defs[] = {
+       [CT_FUNC_HID_IDX].s     = "HID Interface",
+       {},                     /* end of list */
+};
+
+static struct usb_gadget_strings ct_func_string_table = {
+       .language       = 0x0409,       /* en-US */
+       .strings        = ct_func_string_defs,
+};
+
+static struct usb_gadget_strings *ct_func_strings[] = {
+       &ct_func_string_table,
+       NULL,
+};

The string has *always* been "HID Interface"

> If this isn't user configurable, then it's probably a highly
> questionable choice to give this any default value other than 0x00.

why?

> This would have been quite a bit easier to code, too, so I'm *really*
> scratching my head about this.

what would've been easier to code?

>> I may be persuaded otherwise, but I need to see arguments
>> other than "I want to trick some SW into thinking I'm something else".
>
> Obviously I'm biased and scratching my own itch, but why would you
> consider that not to be an important argument?

Because you're asking me to accept code that will be used only by you
for, perhaps, a few weeks while you make (possibly) openocd think your
device is a CMSIS-DAP. If that's the only usecase, you can modify the
string in your own kernel tree. If it's a one-off thing, we don't need
that code in upstream.

Now, if there's a real usecase for this. Something that can attract, as
per Dave B., 3 or more users, then we can consider adding it
upstream. Remember that if we add support for changing interface
strings, it has to be implemented for *all* functions and validated on
all functions, then properly documented as a configfs ABI which can
never change anymore.

> Being able to fake being a particular piece of USB hardware that's
> currently tied to some ancient Windows binary driver is a great way to
> insert Linux into industrial and lab control pipelines.  Giving a

fair enough, but your making a claim that help you and only you :-)

> control board the ability to now also be accessed via ethernet or
> wireless (or even a better USB protocol) and thus now has an upgrade or
> higher performance path is a *really* useful thing.  And the Beaglebone
> Black is a really good "protocol engine".  Finally, after making the usb
> gadget emulation work, I can probably blow a bunch of Windows machines
> away completely since something like a Beaglebone Black is more than
> sufficient to handle the control without any outside intervention.

You're getting to a point where things start to get interesting. What
exactly are you trying to do?

> My end target isn't "CMSIS-DAP"--there are a zillion really cheap
> CMSIS-DAP devices so emulating CMSIS-DAP would be mostly pointless
> pedantry (although perhaps a good usb gadget tutorial).  CMSIS-DAP just
> happens to be a really good test case as it is an extremely well
> documented HID-based protocol and has extensive tests to validate it.
> Consequently, I can be sure that most bugs and difficulties are
> localized to my gadget implementation code or the gadget driver.

Fair enough

> Anyhow, let me know whether I should attack the problem or not.  I

I can't prevent you from working on it, I just think that:

a) perhaps you haven't considered all the intricacies around supporting
   this upstream (hint: you can't implement for f_hid alone)

b) you haven't convinced me that this has the potential for more than
   one user

c) you haven't considered the extra that comes with making this into an
   ABI

> suspect my biggest issue is simply that I will have to do all the work
> on a Beaglebone Black as I really don't know of any other way to test
> that gadget subsystem.  This will probably be okay if I can build it as

you could use dummy_hcd as well.

> a module--if I have to do a full kernel build it will probably demand
> that I set up a cross-compiling environment (and that isn't trivial).

cross-compilation for the kernel is trivial in most distributions. Just
install arm-linux-gnueabihf toolchain which should already be packaged
for you.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Unable to set "iInterface" in usb gadget via configfs
  2020-01-17  9:25         ` Felipe Balbi
@ 2020-01-18  0:58           ` Andrew P. Lentvorski
  2020-01-19 16:45             ` Felipe Balbi
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew P. Lentvorski @ 2020-01-18  0:58 UTC (permalink / raw)
  To: Felipe Balbi, Alan Stern; +Cc: linux-usb

On 1/17/20 1:25 AM, Felipe Balbi wrote:

> why? No behavior changed. Look at the very first commit on
> f_hid.c. commit 71adf118946957839a13aa4d1094183e05c6c094 contains the
> following:

Oops.  I missed that.  Sorry for not being complete enough.  My fault.

> Now, if there's a real usecase for this. Something that can attract, as
> per Dave B., 3 or more users, then we can consider adding it
> upstream. Remember that if we add support for changing interface
> strings, it has to be implemented for *all* functions and validated on
> all functions, then properly documented as a configfs ABI which can
> never change anymore.

Erg.  I did not realize that this was not going to be limited to just
f_hid.c.

That's ... ugly.  Reeeally ugly.  And a *LOT* of work.

I can certainly see that "some devices do this" is nowhere close to
enough justification for that.

>> control board the ability to now also be accessed via ethernet or
>> wireless (or even a better USB protocol) and thus now has an upgrade or
>> higher performance path is a *really* useful thing.  And the Beaglebone
>> Black is a really good "protocol engine".  Finally, after making the usb
>> gadget emulation work, I can probably blow a bunch of Windows machines
>> away completely since something like a Beaglebone Black is more than
>> sufficient to handle the control without any outside intervention.
> 
> You're getting to a point where things start to get interesting. What
> exactly are you trying to do?

I've got both a GPIB (with USB 1.0(!) only--as far as I can tell--talk
about a relic) and an industrial controller (unknown protocol but USB
tracing and a signal analyzer shows pretty much 1-1 so reverse
engineering it doesn't seem problematic) currently sitting on my desk.
I have one system which has enough USB devices in it that it won't work
on a USB 3 system because the Intel USB 3 chipset allocates twice as
many endpoints per device and hits an internal limit--they want a USB
upgrade as an intermediate move to ethernet.

I have a half-dozen other similar type requests queued behind these.
People are finally reaching a critical point where they can't keep
ancient hardware, ancient drivers, ancient Windows, and ancient
computers all running anymore--even VM's are starting to fail as too
many things have some level of timing baked into the driver (normally
unintentionally).

>> Anyhow, let me know whether I should attack the problem or not.

At this point, I suspect that the correct answer is "Keep an eye on this
while moving forward."

If I stumble over more drivers that are trying to use iInterface for
some reason, I will see if setting it to 0x00 makes things choose a
different path.  Simply changing the default to effectively 0x00-unused
seems like a lot less work and might pick up 90% of the use cases.  It
also means the scope would be limited to f_hid.c.  If I hit this a
couple times, I'll have a lot more justification behind me.

If I start seeing cases where I actually need to specify the iInterface
stuff, I'll come back with data and we can revisit this again.  I
suspect that someone is eventually going to drop a CDC class device in
front of me, and that will give me more data, too.  If ever I reach the
 point where I have multiple devices working around this, hopefully you
will find my arguments far more persuasive.  :)

> you could use dummy_hcd as well.

Interesting.  I did not know about this, but I will keep it in mind.


Thanks for being so patient.  Sorry for wasting your time only to come
back to "do nothing".

-a

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Unable to set "iInterface" in usb gadget via configfs
  2020-01-18  0:58           ` Andrew P. Lentvorski
@ 2020-01-19 16:45             ` Felipe Balbi
  0 siblings, 0 replies; 8+ messages in thread
From: Felipe Balbi @ 2020-01-19 16:45 UTC (permalink / raw)
  To: Andrew P. Lentvorski, Alan Stern; +Cc: linux-usb

[-- Attachment #1: Type: text/plain, Size: 4248 bytes --]


Hi Andrew,

"Andrew P. Lentvorski" <bsder@allcaps.org> writes:
> On 1/17/20 1:25 AM, Felipe Balbi wrote:
>
>> why? No behavior changed. Look at the very first commit on
>> f_hid.c. commit 71adf118946957839a13aa4d1094183e05c6c094 contains the
>> following:
>
> Oops.  I missed that.  Sorry for not being complete enough.  My fault.
>
>> Now, if there's a real usecase for this. Something that can attract, as
>> per Dave B., 3 or more users, then we can consider adding it
>> upstream. Remember that if we add support for changing interface
>> strings, it has to be implemented for *all* functions and validated on
>> all functions, then properly documented as a configfs ABI which can
>> never change anymore.
>
> Erg.  I did not realize that this was not going to be limited to just
> f_hid.c.

Well, all functions use the same infrastructure :-)

> That's ... ugly.  Reeeally ugly.  And a *LOT* of work.

yes :-)

> I can certainly see that "some devices do this" is nowhere close to
> enough justification for that.

right

>>> control board the ability to now also be accessed via ethernet or
>>> wireless (or even a better USB protocol) and thus now has an upgrade or
>>> higher performance path is a *really* useful thing.  And the Beaglebone
>>> Black is a really good "protocol engine".  Finally, after making the usb
>>> gadget emulation work, I can probably blow a bunch of Windows machines
>>> away completely since something like a Beaglebone Black is more than
>>> sufficient to handle the control without any outside intervention.
>> 
>> You're getting to a point where things start to get interesting. What
>> exactly are you trying to do?
>
> I've got both a GPIB (with USB 1.0(!) only--as far as I can tell--talk
> about a relic) and an industrial controller (unknown protocol but USB
> tracing and a signal analyzer shows pretty much 1-1 so reverse
> engineering it doesn't seem problematic) currently sitting on my desk.

you've just made totally envious :-p

> I have one system which has enough USB devices in it that it won't work
> on a USB 3 system because the Intel USB 3 chipset allocates twice as
> many endpoints per device and hits an internal limit--they want a USB
> upgrade as an intermediate move to ethernet.

I understand

> I have a half-dozen other similar type requests queued behind these.
> People are finally reaching a critical point where they can't keep
> ancient hardware, ancient drivers, ancient Windows, and ancient
> computers all running anymore--even VM's are starting to fail as too
> many things have some level of timing baked into the driver (normally
> unintentionally).
>
>>> Anyhow, let me know whether I should attack the problem or not.
>
> At this point, I suspect that the correct answer is "Keep an eye on this
> while moving forward."

that's a good approach for now

> If I stumble over more drivers that are trying to use iInterface for
> some reason, I will see if setting it to 0x00 makes things choose a
> different path.  Simply changing the default to effectively 0x00-unused
> seems like a lot less work and might pick up 90% of the use cases.  It
> also means the scope would be limited to f_hid.c.  If I hit this a
> couple times, I'll have a lot more justification behind me.
>
> If I start seeing cases where I actually need to specify the iInterface
> stuff, I'll come back with data and we can revisit this again.  I
> suspect that someone is eventually going to drop a CDC class device in
> front of me, and that will give me more data, too.  If ever I reach the
>  point where I have multiple devices working around this, hopefully you
> will find my arguments far more persuasive.  :)

Definitely, if you find a couple more usecases where this is needed,
then we have much stronger evidence that this will be needed for real.

>> you could use dummy_hcd as well.
>
> Interesting.  I did not know about this, but I will keep it in mind.
>
> Thanks for being so patient.  Sorry for wasting your time only to come
> back to "do nothing".

no worries, you didn't waste my time at all. I had never considered the
usecases you mentioned before this thread ;-)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-14 23:58 Unable to set "iInterface" in usb gadget via configfs Andrew P. Lentvorski
2020-01-15 15:14 ` Alan Stern
2020-01-16  2:23   ` Andrew P. Lentvorski
2020-01-16 13:02     ` Felipe Balbi
2020-01-17  0:39       ` Andrew P. Lentvorski
2020-01-17  9:25         ` Felipe Balbi
2020-01-18  0:58           ` Andrew P. Lentvorski
2020-01-19 16:45             ` Felipe Balbi

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git