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

[Ricardo Ribalda Delgado]

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

>
> 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.


> thanks,

Thank you :)
>
> greg k-h



-- 
Ricardo Ribalda
--
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/