Quoting Andrew Lunn <and...@lunn.ch>:
On Wed, Jan 23, 2019 at 10:20:46PM +0100, René van Dorst wrote:
Without this patch sfp code retries to read the full struct sfp_eeprom_id
id out of the SFP eeprom. Sizeof(id) is 96 bytes.
My i2c hardware, Mediatek mt7621, has a rx buffer of 64 bytes.
So sfp_read gets -NOSUPPORTED back on his turn return -EAGAIN.
Same issue is with the SFP_EXT_STATUS data which is 92 bytes.
By split-up the request in multiple smaller requests with a max size of i2c
max_read_len, we can readout the SFP module successfully.
Tested with MT7621 and two Fiberstore modules SFP-GB-GE-T and SFP-GE-BX.
Signed-off-by: René van Dorst <opensou...@vdorst.com>
---
drivers/net/phy/sfp.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index fd8bb998ae52..1352a19571cd 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -367,7 +367,28 @@ static void sfp_set_state(struct sfp *sfp,
unsigned int state)
static int sfp_read(struct sfp *sfp, bool a2, u8 addr, void *buf,
size_t len)
{
- return sfp->read(sfp, a2, addr, buf, len);
+ const struct i2c_adapter_quirks *q = sfp->i2c->quirks;
+ int ret;
+ size_t rx_bytes = 0;
+
+ /* Many i2c hw have limited rx buffers, split-up request when needed. */
+ while ((q->max_read_len) && (len > q->max_read_len)) {
+ ret = sfp->read(sfp, a2, addr, buf, q->max_read_len);
Hi René
I think you want to pass MIN(len, q->max_read_len) to read().
Hi Andrew,
max_read_len is 0 when there is no quirk.
I can write it a bit differently depending on the outcome of my other email.
+ if (ret < 0)
+ return ret;
+ rx_bytes += ret;
+ addr += q->max_read_len;
+ buf += q->max_read_len;
+ len -= q->max_read_len;
I would prefer you add ret, not q->max_read_len. There is a danger it
returned less bytes than you asked for.
Getting less bytes then asked is already an error I think.
I could check the return size and directly return the number of bytes that I
have. The callers are checking for size and they can retry if wanted. So that
should not be an issue.
+ }
+
+ ret = sfp->read(sfp, a2, addr, buf, len);
+ if (ret < 0)
+ return ret;
+
+ rx_bytes += ret;
+
+ return rx_bytes;
}
static int sfp_write(struct sfp *sfp, bool a2, u8 addr, void *buf,
size_t len)
Doesn't write need the same handling?
By reading the SSF spec we can write to a user writable EERPOM area of
120 bytes.
But the current code has only has 1 sfp_write for a byte value.
So for now I should say no.
Greats,
René
