* [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround
@ 2020-11-23 21:53 Russell King
2020-11-24 0:18 ` kernel test robot
2020-11-24 0:20 ` Andrew Lunn
0 siblings, 2 replies; 6+ messages in thread
From: Russell King @ 2020-11-23 21:53 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit; +Cc: David S. Miller, netdev, Jakub Kicinski
Add a workaround for the VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0
GPON module which the manufacturer states needs single byte I2C reads
to the EEPROM.
Reported-by: Thomas Schreiber <tschreibe@gmail.com>
Tested-by: Thomas Schreiber <tschreibe@gmail.com>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/sfp.c | 45 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index debf91412a72..1e347afa951e 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -219,6 +219,7 @@ struct sfp {
struct sfp_bus *sfp_bus;
struct phy_device *mod_phy;
const struct sff_data *type;
+ size_t i2c_block_size;
u32 max_power_mW;
unsigned int (*get_state)(struct sfp *);
@@ -335,10 +336,19 @@ static int sfp_i2c_read(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
size_t len)
{
struct i2c_msg msgs[2];
- u8 bus_addr = a2 ? 0x51 : 0x50;
+ size_t block_size;
size_t this_len;
+ u8 bus_addr;
int ret;
+ if (a2) {
+ block_size = 16;
+ bus_addr = 0x51;
+ } else {
+ block_size = sfp->i2c_block_size;
+ bus_addr = 0x50;
+ }
+
msgs[0].addr = bus_addr;
msgs[0].flags = 0;
msgs[0].len = 1;
@@ -350,8 +360,8 @@ static int sfp_i2c_read(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
while (len) {
this_len = len;
- if (this_len > 16)
- this_len = 16;
+ if (this_len > sfp->i2c_block_size)
+ this_len = sfp->i2c_block_size;
msgs[1].len = this_len;
@@ -1673,14 +1683,20 @@ static int sfp_sm_mod_probe(struct sfp *sfp, bool report)
u8 check;
int ret;
- ret = sfp_read(sfp, false, 0, &id, sizeof(id));
+ /* Some modules (CarlitoxxPro CPGOS03-0490) do not support multibyte
+ * reads from the EEPROM, so start by reading the base identifying
+ * information one byte at a time.
+ */
+ sfp->i2c_block_size = 1;
+
+ ret = sfp_read(sfp, false, 0, &id.base, sizeof(id.base));
if (ret < 0) {
if (report)
dev_err(sfp->dev, "failed to read EEPROM: %d\n", ret);
return -EAGAIN;
}
- if (ret != sizeof(id)) {
+ if (ret != sizeof(id.base)) {
dev_err(sfp->dev, "EEPROM short read: %d\n", ret);
return -EAGAIN;
}
@@ -1719,6 +1735,25 @@ static int sfp_sm_mod_probe(struct sfp *sfp, bool report)
}
}
+ /* Some modules (Nokia 3FE46541AA) lock up if byte 0x51 is read as a
+ * single read. Switch back to reading 16 byte blocks unless we have
+ * a CarlitoxxPro module (rebranded VSOL V2801F).
+ */
+ if (memcmp(id.base.vendor_name, "VSOL ", 16))
+ sfp->i2c_block_size = 16;
+
+ ret = sfp_read(sfp, false, SFP_CC_BASE + 1, &id.ext, sizeof(id.ext));
+ if (ret < 0) {
+ if (report)
+ dev_err(sfp->dev, "failed to read EEPROM: %d\n", ret);
+ return -EAGAIN;
+ }
+
+ if (ret != sizeof(id.ext)) {
+ dev_err(sfp->dev, "EEPROM short read: %d\n", ret);
+ return -EAGAIN;
+ }
+
check = sfp_check(&id.ext, sizeof(id.ext) - 1);
if (check != id.ext.cc_ext) {
if (cotsworks) {
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround
2020-11-23 21:53 [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround Russell King
@ 2020-11-24 0:18 ` kernel test robot
2020-11-25 14:00 ` Russell King - ARM Linux admin
2020-11-24 0:20 ` Andrew Lunn
1 sibling, 1 reply; 6+ messages in thread
From: kernel test robot @ 2020-11-24 0:18 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Heiner Kallweit
Cc: kbuild-all, netdev, Jakub Kicinski
[-- Attachment #1: Type: text/plain, Size: 2855 bytes --]
Hi Russell,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Russell-King/net-sfp-VSOL-V2801F-CarlitoxxPro-CPGOS03-0490-v2-0-workaround/20201124-055921
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 8e1e33ffa696b2d779dd5cd422a80960b88e508c
config: arc-randconfig-r016-20201123 (attached as .config)
compiler: arc-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/90849b26224de3b2e508f1c3fa31665f4fd72d0a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Russell-King/net-sfp-VSOL-V2801F-CarlitoxxPro-CPGOS03-0490-v2-0-workaround/20201124-055921
git checkout 90849b26224de3b2e508f1c3fa31665f4fd72d0a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/net/phy/sfp.c: In function 'sfp_i2c_read':
>> drivers/net/phy/sfp.c:339:9: warning: variable 'block_size' set but not used [-Wunused-but-set-variable]
339 | size_t block_size;
| ^~~~~~~~~~
vim +/block_size +339 drivers/net/phy/sfp.c
334
335 static int sfp_i2c_read(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
336 size_t len)
337 {
338 struct i2c_msg msgs[2];
> 339 size_t block_size;
340 size_t this_len;
341 u8 bus_addr;
342 int ret;
343
344 if (a2) {
345 block_size = 16;
346 bus_addr = 0x51;
347 } else {
348 block_size = sfp->i2c_block_size;
349 bus_addr = 0x50;
350 }
351
352 msgs[0].addr = bus_addr;
353 msgs[0].flags = 0;
354 msgs[0].len = 1;
355 msgs[0].buf = &dev_addr;
356 msgs[1].addr = bus_addr;
357 msgs[1].flags = I2C_M_RD;
358 msgs[1].len = len;
359 msgs[1].buf = buf;
360
361 while (len) {
362 this_len = len;
363 if (this_len > sfp->i2c_block_size)
364 this_len = sfp->i2c_block_size;
365
366 msgs[1].len = this_len;
367
368 ret = i2c_transfer(sfp->i2c, msgs, ARRAY_SIZE(msgs));
369 if (ret < 0)
370 return ret;
371
372 if (ret != ARRAY_SIZE(msgs))
373 break;
374
375 msgs[1].buf += this_len;
376 dev_addr += this_len;
377 len -= this_len;
378 }
379
380 return msgs[1].buf - (u8 *)buf;
381 }
382
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26847 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround
2020-11-23 21:53 [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround Russell King
2020-11-24 0:18 ` kernel test robot
@ 2020-11-24 0:20 ` Andrew Lunn
2020-11-24 9:43 ` Russell King - ARM Linux admin
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2020-11-24 0:20 UTC (permalink / raw)
To: Russell King; +Cc: Heiner Kallweit, David S. Miller, netdev, Jakub Kicinski
> @@ -335,10 +336,19 @@ static int sfp_i2c_read(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
> size_t len)
> {
> struct i2c_msg msgs[2];
> - u8 bus_addr = a2 ? 0x51 : 0x50;
> + size_t block_size;
> size_t this_len;
> + u8 bus_addr;
> int ret;
>
> + if (a2) {
> + block_size = 16;
> + bus_addr = 0x51;
Hi Russell, Thomas
Does this man the diagnostic page can be read 16 bytes at a time, even
when the other page has to be 1 bytes at a time? That seems rather
odd. Or is the diagnostic page not implemented in these SFPs?
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround
2020-11-24 0:20 ` Andrew Lunn
@ 2020-11-24 9:43 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-24 9:43 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Heiner Kallweit, David S. Miller, netdev, Jakub Kicinski
On Tue, Nov 24, 2020 at 01:20:21AM +0100, Andrew Lunn wrote:
> > @@ -335,10 +336,19 @@ static int sfp_i2c_read(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
> > size_t len)
> > {
> > struct i2c_msg msgs[2];
> > - u8 bus_addr = a2 ? 0x51 : 0x50;
> > + size_t block_size;
> > size_t this_len;
> > + u8 bus_addr;
> > int ret;
> >
> > + if (a2) {
> > + block_size = 16;
> > + bus_addr = 0x51;
>
> Hi Russell, Thomas
>
> Does this man the diagnostic page can be read 16 bytes at a time, even
> when the other page has to be 1 bytes at a time? That seems rather
> odd. Or is the diagnostic page not implemented in these SFPs?
SFF8472 requires that multibyte values are read using sequential
reads. So we can't use single byte reads to read a multibyte value -
it's just not atomic.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround
2020-11-24 0:18 ` kernel test robot
@ 2020-11-25 14:00 ` Russell King - ARM Linux admin
2020-12-02 13:11 ` Russell King - ARM Linux admin
0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-25 14:00 UTC (permalink / raw)
To: kernel test robot
Cc: Andrew Lunn, Heiner Kallweit, kbuild-all, netdev, Jakub Kicinski
On Tue, Nov 24, 2020 at 08:18:56AM +0800, kernel test robot wrote:
> All warnings (new ones prefixed by >>):
>
> drivers/net/phy/sfp.c: In function 'sfp_i2c_read':
> >> drivers/net/phy/sfp.c:339:9: warning: variable 'block_size' set but not used [-Wunused-but-set-variable]
> 339 | size_t block_size;
> | ^~~~~~~~~~
I'm waiting for Thomas to re-test the fixed patch I sent, but Thomas
seems to be of the opinion that there's no need to re-test, despite
the fixed patch having the intended effect of changing the behaviour
on the I2C bus.
If nothing is forthcoming, I'm intending to drop the patch; we don't
need to waste time supporting untested workarounds for what are
essentially broken SFPs by vendors twisting the SFP MSA in the
kernel.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround
2020-11-25 14:00 ` Russell King - ARM Linux admin
@ 2020-12-02 13:11 ` Russell King - ARM Linux admin
0 siblings, 0 replies; 6+ messages in thread
From: Russell King - ARM Linux admin @ 2020-12-02 13:11 UTC (permalink / raw)
To: kernel test robot, Thomas Schreiber
Cc: Andrew Lunn, Heiner Kallweit, kbuild-all, netdev, Jakub Kicinski
On Wed, Nov 25, 2020 at 02:00:20PM +0000, Russell King - ARM Linux admin wrote:
> On Tue, Nov 24, 2020 at 08:18:56AM +0800, kernel test robot wrote:
> > All warnings (new ones prefixed by >>):
> >
> > drivers/net/phy/sfp.c: In function 'sfp_i2c_read':
> > >> drivers/net/phy/sfp.c:339:9: warning: variable 'block_size' set but not used [-Wunused-but-set-variable]
> > 339 | size_t block_size;
> > | ^~~~~~~~~~
>
> I'm waiting for Thomas to re-test the fixed patch I sent, but Thomas
> seems to be of the opinion that there's no need to re-test, despite
> the fixed patch having the intended effect of changing the behaviour
> on the I2C bus.
>
> If nothing is forthcoming, I'm intending to drop the patch; we don't
> need to waste time supporting untested workarounds for what are
> essentially broken SFPs by vendors twisting the SFP MSA in the
> kernel.
I have had no further co-operation from Thomas so far. If I don't hear
from someone who is able to test this module by this weekend, I will be
dropping this patch from my repository.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-02 13:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23 21:53 [PATCH net-next] net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround Russell King
2020-11-24 0:18 ` kernel test robot
2020-11-25 14:00 ` Russell King - ARM Linux admin
2020-12-02 13:11 ` Russell King - ARM Linux admin
2020-11-24 0:20 ` Andrew Lunn
2020-11-24 9:43 ` Russell King - ARM Linux admin
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).