Re: [PATCH] usb: gadget: net2280: Remove pci_class from PCI_TABLE

[BjÃrn Mork]

Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> writes:
> On Wed, Aug 27, 2014 at 09:39:43PM +0200, Ricardo Ribalda Delgado wrote:
>> 
>> return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",

That final 'x' does look like a typo, doesn't it?  We are otherwise
consistently using upper-case hex digits for field values and lower case
letter for field names.  But it looks like it has been like that since
the beginning, so it might be difficult to fix...

> No, the root cause of the problem is a userspace tool looking at a hex
> value as a string and not a number.  It doesn't matter if we print it in
> upper or lower case, it's a digit, not a string.  Do the numeric
> compare, not a string compare.

Now I don't really know much about the history here, but the format of
module aliases, using wildcards, seem to suggest a string match to me.
Do you really mean that these strings should be parsed into field names
+ values before matching?  If that was the intention then surely we
would have exported the fields one-by-one as separate sysfs attributes?
Ref the "one value per file" policy.

>> Not many drivers define the pci interface and there is no other driver
>> that has the same vendor  and product id. Therefore I see no hurt in
>> adding both patches, one to make the driver broader, and another to
>> fix pci-sysfs.
>> 
>> Also, the change on pci-sysfs might affect more stuff and therefore
>> take longer to be applied.
>
> As we have been printing the value to userspace in this way for well
> over a decade now, and nothing has changed, I say it's a userspace bug
> that you should fix instead.  Don't work around broken user programs in
> the kernel by changing something that has been stable for 10+ years.
>
> Ok, sorry, not 10+ years, the commit was written May of 2005, so 9
> years.

well, just looking at a few common PCI devices on my PCs I wonder if the
reason this hasn't been a problem is because there are _very_ few PCI
programming interfaces using anything by 0-9 digits.  One?  Looking at the
modules built by Debian I can only find one udc module matching on any
hex value:

 bjorn@nemi:~$  grep pci: /lib/modules/3.16-trunk-amd64/modules.alias|egrep "i[A-F]"
 alias pci:v000010DBd00008808sv*sd*bc0Csc03iFE* pch_udc
 alias pci:v000010DBd0000801Dsv*sd*bc0Csc03iFE* pch_udc
 alias pci:v00008086d00008808sv*sd*bc0Csc03iFE* pch_udc

This makes me wonder if this is exclusively a problem for PCI UDCs,
which tend to be pretty rare devices?


BjÃrn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/