[PATCH] pci/pci-sysfs: Set pci interface in uppercase

[Ricardo Ribalda Delgado]

There is a missmatch between the way file2alias generates the modalias
and the way the pci driver generates it.

Some implementations of modprobe will fail to load the driver for a pci
device automatically when the pci interface is defined on the driver. As
one will be in uppercase and the other in lowercase.

Fortunatelly not many drivers define this.

Signed-off-by: Ricardo Ribalda Delgado <***@gmail.com>
---
drivers/pci/pci-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..76ef791 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,7 +177,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pci_dev = to_pci_dev(dev);

- return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ 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),

[Greg KH]

Post by Ricardo Ribalda Delgado
There is a missmatch between the way file2alias generates the modalias
and the way the pci driver generates it.
Some implementations of modprobe will fail to load the driver for a pci
device automatically when the pci interface is defined on the driver. As
one will be in uppercase and the other in lowercase.
Fortunatelly not many drivers define this.
---
drivers/pci/pci-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..76ef791 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,7 +177,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ 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),

As said in the other thread about this issue, no, this code has been
here for over 9 years just fine. Please fix your userspace code that is
trying to compare hex values as a string and not a numeric value, that
is the stuff that is wrong, not the kernel.

thanks,

greg k-h

[Greg KH]

Post by Greg KH

Post by Ricardo Ribalda Delgado
There is a missmatch between the way file2alias generates the modalias
and the way the pci driver generates it.
Some implementations of modprobe will fail to load the driver for a pci
device automatically when the pci interface is defined on the driver. As
one will be in uppercase and the other in lowercase.
Fortunatelly not many drivers define this.
---
drivers/pci/pci-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..76ef791 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,7 +177,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ 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),

As said in the other thread about this issue, no, this code has been
here for over 9 years just fine. Please fix your userspace code that is
trying to compare hex values as a string and not a numeric value, that
is the stuff that is wrong, not the kernel.

Oh wait, I see what you are worried about now, the mis-match for just
the upper bits of the class value.

Yeah, that's a bug, sorry about that, a 9+ year old one, nice catch :)

Bjorn, feel free to apply this, sorry for the earlier objection. Also
please mark it for stable tree inclusion so this gets backported
properly.

thanks,

greg k-h

[Ricardo Ribalda Delgado]

No worries,

I have to mark for stable it or Bjorn? It it is me, how :) ?


Regards!


ps: For other people reading this thread, the kmod/modprobe is in
http://anonscm.debian.org/cgit/users/md/kmod.git/tree/libkmod/libkmod-index.c
and handles all the modalias as strings without differing the type.

Post by Greg KH

Post by Greg KH

Post by Ricardo Ribalda Delgado
There is a missmatch between the way file2alias generates the modalias
and the way the pci driver generates it.
Some implementations of modprobe will fail to load the driver for a pci
device automatically when the pci interface is defined on the driver. As
one will be in uppercase and the other in lowercase.
Fortunatelly not many drivers define this.
---
drivers/pci/pci-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..76ef791 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,7 +177,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ 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),

As said in the other thread about this issue, no, this code has been
here for over 9 years just fine. Please fix your userspace code that is
trying to compare hex values as a string and not a numeric value, that
is the stuff that is wrong, not the kernel.

Oh wait, I see what you are worried about now, the mis-match for just
the upper bits of the class value.
Yeah, that's a bug, sorry about that, a 9+ year old one, nice catch :)
Bjorn, feel free to apply this, sorry for the earlier objection. Also
please mark it for stable tree inclusion so this gets backported
properly.
thanks,
greg k-h

[Greg KH]

Post by Ricardo Ribalda Delgado
No worries,
I have to mark for stable it or Bjorn? It it is me, how :) ?

Bjorn can when he applies it, for details on the process, see the kernel
file Documentation/stable_kernel_rules.txt

Post by Ricardo Ribalda Delgado
ps: For other people reading this thread, the kmod/modprobe is in
http://anonscm.debian.org/cgit/users/md/kmod.git/tree/libkmod/libkmod-index.c
and handles all the modalias as strings without differing the type.

That sounds wrong, and odds are, will cause more problems over time.
These are hex values, not strings :(

thanks,

greg k-h

[Ricardo Ribalda Delgado]

Hello Greg

Post by Greg KH

Post by Ricardo Ribalda Delgado
No worries,
I have to mark for stable it or Bjorn? It it is me, how :) ?

Bjorn can when he applies it, for details on the process, see the kernel
file Documentation/stable_kernel_rules.txt

Post by Ricardo Ribalda Delgado
ps: For other people reading this thread, the kmod/modprobe is in
http://anonscm.debian.org/cgit/users/md/kmod.git/tree/libkmod/libkmod-index.c
and handles all the modalias as strings without differing the type.

That sounds wrong, and odds are, will cause more problems over time.
These are hex values, not strings :(

I totally see your point, but I disagree on the method.

I think is our resposibility (modalias/file2alias) to provide a
matcheable string, otherwise modprobe should be aware of all the types
(pci, usb, spi, vmbus.....)

As we keep adding types, and they don't follow any standard, we would
be adding a dependecy between kmod and the kernel, and duplicating
code. That is bad systemwise

Maybe the code on kmod should not be case sensitive, that is all :)

