| Message ID | 20190821153545.17635-4-jonnyc@amazon.com |
|---|---|
| State | Accepted |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Amazon's Annapurna Labs DT-based PCIe host controller driver | expand |
On Wed, Aug 21, 2019 at 06:35:43PM +0300, Jonathan Chocron wrote: > The Amazon Annapurna Labs PCIe Root Port exposes the VPD capability, > but there is no actual support for it. > > The reason for not using the already existing quirk_blacklist_vpd() > is that, although this fails pci_vpd_read/write, the 'vpd' sysfs > entry still exists. When running lspci -vv, for example, this > results in the following error: > > pcilib: sysfs_read_vpd: read failed: Input/output error Oh that's not nice. It's probably triggered by the -EIO in pci_vpd_read. A quick search online seems to show that other people have experienced this too - though from as far as I can tell this just gives you a warning and pcilib will continnue to give other output? I guess every vpd blacklist'd driver will have the same issue. And for this reason I don't think that this patch is the right solution - as otherwise all the other blacklisted drivers could follow your lead. I don't think you need to fix this specifically for the AL driver and so I'd suggest that you can probably drop this patch. (Ideally pciutils could be updated to not warn for this specific use-case). Thanks, Andrew Murray > > This quirk removes the sysfs entry, which avoids the error print. > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > Reviewed-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > --- > drivers/pci/vpd.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > index 4963c2e2bd4c..c23a8ec08db9 100644 > --- a/drivers/pci/vpd.c > +++ b/drivers/pci/vpd.c > @@ -644,4 +644,20 @@ static void quirk_chelsio_extend_vpd(struct pci_dev *dev) > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID, > quirk_chelsio_extend_vpd); > > +static void quirk_al_vpd_release(struct pci_dev *dev) > +{ > + if (dev->vpd) { > + pci_vpd_release(dev); > + dev->vpd = NULL; > + pci_warn(dev, FW_BUG "Releasing VPD capability (No support for VPD read/write transactions)\n"); > + } > +} > + > +/* > + * The 0031 device id is reused for other non Root Port device types, > + * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI class. > + */ > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031, > + PCI_CLASS_BRIDGE_PCI, 8, quirk_al_vpd_release); > + > #endif > -- > 2.17.1 >
On Thu, 2019-08-22 at 12:41 +0100, Andrew Murray wrote: > On Wed, Aug 21, 2019 at 06:35:43PM +0300, Jonathan Chocron wrote: > > The Amazon Annapurna Labs PCIe Root Port exposes the VPD > > capability, > > but there is no actual support for it. > > > > The reason for not using the already existing quirk_blacklist_vpd() > > is that, although this fails pci_vpd_read/write, the 'vpd' sysfs > > entry still exists. When running lspci -vv, for example, this > > results in the following error: > > > > pcilib: sysfs_read_vpd: read failed: Input/output error > > Oh that's not nice. It's probably triggered by the -EIO in > pci_vpd_read. > A quick search online seems to show that other people have > experienced > this too - though from as far as I can tell this just gives you a > warning and pcilib will continnue to give other output? > Correct. > I guess every vpd blacklist'd driver will have the same issue. And > for > this reason I don't think that this patch is the right solution - as > otherwise all the other blacklisted drivers could follow your lead. > I think that going forward, they should follow my lead, I just didn't want to possibly break any assumptions other vendors' tools might have regarding the existence/non-existence of the vpd sysfs entry. > I don't think you need to fix this specifically for the AL driver and > so > I'd suggest that you can probably drop this patch. (Ideally pciutils > could be updated to not warn for this specific use-case). > I don't think that solution should be implemented in pcituils. It rightfully warns when it fails to read from the vpd sysfs file - it first 'open's the file which succeeds, and then fails when trying to 'read' from it. I don't think that it should specifically "mask" out -EIO, since it shouldn't have to "know" that the underlying reason is a VPD quirk (or more precisely vpd->len == 0). Furthermore, it is possible that this error code would be returned for some other reason (not sure if currently this occurs). I think that if the device doesn't properly support vpd, the kernel shouldn't expose the "empty" sysfs file in the first place. In the long run, quirk_blacklist_vpd() should probably be modified to do what our quirk does or something similar (and then the al quirk can be removed). What do you think? > Thanks, > > Andrew Murray > > > > > This quirk removes the sysfs entry, which avoids the error print. > > > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > Reviewed-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > --- > > drivers/pci/vpd.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > > index 4963c2e2bd4c..c23a8ec08db9 100644 > > --- a/drivers/pci/vpd.c > > +++ b/drivers/pci/vpd.c > > @@ -644,4 +644,20 @@ static void quirk_chelsio_extend_vpd(struct > > pci_dev *dev) > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID, > > quirk_chelsio_extend_vpd); > > > > +static void quirk_al_vpd_release(struct pci_dev *dev) > > +{ > > + if (dev->vpd) { > > + pci_vpd_release(dev); > > + dev->vpd = NULL; > > + pci_warn(dev, FW_BUG "Releasing VPD capability (No > > support for VPD read/write transactions)\n"); > > + } > > +} > > + > > +/* > > + * The 0031 device id is reused for other non Root Port device > > types, > > + * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI > > class. > > + */ > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, > > 0x0031, > > + PCI_CLASS_BRIDGE_PCI, 8, > > quirk_al_vpd_release); > > + > > #endif > > -- > > 2.17.1 > >
On Thu, Aug 22, 2019 at 02:36:24PM +0000, Chocron, Jonathan wrote: > On Thu, 2019-08-22 at 12:41 +0100, Andrew Murray wrote: > > On Wed, Aug 21, 2019 at 06:35:43PM +0300, Jonathan Chocron wrote: > > > The Amazon Annapurna Labs PCIe Root Port exposes the VPD > > > capability, > > > but there is no actual support for it. > > > > > > The reason for not using the already existing quirk_blacklist_vpd() > > > is that, although this fails pci_vpd_read/write, the 'vpd' sysfs > > > entry still exists. When running lspci -vv, for example, this > > > results in the following error: > > > > > > pcilib: sysfs_read_vpd: read failed: Input/output error > > > > Oh that's not nice. It's probably triggered by the -EIO in > > pci_vpd_read. > > A quick search online seems to show that other people have > > experienced > > this too - though from as far as I can tell this just gives you a > > warning and pcilib will continnue to give other output? > > > Correct. > > > I guess every vpd blacklist'd driver will have the same issue. And > > for > > this reason I don't think that this patch is the right solution - as > > otherwise all the other blacklisted drivers could follow your lead. > > > I think that going forward, they should follow my lead, I just didn't > want to possibly break any assumptions other vendors' tools might have > regarding the existence/non-existence of the vpd sysfs entry. > > > I don't think you need to fix this specifically for the AL driver and > > so > > I'd suggest that you can probably drop this patch. (Ideally pciutils > > could be updated to not warn for this specific use-case). > > > I don't think that solution should be implemented in pcituils. It > rightfully warns when it fails to read from the vpd sysfs file - it > first 'open's the file which succeeds, and then fails when trying to > 'read' from it. Indeed - this is correct. > I don't think that it should specifically "mask" out > -EIO, since it shouldn't have to "know" that the underlying reason is a You're probably right - I guess the kernel should document somewhere (ABI/testing/sysfs-bus-pci?) what the kernel does when such a quirk exists, then userspace can conform. For example if -EIO cannot be returned any other way then it would be OK for pciutils to mask it out - but its ambigious at the moment. > VPD quirk (or more precisely vpd->len == 0). Furthermore, it is > possible that this error code would be returned for some other reason > (not sure if currently this occurs). > > I think that if the device doesn't properly support vpd, the kernel > shouldn't expose the "empty" sysfs file in the first place. > > In the long run, quirk_blacklist_vpd() should probably be modified to > do what our quirk does or something similar (and then the al quirk can > be removed). What do you think? When I first saw your quirk, I did wonder why quirk_blacklist_vpd doesn't do what your quirk does. Perhaps there isn't a reason. It was first introduced in 2016: 7c20078a8197 ("PCI: Prevent VPD access for buggy devices") Some may argue that actually because your hardware has a VPD capability it should have the sysfs file - but the capability doesn't work and so the sysfs file should return an error. I'd be keen to change quirk_blacklist_vpd - Babu, Bjorn any objections? Thanks, Andrew Murray > > > Thanks, > > > > Andrew Murray > > > > > > > > This quirk removes the sysfs entry, which avoids the error print. > > > > > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > > Reviewed-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > > --- > > > drivers/pci/vpd.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > > > index 4963c2e2bd4c..c23a8ec08db9 100644 > > > --- a/drivers/pci/vpd.c > > > +++ b/drivers/pci/vpd.c > > > @@ -644,4 +644,20 @@ static void quirk_chelsio_extend_vpd(struct > > > pci_dev *dev) > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID, > > > quirk_chelsio_extend_vpd); > > > > > > +static void quirk_al_vpd_release(struct pci_dev *dev) > > > +{ > > > + if (dev->vpd) { > > > + pci_vpd_release(dev); > > > + dev->vpd = NULL; > > > + pci_warn(dev, FW_BUG "Releasing VPD capability (No > > > support for VPD read/write transactions)\n"); > > > + } > > > +} > > > + > > > +/* > > > + * The 0031 device id is reused for other non Root Port device > > > types, > > > + * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI > > > class. > > > + */ > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, > > > 0x0031, > > > + PCI_CLASS_BRIDGE_PCI, 8, > > > quirk_al_vpd_release); > > > + > > > #endif > > > -- > > > 2.17.1 > > >
[+CC Babu Moger <babu.moger@oracle.com>] On Thu, Aug 22, 2019 at 02:36:24PM +0000, Chocron, Jonathan wrote: > On Thu, 2019-08-22 at 12:41 +0100, Andrew Murray wrote: > > On Wed, Aug 21, 2019 at 06:35:43PM +0300, Jonathan Chocron wrote: > > > The Amazon Annapurna Labs PCIe Root Port exposes the VPD > > > capability, > > > but there is no actual support for it. > > > > > > The reason for not using the already existing quirk_blacklist_vpd() > > > is that, although this fails pci_vpd_read/write, the 'vpd' sysfs > > > entry still exists. When running lspci -vv, for example, this > > > results in the following error: > > > > > > pcilib: sysfs_read_vpd: read failed: Input/output error > > > > Oh that's not nice. It's probably triggered by the -EIO in > > pci_vpd_read. > > A quick search online seems to show that other people have > > experienced > > this too - though from as far as I can tell this just gives you a > > warning and pcilib will continnue to give other output? > > > Correct. > > > I guess every vpd blacklist'd driver will have the same issue. And > > for > > this reason I don't think that this patch is the right solution - as > > otherwise all the other blacklisted drivers could follow your lead. > > > I think that going forward, they should follow my lead, I just didn't > want to possibly break any assumptions other vendors' tools might have > regarding the existence/non-existence of the vpd sysfs entry. > > > I don't think you need to fix this specifically for the AL driver and > > so > > I'd suggest that you can probably drop this patch. (Ideally pciutils > > could be updated to not warn for this specific use-case). > > > I don't think that solution should be implemented in pcituils. It > rightfully warns when it fails to read from the vpd sysfs file - it > first 'open's the file which succeeds, and then fails when trying to > 'read' from it. I don't think that it should specifically "mask" out > -EIO, since it shouldn't have to "know" that the underlying reason is a > VPD quirk (or more precisely vpd->len == 0). Furthermore, it is > possible that this error code would be returned for some other reason > (not sure if currently this occurs). > > I think that if the device doesn't properly support vpd, the kernel > shouldn't expose the "empty" sysfs file in the first place. > > In the long run, quirk_blacklist_vpd() should probably be modified to > do what our quirk does or something similar (and then the al quirk can > be removed). What do you think? > > > Thanks, > > > > Andrew Murray > > > > > > > > This quirk removes the sysfs entry, which avoids the error print. > > > > > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > > Reviewed-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > > --- > > > drivers/pci/vpd.c | 16 ++++++++++++++++ > > > 1 file changed, 16 insertions(+) > > > > > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > > > index 4963c2e2bd4c..c23a8ec08db9 100644 > > > --- a/drivers/pci/vpd.c > > > +++ b/drivers/pci/vpd.c > > > @@ -644,4 +644,20 @@ static void quirk_chelsio_extend_vpd(struct > > > pci_dev *dev) > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID, > > > quirk_chelsio_extend_vpd); > > > > > > +static void quirk_al_vpd_release(struct pci_dev *dev) > > > +{ > > > + if (dev->vpd) { > > > + pci_vpd_release(dev); > > > + dev->vpd = NULL; > > > + pci_warn(dev, FW_BUG "Releasing VPD capability (No > > > support for VPD read/write transactions)\n"); > > > + } > > > +} > > > + > > > +/* > > > + * The 0031 device id is reused for other non Root Port device > > > types, > > > + * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI > > > class. > > > + */ > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, > > > 0x0031, > > > + PCI_CLASS_BRIDGE_PCI, 8, > > > quirk_al_vpd_release); > > > + > > > #endif > > > -- > > > 2.17.1 > > >
On Thu, 2019-08-22 at 16:07 +0100, Andrew Murray wrote: > On Thu, Aug 22, 2019 at 02:36:24PM +0000, Chocron, Jonathan wrote: > > On Thu, 2019-08-22 at 12:41 +0100, Andrew Murray wrote: > > > On Wed, Aug 21, 2019 at 06:35:43PM +0300, Jonathan Chocron wrote: > > > > The Amazon Annapurna Labs PCIe Root Port exposes the VPD > > > > capability, > > > > but there is no actual support for it. > > > > > > > > The reason for not using the already existing > > > > quirk_blacklist_vpd() > > > > is that, although this fails pci_vpd_read/write, the 'vpd' > > > > sysfs > > > > entry still exists. When running lspci -vv, for example, this > > > > results in the following error: > > > > > > > > pcilib: sysfs_read_vpd: read failed: Input/output error > > > > > > Oh that's not nice. It's probably triggered by the -EIO in > > > pci_vpd_read. > > > A quick search online seems to show that other people have > > > experienced > > > this too - though from as far as I can tell this just gives you a > > > warning and pcilib will continnue to give other output? > > > > > > > Correct. > > > > > I guess every vpd blacklist'd driver will have the same issue. > > > And > > > for > > > this reason I don't think that this patch is the right solution - > > > as > > > otherwise all the other blacklisted drivers could follow your > > > lead. > > > > > > > I think that going forward, they should follow my lead, I just > > didn't > > want to possibly break any assumptions other vendors' tools might > > have > > regarding the existence/non-existence of the vpd sysfs entry. > > > > > I don't think you need to fix this specifically for the AL driver > > > and > > > so > > > I'd suggest that you can probably drop this patch. (Ideally > > > pciutils > > > could be updated to not warn for this specific use-case). > > > > > > > I don't think that solution should be implemented in pcituils. It > > rightfully warns when it fails to read from the vpd sysfs file - it > > first 'open's the file which succeeds, and then fails when trying > > to > > 'read' from it. > > Indeed - this is correct. > > > I don't think that it should specifically "mask" out > > -EIO, since it shouldn't have to "know" that the underlying reason > > is a > > You're probably right - I guess the kernel should document somewhere > (ABI/testing/sysfs-bus-pci?) what the kernel does when such a quirk > exists, > then userspace can conform. For example if -EIO cannot be returned > any > other way then it would be OK for pciutils to mask it out - but its > ambigious at the moment. > > > VPD quirk (or more precisely vpd->len == 0). Furthermore, it is > > possible that this error code would be returned for some other > > reason > > (not sure if currently this occurs). > > > > I think that if the device doesn't properly support vpd, the kernel > > shouldn't expose the "empty" sysfs file in the first place. > > > > In the long run, quirk_blacklist_vpd() should probably be modified > > to > > do what our quirk does or something similar (and then the al quirk > > can > > be removed). What do you think? > > When I first saw your quirk, I did wonder why quirk_blacklist_vpd > doesn't > do what your quirk does. Perhaps there isn't a reason. It was first > introduced in 2016: > > 7c20078a8197 ("PCI: Prevent VPD access for buggy devices") > > Some may argue that actually because your hardware has a VPD > capability > it should have the sysfs file - but the capability doesn't work and > so > the sysfs file should return an error. > > I'd be keen to change quirk_blacklist_vpd - Babu, Bjorn any > objections? > Since the merge window is closing and I don't want to affect any other PCIe controllers without having their maintainers testing this change, I'll remove this function and register our device_id with the existing quirk_blacklist_vpd. This will be part of v5. I'll then submit a separate patch (for the next kernel version) which changes the quirk_blacklist_vpd to do what I originally intended. > Thanks, > > Andrew Murray > > > > > > Thanks, > > > > > > Andrew Murray > > > > > > > > > > > This quirk removes the sysfs entry, which avoids the error > > > > print. > > > > > > > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > > > Reviewed-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > > > --- > > > > drivers/pci/vpd.c | 16 ++++++++++++++++ > > > > 1 file changed, 16 insertions(+) > > > > > > > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > > > > index 4963c2e2bd4c..c23a8ec08db9 100644 > > > > --- a/drivers/pci/vpd.c > > > > +++ b/drivers/pci/vpd.c > > > > @@ -644,4 +644,20 @@ static void > > > > quirk_chelsio_extend_vpd(struct > > > > pci_dev *dev) > > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID, > > > > quirk_chelsio_extend_vpd); > > > > > > > > +static void quirk_al_vpd_release(struct pci_dev *dev) > > > > +{ > > > > + if (dev->vpd) { > > > > + pci_vpd_release(dev); > > > > + dev->vpd = NULL; > > > > + pci_warn(dev, FW_BUG "Releasing VPD capability > > > > (No > > > > support for VPD read/write transactions)\n"); > > > > + } > > > > +} > > > > + > > > > +/* > > > > + * The 0031 device id is reused for other non Root Port device > > > > types, > > > > + * therefore the quirk is registered for the > > > > PCI_CLASS_BRIDGE_PCI > > > > class. > > > > + */ > > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_L > > > > ABS, > > > > 0x0031, > > > > + PCI_CLASS_BRIDGE_PCI, 8, > > > > quirk_al_vpd_release); > > > > + > > > > #endif > > > > -- > > > > 2.17.1 > > > >
On Wed, Sep 04, 2019 at 01:36:12PM +0000, Chocron, Jonathan wrote: > On Thu, 2019-08-22 at 16:07 +0100, Andrew Murray wrote: > > On Thu, Aug 22, 2019 at 02:36:24PM +0000, Chocron, Jonathan wrote: > > > On Thu, 2019-08-22 at 12:41 +0100, Andrew Murray wrote: > > > > On Wed, Aug 21, 2019 at 06:35:43PM +0300, Jonathan Chocron wrote: > > > > > The Amazon Annapurna Labs PCIe Root Port exposes the VPD > > > > > capability, > > > > > but there is no actual support for it. > > > > > > > > > > The reason for not using the already existing > > > > > quirk_blacklist_vpd() > > > > > is that, although this fails pci_vpd_read/write, the 'vpd' > > > > > sysfs > > > > > entry still exists. When running lspci -vv, for example, this > > > > > results in the following error: > > > > > > > > > > pcilib: sysfs_read_vpd: read failed: Input/output error > > > > > > > > Oh that's not nice. It's probably triggered by the -EIO in > > > > pci_vpd_read. > > > > A quick search online seems to show that other people have > > > > experienced > > > > this too - though from as far as I can tell this just gives you a > > > > warning and pcilib will continnue to give other output? > > > > > > > > > > Correct. > > > > > > > I guess every vpd blacklist'd driver will have the same issue. > > > > And > > > > for > > > > this reason I don't think that this patch is the right solution - > > > > as > > > > otherwise all the other blacklisted drivers could follow your > > > > lead. > > > > > > > > > > I think that going forward, they should follow my lead, I just > > > didn't > > > want to possibly break any assumptions other vendors' tools might > > > have > > > regarding the existence/non-existence of the vpd sysfs entry. > > > > > > > I don't think you need to fix this specifically for the AL driver > > > > and > > > > so > > > > I'd suggest that you can probably drop this patch. (Ideally > > > > pciutils > > > > could be updated to not warn for this specific use-case). > > > > > > > > > > I don't think that solution should be implemented in pcituils. It > > > rightfully warns when it fails to read from the vpd sysfs file - it > > > first 'open's the file which succeeds, and then fails when trying > > > to > > > 'read' from it. > > > > Indeed - this is correct. > > > > > I don't think that it should specifically "mask" out > > > -EIO, since it shouldn't have to "know" that the underlying reason > > > is a > > > > You're probably right - I guess the kernel should document somewhere > > (ABI/testing/sysfs-bus-pci?) what the kernel does when such a quirk > > exists, > > then userspace can conform. For example if -EIO cannot be returned > > any > > other way then it would be OK for pciutils to mask it out - but its > > ambigious at the moment. > > > > > VPD quirk (or more precisely vpd->len == 0). Furthermore, it is > > > possible that this error code would be returned for some other > > > reason > > > (not sure if currently this occurs). > > > > > > I think that if the device doesn't properly support vpd, the kernel > > > shouldn't expose the "empty" sysfs file in the first place. > > > > > > In the long run, quirk_blacklist_vpd() should probably be modified > > > to > > > do what our quirk does or something similar (and then the al quirk > > > can > > > be removed). What do you think? > > > > When I first saw your quirk, I did wonder why quirk_blacklist_vpd > > doesn't > > do what your quirk does. Perhaps there isn't a reason. It was first > > introduced in 2016: > > > > 7c20078a8197 ("PCI: Prevent VPD access for buggy devices") > > > > Some may argue that actually because your hardware has a VPD > > capability > > it should have the sysfs file - but the capability doesn't work and > > so > > the sysfs file should return an error. > > > > I'd be keen to change quirk_blacklist_vpd - Babu, Bjorn any > > objections? > > > Since the merge window is closing and I don't want to affect any other > PCIe controllers without having their maintainers testing this change, > I'll remove this function and register our device_id with the existing > quirk_blacklist_vpd. This will be part of v5. Thanks - this sounds like a reasonable approach. > > I'll then submit a separate patch (for the next kernel version) which > changes the quirk_blacklist_vpd to do what I originally intended. Thanks - we can then see what the wider consensus on this is. I'll look forward to your respin. Thanks, Andrew Murray > > > Thanks, > > > > Andrew Murray > > > > > > > > > Thanks, > > > > > > > > Andrew Murray > > > > > > > > > > > > > > This quirk removes the sysfs entry, which avoids the error > > > > > print. > > > > > > > > > > Signed-off-by: Jonathan Chocron <jonnyc@amazon.com> > > > > > Reviewed-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com> > > > > > --- > > > > > drivers/pci/vpd.c | 16 ++++++++++++++++ > > > > > 1 file changed, 16 insertions(+) > > > > > > > > > > diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c > > > > > index 4963c2e2bd4c..c23a8ec08db9 100644 > > > > > --- a/drivers/pci/vpd.c > > > > > +++ b/drivers/pci/vpd.c > > > > > @@ -644,4 +644,20 @@ static void > > > > > quirk_chelsio_extend_vpd(struct > > > > > pci_dev *dev) > > > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID, > > > > > quirk_chelsio_extend_vpd); > > > > > > > > > > +static void quirk_al_vpd_release(struct pci_dev *dev) > > > > > +{ > > > > > + if (dev->vpd) { > > > > > + pci_vpd_release(dev); > > > > > + dev->vpd = NULL; > > > > > + pci_warn(dev, FW_BUG "Releasing VPD capability > > > > > (No > > > > > support for VPD read/write transactions)\n"); > > > > > + } > > > > > +} > > > > > + > > > > > +/* > > > > > + * The 0031 device id is reused for other non Root Port device > > > > > types, > > > > > + * therefore the quirk is registered for the > > > > > PCI_CLASS_BRIDGE_PCI > > > > > class. > > > > > + */ > > > > > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_L > > > > > ABS, > > > > > 0x0031, > > > > > + PCI_CLASS_BRIDGE_PCI, 8, > > > > > quirk_al_vpd_release); > > > > > + > > > > > #endif > > > > > -- > > > > > 2.17.1 > > > > >
diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c index 4963c2e2bd4c..c23a8ec08db9 100644 --- a/drivers/pci/vpd.c +++ b/drivers/pci/vpd.c @@ -644,4 +644,20 @@ static void quirk_chelsio_extend_vpd(struct pci_dev *dev) DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID, quirk_chelsio_extend_vpd); +static void quirk_al_vpd_release(struct pci_dev *dev) +{ + if (dev->vpd) { + pci_vpd_release(dev); + dev->vpd = NULL; + pci_warn(dev, FW_BUG "Releasing VPD capability (No support for VPD read/write transactions)\n"); + } +} + +/* + * The 0031 device id is reused for other non Root Port device types, + * therefore the quirk is registered for the PCI_CLASS_BRIDGE_PCI class. + */ +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_AMAZON_ANNAPURNA_LABS, 0x0031, + PCI_CLASS_BRIDGE_PCI, 8, quirk_al_vpd_release); + #endif