* [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing
@ 2020-12-17 20:43 Heiner Kallweit
2021-03-07 18:27 ` Krzysztof Wilczyński
0 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2020-12-17 20:43 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: linux-pci
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(+)
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);
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing
2020-12-17 20:43 [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing Heiner Kallweit
@ 2021-03-07 18:27 ` Krzysztof Wilczyński
2021-03-07 21:34 ` Heiner Kallweit
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Wilczyński @ 2021-03-07 18:27 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Bjorn Helgaas, linux-pci
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing
2021-03-07 18:27 ` Krzysztof Wilczyński
@ 2021-03-07 21:34 ` Heiner Kallweit
2021-03-30 19:56 ` Bjorn Helgaas
0 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2021-03-07 21:34 UTC (permalink / raw)
To: Krzysztof Wilczyński; +Cc: Bjorn Helgaas, linux-pci
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing
2021-03-07 21:34 ` Heiner Kallweit
@ 2021-03-30 19:56 ` Bjorn Helgaas
2021-03-31 11:14 ` Heiner Kallweit
0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2021-03-30 19:56 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: Krzysztof Wilczyński, linux-pci
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()?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing
2021-03-30 19:56 ` Bjorn Helgaas
@ 2021-03-31 11:14 ` Heiner Kallweit
2021-04-01 12:04 ` Heiner Kallweit
0 siblings, 1 reply; 6+ messages in thread
From: Heiner Kallweit @ 2021-03-31 11:14 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Krzysztof Wilczyński, linux-pci
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"
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing
2021-03-31 11:14 ` Heiner Kallweit
@ 2021-04-01 12:04 ` Heiner Kallweit
0 siblings, 0 replies; 6+ messages in thread
From: Heiner Kallweit @ 2021-04-01 12:04 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Krzysztof Wilczyński, linux-pci
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-01 18:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-17 20:43 [PATCH] PCI/VPD: Silence warning if optional VPD PROM is missing Heiner Kallweit
2021-03-07 18:27 ` Krzysztof Wilczyński
2021-03-07 21:34 ` Heiner Kallweit
2021-03-30 19:56 ` Bjorn Helgaas
2021-03-31 11:14 ` Heiner Kallweit
2021-04-01 12:04 ` Heiner Kallweit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).