On Wed, Aug 27, 2014 at 09:39:43PM +0200, Ricardo Ribalda Delgado wrote: > Hello Greg > > > > > > On Wed, Aug 27, 2014 at 9:25 PM, Greg Kroah-Hartman > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > On Wed, Aug 27, 2014 at 03:00:29PM +0200, Ricardo Ribalda Delgado wrote: > >> Defining the vendor and the product id should be enough to discriminate > >> the device. > >> > >> The reason for this patch is that there is a missmatch betweed the > >> modalias showed by sysfs and the modalias generated by file2alias. > >> > >> One expects the programming interface in uppercase and the other > >> generates it in lowercase. > > > > I don't understand, what is wrong here? Who does it in uppercase and > > who in lower? And does it matter? It's just a numeric value that > > should not be used as a string compare. > > > > In pci-sysfs: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/pci-sysfs.c#n175 > > static ssize_t modalias_show(struct device *dev, struct device_attribute *attr, > char *buf) > { > struct pci_dev *pci_dev = to_pci_dev(dev); > > return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n", > pci_dev->vendor, pci_dev->device, > pci_dev->subsystem_vendor, pci_dev->subsystem_device, > (u8)(pci_dev->class >> 16), (u8)(pci_dev->class >> 8), > (u8)(pci_dev->class)); > } > > > In file2alias: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts/mod/file2alias.c > > #define ADD(str, sep, cond, field) \ > do { \ > strcat(str, sep); \ > if (cond) \ > sprintf(str + strlen(str), \ > sizeof(field) == 1 ? "%02X" : \ > sizeof(field) == 2 ? "%04X" : \ > sizeof(field) == 4 ? "%08X" : "", \ > field); \ > else \ > sprintf(str + strlen(str), "*"); \ > } while(0) > > > ADD(alias, "bc", baseclass_mask == 0xFF, baseclass); > ADD(alias, "sc", subclass_mask == 0xFF, subclass); > ADD(alias, "i", interface_mask == 0xFF, interface); > > > > >> This means that some implementations modprobe will fail to load the > >> driver. > > > > What implementations fail to work? Shouldn't we fix the root of the > > problem and not just patch up all drivers to display incorrect data? > > At least the implementation of kmod in yocproject does a case sensitive match. > > > I have already sent a patch to fix what I consider the root of the problem > > https://lkml.org/lkml/2014/8/27/242 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. > > And I mean incorrect, as you are changing the values here from being > > very specific, to being much broader. > > > > 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. Fix your module loading code please. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
