* [PATCH] sfp: allow cotsworks modules
@ 2018-03-28 10:18 Russell King
2018-03-28 10:33 ` Joe Perches
2018-03-29 18:31 ` David Miller
0 siblings, 2 replies; 7+ messages in thread
From: Russell King @ 2018-03-28 10:18 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev
Cotsworks modules fail the checksums - it appears that Cotsworks
reprograms the EEPROM at the end of production with the final product
information (serial, date code, and exact part number for module
options) and fails to update the checksum.
Work around this by detecting the Cotsworks name in the manufacturer
field, and reducing the checksum failures to warnings rather than a
hard error.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/sfp.c | 41 +++++++++++++++++++++++++++++++----------
1 file changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 83bf4959b043..4ab6e9a50bbe 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -560,6 +560,7 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
{
/* SFP module inserted - read I2C data */
struct sfp_eeprom_id id;
+ bool cotsworks;
u8 check;
int ret;
@@ -574,23 +575,43 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
return -EAGAIN;
}
+ /* Cotsworks do not seem to update the checksums when they
+ * do the final programming with the final module part number,
+ * serial number and date code.
+ */
+ cotsworks = !memcmp(id.base.vendor_name, "COTSWORKS ", 16);
+
/* Validate the checksum over the base structure */
check = sfp_check(&id.base, sizeof(id.base) - 1);
if (check != id.base.cc_base) {
- dev_err(sfp->dev,
- "EEPROM base structure checksum failure: 0x%02x\n",
- check);
- print_hex_dump(KERN_ERR, "sfp EE: ", DUMP_PREFIX_OFFSET,
- 16, 1, &id, sizeof(id.base) - 1, true);
- return -EINVAL;
+ if (cotsworks) {
+ dev_warn(sfp->dev,
+ "EEPROM base structure checksum failure (0x%02x != 0x%02x)\n",
+ check, id.base.cc_base);
+ } else {
+ dev_err(sfp->dev,
+ "EEPROM base structure checksum failure: 0x%02x != 0x%02x\n",
+ check, id.base.cc_base);
+ print_hex_dump(KERN_ERR, "sfp EE: ", DUMP_PREFIX_OFFSET,
+ 16, 1, &id, sizeof(id), true);
+ return -EINVAL;
+ }
}
check = sfp_check(&id.ext, sizeof(id.ext) - 1);
if (check != id.ext.cc_ext) {
- dev_err(sfp->dev,
- "EEPROM extended structure checksum failure: 0x%02x\n",
- check);
- memset(&id.ext, 0, sizeof(id.ext));
+ if (cotsworks) {
+ dev_warn(sfp->dev,
+ "EEPROM extended structure checksum failure (0x%02x != 0x%02x)\n",
+ check, id.ext.cc_ext);
+ } else {
+ dev_err(sfp->dev,
+ "EEPROM extended structure checksum failure: 0x%02x != 0x%02x\n",
+ check, id.ext.cc_ext);
+ print_hex_dump(KERN_ERR, "sfp EE: ", DUMP_PREFIX_OFFSET,
+ 16, 1, &id, sizeof(id), true);
+ memset(&id.ext, 0, sizeof(id.ext));
+ }
}
sfp->id = id;
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] sfp: allow cotsworks modules
2018-03-28 10:18 [PATCH] sfp: allow cotsworks modules Russell King
@ 2018-03-28 10:33 ` Joe Perches
2018-03-28 10:41 ` Russell King - ARM Linux
2018-03-29 18:31 ` David Miller
1 sibling, 1 reply; 7+ messages in thread
From: Joe Perches @ 2018-03-28 10:33 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Florian Fainelli; +Cc: netdev
On Wed, 2018-03-28 at 11:18 +0100, Russell King wrote:
> Cotsworks modules fail the checksums - it appears that Cotsworks
> reprograms the EEPROM at the end of production with the final product
> information (serial, date code, and exact part number for module
> options) and fails to update the checksum.
trivia:
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
[]
> @@ -574,23 +575,43 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
[]
> + if (cotsworks) {
> + dev_warn(sfp->dev,
> + "EEPROM base structure checksum failure (0x%02x != 0x%02x)\n",
> + check, id.base.cc_base);
> + } else {
> + dev_err(sfp->dev,
> + "EEPROM base structure checksum failure: 0x%02x != 0x%02x\n",
It'd be better to move this above the if and
use only a single format string instead of
using 2 slightly different formats.
> + check, id.base.cc_base);
> + print_hex_dump(KERN_ERR, "sfp EE: ", DUMP_PREFIX_OFFSET,
> + 16, 1, &id, sizeof(id), true);
> + return -EINVAL;
> + }
> }
>
> check = sfp_check(&id.ext, sizeof(id.ext) - 1);
> if (check != id.ext.cc_ext) {
> - dev_err(sfp->dev,
> - "EEPROM extended structure checksum failure: 0x%02x\n",
> - check);
> - memset(&id.ext, 0, sizeof(id.ext));
> + if (cotsworks) {
> + dev_warn(sfp->dev,
> + "EEPROM extended structure checksum failure (0x%02x != 0x%02x)\n",
> + check, id.ext.cc_ext);
> + } else {
> + dev_err(sfp->dev,
> + "EEPROM extended structure checksum failure: 0x%02x != 0x%02x\n",
> + check, id.ext.cc_ext);
here too
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sfp: allow cotsworks modules
2018-03-28 10:33 ` Joe Perches
@ 2018-03-28 10:41 ` Russell King - ARM Linux
2018-03-28 16:19 ` Joe Perches
0 siblings, 1 reply; 7+ messages in thread
From: Russell King - ARM Linux @ 2018-03-28 10:41 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Lunn, Florian Fainelli, netdev
On Wed, Mar 28, 2018 at 03:33:57AM -0700, Joe Perches wrote:
> On Wed, 2018-03-28 at 11:18 +0100, Russell King wrote:
> > Cotsworks modules fail the checksums - it appears that Cotsworks
> > reprograms the EEPROM at the end of production with the final product
> > information (serial, date code, and exact part number for module
> > options) and fails to update the checksum.
>
> trivia:
>
> > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> []
> > @@ -574,23 +575,43 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
> []
> > + if (cotsworks) {
> > + dev_warn(sfp->dev,
> > + "EEPROM base structure checksum failure (0x%02x != 0x%02x)\n",
> > + check, id.base.cc_base);
> > + } else {
> > + dev_err(sfp->dev,
> > + "EEPROM base structure checksum failure: 0x%02x != 0x%02x\n",
>
> It'd be better to move this above the if and
> use only a single format string instead of
> using 2 slightly different formats.
No. I think you've missed the fact that one is a _warning_ the other is
an _error_ and they are emitted at the appropriate severity. It's not
just that the format strings are slightly different.
>
> > + check, id.base.cc_base);
> > + print_hex_dump(KERN_ERR, "sfp EE: ", DUMP_PREFIX_OFFSET,
> > + 16, 1, &id, sizeof(id), true);
> > + return -EINVAL;
> > + }
> > }
> >
> > check = sfp_check(&id.ext, sizeof(id.ext) - 1);
> > if (check != id.ext.cc_ext) {
> > - dev_err(sfp->dev,
> > - "EEPROM extended structure checksum failure: 0x%02x\n",
> > - check);
> > - memset(&id.ext, 0, sizeof(id.ext));
> > + if (cotsworks) {
> > + dev_warn(sfp->dev,
> > + "EEPROM extended structure checksum failure (0x%02x != 0x%02x)\n",
> > + check, id.ext.cc_ext);
> > + } else {
> > + dev_err(sfp->dev,
> > + "EEPROM extended structure checksum failure: 0x%02x != 0x%02x\n",
> > + check, id.ext.cc_ext);
>
>
> here too
Same applies.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sfp: allow cotsworks modules
2018-03-28 10:41 ` Russell King - ARM Linux
@ 2018-03-28 16:19 ` Joe Perches
2018-03-28 16:51 ` Russell King - ARM Linux
0 siblings, 1 reply; 7+ messages in thread
From: Joe Perches @ 2018-03-28 16:19 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Andrew Lunn, Florian Fainelli, netdev
On Wed, 2018-03-28 at 11:41 +0100, Russell King - ARM Linux wrote:
> On Wed, Mar 28, 2018 at 03:33:57AM -0700, Joe Perches wrote:
> > On Wed, 2018-03-28 at 11:18 +0100, Russell King wrote:
> > > Cotsworks modules fail the checksums - it appears that Cotsworks
> > > reprograms the EEPROM at the end of production with the final product
> > > information (serial, date code, and exact part number for module
> > > options) and fails to update the checksum.
> >
> > trivia:
> >
> > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> >
> > []
> > > @@ -574,23 +575,43 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
> >
> > []
> > > + if (cotsworks) {
> > > + dev_warn(sfp->dev,
> > > + "EEPROM base structure checksum failure (0x%02x != 0x%02x)\n",
> > > + check, id.base.cc_base);
> > > + } else {
> > > + dev_err(sfp->dev,
> > > + "EEPROM base structure checksum failure: 0x%02x != 0x%02x\n",
> >
> > It'd be better to move this above the if and
> > use only a single format string instead of
> > using 2 slightly different formats.
>
> No. I think you've missed the fact that one is a _warning_ the other is
> an _error_ and they are emitted at the appropriate severity. It's not
> just that the format strings are slightly different.
Right. Still nicer to use the same formats.
cheers, Joe
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sfp: allow cotsworks modules
2018-03-28 16:19 ` Joe Perches
@ 2018-03-28 16:51 ` Russell King - ARM Linux
2018-03-28 17:05 ` Joe Perches
0 siblings, 1 reply; 7+ messages in thread
From: Russell King - ARM Linux @ 2018-03-28 16:51 UTC (permalink / raw)
To: Joe Perches; +Cc: Andrew Lunn, Florian Fainelli, netdev
On Wed, Mar 28, 2018 at 09:19:01AM -0700, Joe Perches wrote:
> On Wed, 2018-03-28 at 11:41 +0100, Russell King - ARM Linux wrote:
> > On Wed, Mar 28, 2018 at 03:33:57AM -0700, Joe Perches wrote:
> > > On Wed, 2018-03-28 at 11:18 +0100, Russell King wrote:
> > > > Cotsworks modules fail the checksums - it appears that Cotsworks
> > > > reprograms the EEPROM at the end of production with the final product
> > > > information (serial, date code, and exact part number for module
> > > > options) and fails to update the checksum.
> > >
> > > trivia:
> > >
> > > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > >
> > > []
> > > > @@ -574,23 +575,43 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
> > >
> > > []
> > > > + if (cotsworks) {
> > > > + dev_warn(sfp->dev,
> > > > + "EEPROM base structure checksum failure (0x%02x != 0x%02x)\n",
> > > > + check, id.base.cc_base);
> > > > + } else {
> > > > + dev_err(sfp->dev,
> > > > + "EEPROM base structure checksum failure: 0x%02x != 0x%02x\n",
> > >
> > > It'd be better to move this above the if and
> > > use only a single format string instead of
> > > using 2 slightly different formats.
> >
> > No. I think you've missed the fact that one is a _warning_ the other is
> > an _error_ and they are emitted at the appropriate severity. It's not
> > just that the format strings are slightly different.
>
> Right. Still nicer to use the same formats.
I'll stick a "Warning:" and "Error:" tag before them if you really
want the rest of the message to be identically formatted - otherwise,
when seeing reports from people's dmesg, there will be nothing to
indicate which message was printed.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] sfp: allow cotsworks modules
2018-03-28 16:51 ` Russell King - ARM Linux
@ 2018-03-28 17:05 ` Joe Perches
0 siblings, 0 replies; 7+ messages in thread
From: Joe Perches @ 2018-03-28 17:05 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Andrew Lunn, Florian Fainelli, netdev
On Wed, 2018-03-28 at 17:51 +0100, Russell King - ARM Linux wrote:
> On Wed, Mar 28, 2018 at 09:19:01AM -0700, Joe Perches wrote:
> > On Wed, 2018-03-28 at 11:41 +0100, Russell King - ARM Linux wrote:
> > > On Wed, Mar 28, 2018 at 03:33:57AM -0700, Joe Perches wrote:
> > > > On Wed, 2018-03-28 at 11:18 +0100, Russell King wrote:
> > > > > Cotsworks modules fail the checksums - it appears that Cotsworks
> > > > > reprograms the EEPROM at the end of production with the final product
> > > > > information (serial, date code, and exact part number for module
> > > > > options) and fails to update the checksum.
> > > >
> > > > trivia:
> > > >
> > > > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> > > >
> > > > []
> > > > > @@ -574,23 +575,43 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
> > > >
> > > > []
> > > > > + if (cotsworks) {
> > > > > + dev_warn(sfp->dev,
> > > > > + "EEPROM base structure checksum failure (0x%02x != 0x%02x)\n",
> > > > > + check, id.base.cc_base);
> > > > > + } else {
> > > > > + dev_err(sfp->dev,
> > > > > + "EEPROM base structure checksum failure: 0x%02x != 0x%02x\n",
> > > >
> > > > It'd be better to move this above the if and
> > > > use only a single format string instead of
> > > > using 2 slightly different formats.
> > >
> > > No. I think you've missed the fact that one is a _warning_ the other is
> > > an _error_ and they are emitted at the appropriate severity. It's not
> > > just that the format strings are slightly different.
> >
> > Right. Still nicer to use the same formats.
>
> I'll stick a "Warning:" and "Error:" tag before them if you really
> want the rest of the message to be identically formatted - otherwise,
> when seeing reports from people's dmesg, there will be nothing to
> indicate which message was printed.
Not necessary. It was just a trivial size saving
from the format deduplication.
There is another dmesg info line
dev_info(sfp->dev, "module %.*s %.*s rev %.*s sn %.*s dc %.*s\n",
(int)sizeof(id.base.vendor_name), id.base.vendor_name,
(int)sizeof(id.base.vendor_pn), id.base.vendor_pn,
(int)sizeof(id.base.vendor_rev), id.base.vendor_rev,
(int)sizeof(id.ext.vendor_sn), id.ext.vendor_sn,
(int)sizeof(id.ext.datecode), id.ext.datecode);
the next line later which shows that it's a "COTSWORKS ".
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] sfp: allow cotsworks modules
2018-03-28 10:18 [PATCH] sfp: allow cotsworks modules Russell King
2018-03-28 10:33 ` Joe Perches
@ 2018-03-29 18:31 ` David Miller
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2018-03-29 18:31 UTC (permalink / raw)
To: rmk+kernel; +Cc: andrew, f.fainelli, netdev
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Wed, 28 Mar 2018 11:18:25 +0100
> Cotsworks modules fail the checksums - it appears that Cotsworks
> reprograms the EEPROM at the end of production with the final product
> information (serial, date code, and exact part number for module
> options) and fails to update the checksum.
>
> Work around this by detecting the Cotsworks name in the manufacturer
> field, and reducing the checksum failures to warnings rather than a
> hard error.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied, thanks Russell.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-03-29 18:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 10:18 [PATCH] sfp: allow cotsworks modules Russell King
2018-03-28 10:33 ` Joe Perches
2018-03-28 10:41 ` Russell King - ARM Linux
2018-03-28 16:19 ` Joe Perches
2018-03-28 16:51 ` Russell King - ARM Linux
2018-03-28 17:05 ` Joe Perches
2018-03-29 18:31 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.