| Message ID | b04a0e46-0b97-da3d-aa77-b05c9b37d21f@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/VPD: Silence warning if optional VPD PROM is missing | expand |
Hi Heiner, > Realtek RTL8169/8168/8125 NIC families indicate VPD capability and an > optional VPD EEPROM can be connected via I2C/SPI. However I haven't > seen any card or system with such a VPD EEPROM yet. The missing EEPROM > causes the following warning whenever e.g. lscpi -vv is executed. > > invalid short VPD tag 00 at offset 01 > > The warning confuses users, I think we should handle the situation more > gentle. Therefore, if first VPD byte is read as 0x00, assume a missing > optional VPD PROM as and silently set the VPD length to 0. [...] True. I saw people on different forum and IRC asking for clarification assuming their NIC broke, or that something is wrong, so this would indeed save them some worry, nice! Having said that, I also saw this particular warning showing up for some storage controllers (often some SAS cards), so a question here: would it warrant adding a pci_dbg() with an appropriate message rather than just returning 0? I wonder if this might be useful for someone who is trying to troubleshoot and/or debug some issues with their device. What do you think? Krzysztof
On 07.03.2021 19:27, Krzysztof Wilczyński wrote: > Hi Heiner, > >> Realtek RTL8169/8168/8125 NIC families indicate VPD capability and an >> optional VPD EEPROM can be connected via I2C/SPI. However I haven't >> seen any card or system with such a VPD EEPROM yet. The missing EEPROM >> causes the following warning whenever e.g. lscpi -vv is executed. >> >> invalid short VPD tag 00 at offset 01 >> >> The warning confuses users, I think we should handle the situation more >> gentle. Therefore, if first VPD byte is read as 0x00, assume a missing >> optional VPD PROM as and silently set the VPD length to 0. > [...] > > True. I saw people on different forum and IRC asking for clarification > assuming their NIC broke, or that something is wrong, so this would > indeed save them some worry, nice! > > Having said that, I also saw this particular warning showing up for some > storage controllers (often some SAS cards), so a question here: would it > warrant adding a pci_dbg() with an appropriate message rather than just > returning 0? I wonder if this might be useful for someone who is trying > to troubleshoot and/or debug some issues with their device. > > What do you think? > I don't have a strong opinion here, but yes, that's something we could do. > Krzysztof >
On Sun, Mar 07, 2021 at 10:34:25PM +0100, Heiner Kallweit wrote: > On 07.03.2021 19:27, Krzysztof Wilczyński wrote: > > Hi Heiner, > > > >> Realtek RTL8169/8168/8125 NIC families indicate VPD capability and an > >> optional VPD EEPROM can be connected via I2C/SPI. However I haven't > >> seen any card or system with such a VPD EEPROM yet. The missing EEPROM > >> causes the following warning whenever e.g. lscpi -vv is executed. > >> > >> invalid short VPD tag 00 at offset 01 > >> > >> The warning confuses users, I think we should handle the situation more > >> gentle. Therefore, if first VPD byte is read as 0x00, assume a missing > >> optional VPD PROM as and silently set the VPD length to 0. > > [...] > > > > True. I saw people on different forum and IRC asking for clarification > > assuming their NIC broke, or that something is wrong, so this would > > indeed save them some worry, nice! > > > > Having said that, I also saw this particular warning showing up for some > > storage controllers (often some SAS cards), so a question here: would it > > warrant adding a pci_dbg() with an appropriate message rather than just > > returning 0? I wonder if this might be useful for someone who is trying > > to troubleshoot and/or debug some issues with their device. > > > > What do you think? > > > I don't have a strong opinion here, but yes, that's something we could do. How about if we just downgrade the pci_warn() to a pci_info()?
On 30.03.2021 21:56, Bjorn Helgaas wrote: > On Sun, Mar 07, 2021 at 10:34:25PM +0100, Heiner Kallweit wrote: >> On 07.03.2021 19:27, Krzysztof Wilczyński wrote: >>> Hi Heiner, >>> >>>> Realtek RTL8169/8168/8125 NIC families indicate VPD capability and an >>>> optional VPD EEPROM can be connected via I2C/SPI. However I haven't >>>> seen any card or system with such a VPD EEPROM yet. The missing EEPROM >>>> causes the following warning whenever e.g. lscpi -vv is executed. >>>> >>>> invalid short VPD tag 00 at offset 01 >>>> >>>> The warning confuses users, I think we should handle the situation more >>>> gentle. Therefore, if first VPD byte is read as 0x00, assume a missing >>>> optional VPD PROM as and silently set the VPD length to 0. >>> [...] >>> >>> True. I saw people on different forum and IRC asking for clarification >>> assuming their NIC broke, or that something is wrong, so this would >>> indeed save them some worry, nice! >>> >>> Having said that, I also saw this particular warning showing up for some >>> storage controllers (often some SAS cards), so a question here: would it >>> warrant adding a pci_dbg() with an appropriate message rather than just >>> returning 0? I wonder if this might be useful for someone who is trying >>> to troubleshoot and/or debug some issues with their device. >>> >>> What do you think? >>> >> I don't have a strong opinion here, but yes, that's something we could do. > > How about if we just downgrade the pci_warn() to a pci_info()? > pci_info() would still expose a quite cryptic message to users and leave them with the question whether something is wrong. If in case of VPD tag 00 a message is desired, I'd say it should be rephrased to something like: "VPD tag 00 at offset 01, assuming missing optional VPD EPROM"
On 31.03.2021 13:14, Heiner Kallweit wrote: > On 30.03.2021 21:56, Bjorn Helgaas wrote: >> On Sun, Mar 07, 2021 at 10:34:25PM +0100, Heiner Kallweit wrote: >>> On 07.03.2021 19:27, Krzysztof Wilczyński wrote: >>>> Hi Heiner, >>>> >>>>> Realtek RTL8169/8168/8125 NIC families indicate VPD capability and an >>>>> optional VPD EEPROM can be connected via I2C/SPI. However I haven't >>>>> seen any card or system with such a VPD EEPROM yet. The missing EEPROM >>>>> causes the following warning whenever e.g. lscpi -vv is executed. >>>>> >>>>> invalid short VPD tag 00 at offset 01 >>>>> >>>>> The warning confuses users, I think we should handle the situation more >>>>> gentle. Therefore, if first VPD byte is read as 0x00, assume a missing >>>>> optional VPD PROM as and silently set the VPD length to 0. >>>> [...] >>>> >>>> True. I saw people on different forum and IRC asking for clarification >>>> assuming their NIC broke, or that something is wrong, so this would >>>> indeed save them some worry, nice! >>>> >>>> Having said that, I also saw this particular warning showing up for some >>>> storage controllers (often some SAS cards), so a question here: would it >>>> warrant adding a pci_dbg() with an appropriate message rather than just >>>> returning 0? I wonder if this might be useful for someone who is trying >>>> to troubleshoot and/or debug some issues with their device. >>>> >>>> What do you think? >>>> >>> I don't have a strong opinion here, but yes, that's something we could do. >> >> How about if we just downgrade the pci_warn() to a pci_info()? >> > pci_info() would still expose a quite cryptic message to users and leave > them with the question whether something is wrong. If in case of VPD tag 00 > a message is desired, I'd say it should be rephrased to something like: > "VPD tag 00 at offset 01, assuming missing optional VPD EPROM" > I submitted a v2 with this change.
diff --git a/drivers/pci/vpd.c b/drivers/pci/vpd.c index ef5165eb3..bd174705f 100644 --- a/drivers/pci/vpd.c +++ b/drivers/pci/vpd.c @@ -89,6 +89,10 @@ static size_t pci_vpd_size(struct pci_dev *dev, size_t old_size) pci_read_vpd(dev, off, 1, header) == 1) { unsigned char tag; + /* assume missing optional VPD PROM */ + if (!header[0] && !off) + return 0; + if (header[0] & PCI_VPD_LRDT) { /* Large Resource Data Type Tag */ tag = pci_vpd_lrdt_tag(header);
Realtek RTL8169/8168/8125 NIC families indicate VPD capability and an optional VPD EEPROM can be connected via I2C/SPI. However I haven't seen any card or system with such a VPD EEPROM yet. The missing EEPROM causes the following warning whenever e.g. lscpi -vv is executed. invalid short VPD tag 00 at offset 01 The warning confuses users, I think we should handle the situation more gentle. Therefore, if first VPD byte is read as 0x00, assume a missing optional VPD PROM as and silently set the VPD length to 0. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/pci/vpd.c | 4 ++++ 1 file changed, 4 insertions(+)