Post by Greg KH
thanks,
greg k-h

Thanks

[Greg KH]

Post by Ricardo Ribalda Delgado
Hello Greg

Post by Greg KH

Post by Ricardo Ribalda Delgado
No worries,
I have to mark for stable it or Bjorn? It it is me, how :) ?

Bjorn can when he applies it, for details on the process, see the kernel
file Documentation/stable_kernel_rules.txt

Post by Ricardo Ribalda Delgado
ps: For other people reading this thread, the kmod/modprobe is in
http://anonscm.debian.org/cgit/users/md/kmod.git/tree/libkmod/libkmod-index.c
and handles all the modalias as strings without differing the type.

That sounds wrong, and odds are, will cause more problems over time.
These are hex values, not strings :(

I totally see your point, but I disagree on the method.
I think is our resposibility (modalias/file2alias) to provide a
matcheable string, otherwise modprobe should be aware of all the types
(pci, usb, spi, vmbus.....)
As we keep adding types, and they don't follow any standard, we would
be adding a dependecy between kmod and the kernel, and duplicating
code. That is bad systemwise
Maybe the code on kmod should not be case sensitive, that is all :)

But this is a class field, which is a bit field, so it can't be compared
with a string compare, as it's a numerical value, not a string...

So maybe kmod needs to be fixed up?

thanks,

greg k-h

[Ricardo Ribalda Delgado]

Hej Bjorn

I have seen that you have updated your pci tree without this patch. Is
there something wrong with it?

If you are already considering it, sorry for the mail, but I wanted to
make sure that it is in 3.17 and then back ported.


Thanks!!

Post by Greg KH

Post by Greg KH

Post by Ricardo Ribalda Delgado
There is a missmatch between the way file2alias generates the modalias
and the way the pci driver generates it.
Some implementations of modprobe will fail to load the driver for a pci
device automatically when the pci interface is defined on the driver. As
one will be in uppercase and the other in lowercase.
Fortunatelly not many drivers define this.
---
drivers/pci/pci-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..76ef791 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,7 +177,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ 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),

As said in the other thread about this issue, no, this code has been
here for over 9 years just fine. Please fix your userspace code that is
trying to compare hex values as a string and not a numeric value, that
is the stuff that is wrong, not the kernel.

Oh wait, I see what you are worried about now, the mis-match for just
the upper bits of the class value.
Yeah, that's a bug, sorry about that, a 9+ year old one, nice catch :)
Bjorn, feel free to apply this, sorry for the earlier objection. Also
please mark it for stable tree inclusion so this gets backported
properly.
thanks,
greg k-h

[Bjorn Helgaas]

On Tue, Sep 2, 2014 at 1:22 PM, Ricardo Ribalda Delgado

Post by Ricardo Ribalda Delgado
Hej Bjorn
I have seen that you have updated your pci tree without this patch. Is
there something wrong with it?
If you are already considering it, sorry for the mail, but I wanted to
make sure that it is in 3.17 and then back ported.

I haven't even looked at it yet; I'm just starting to get caught up.
Since this looks like a fix for a very old bug, I'll probably apply it
for v3.18, and mark it for stable.

Post by Ricardo Ribalda Delgado

Post by Greg KH

Post by Greg KH

Post by Ricardo Ribalda Delgado
There is a missmatch between the way file2alias generates the modalias
and the way the pci driver generates it.
Some implementations of modprobe will fail to load the driver for a pci
device automatically when the pci interface is defined on the driver. As
one will be in uppercase and the other in lowercase.
Fortunatelly not many drivers define this.
---
drivers/pci/pci-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..76ef791 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,7 +177,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ 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),

As said in the other thread about this issue, no, this code has been
here for over 9 years just fine. Please fix your userspace code that is
trying to compare hex values as a string and not a numeric value, that
is the stuff that is wrong, not the kernel.

Oh wait, I see what you are worried about now, the mis-match for just
the upper bits of the class value.
Yeah, that's a bug, sorry about that, a 9+ year old one, nice catch :)
Bjorn, feel free to apply this, sorry for the earlier objection. Also
please mark it for stable tree inclusion so this gets backported
properly.
thanks,
greg k-h

--
Ricardo Ribalda

[Bjorn Helgaas]

Post by Ricardo Ribalda Delgado
There is a missmatch between the way file2alias generates the modalias
and the way the pci driver generates it.
Some implementations of modprobe will fail to load the driver for a pci
device automatically when the pci interface is defined on the driver. As
one will be in uppercase and the other in lowercase.
Fortunatelly not many drivers define this.

Applied to pci/enumeration for v3.18, with Greg's ack and marked for
stable.

Post by Ricardo Ribalda Delgado
---
drivers/pci/pci-sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ff0a90..76ef791 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,7 +177,7 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
{
struct pci_dev *pci_dev = to_pci_dev(dev);
- return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n",
+ 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),
--
2.1.0