The patchset is to improve EEPROM read requests when SFP module is
absent.
ChangeLog:
v1:
https://lore.kernel.org/netdev/[email protected]/
v2:
* reword commit message of "net: sfp: initialize sfp->i2c_block_size
at sfp allocation"
* add second patch to eliminate excessive I2C transfers in
sfp_module_eeprom() and sfp_module_eeprom_by_page()
Ivan Bornyakov (2):
net: sfp: initialize sfp->i2c_block_size at sfp allocation
net: sfp: avoid EEPROM read of absent SFP module
drivers/net/phy/sfp.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
--
2.39.2
sfp->i2c_block_size is initialized at SFP module insertion in
sfp_sm_mod_probe(). Because of that, if SFP module was never inserted
since boot, sfp_read() call will lead to zero-length I2C read attempt,
and not all I2C controllers are happy with zero-length reads.
One way to issue sfp_read() on empty SFP cage is to execute ethtool -m.
If SFP module was never plugged since boot, there will be a zero-length
I2C read attempt.
# ethtool -m xge0
i2c i2c-3: adapter quirk: no zero length (addr 0x0050, size 0, read)
Cannot get Module EEPROM data: Operation not supported
If SFP module was plugged then removed at least once,
sfp->i2c_block_size will be initialized and ethtool -m will fail with
different exit code and without I2C error
# ethtool -m xge0
Cannot get Module EEPROM data: Remote I/O error
Fix this by initializing sfp->i2_block_size at struct sfp allocation
stage so no wild sfp_read() could issue zero-length I2C read.
Signed-off-by: Ivan Bornyakov <[email protected]>
Fixes: 0d035bed2a4a ("net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround")
Cc: [email protected]
---
drivers/net/phy/sfp.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 40c9a64c5e30..5663a184644d 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -212,6 +212,12 @@ static const enum gpiod_flags gpio_flags[] = {
#define SFP_PHY_ADDR 22
#define SFP_PHY_ADDR_ROLLBALL 17
+/* SFP_EEPROM_BLOCK_SIZE is the size of data chunk to read the EEPROM
+ * at a time. Some SFP modules and also some Linux I2C drivers do not like
+ * reads longer than 16 bytes.
+ */
+#define SFP_EEPROM_BLOCK_SIZE 16
+
struct sff_data {
unsigned int gpios;
bool (*module_supported)(const struct sfp_eeprom_id *id);
@@ -1928,11 +1934,7 @@ static int sfp_sm_mod_probe(struct sfp *sfp, bool report)
u8 check;
int ret;
- /* Some SFP modules and also some Linux I2C drivers do not like reads
- * longer than 16 bytes, so read the EEPROM in chunks of 16 bytes at
- * a time.
- */
- sfp->i2c_block_size = 16;
+ sfp->i2c_block_size = SFP_EEPROM_BLOCK_SIZE;
ret = sfp_read(sfp, false, 0, &id.base, sizeof(id.base));
if (ret < 0) {
@@ -2615,6 +2617,7 @@ static struct sfp *sfp_alloc(struct device *dev)
return ERR_PTR(-ENOMEM);
sfp->dev = dev;
+ sfp->i2c_block_size = SFP_EEPROM_BLOCK_SIZE;
mutex_init(&sfp->sm_mutex);
mutex_init(&sfp->st_mutex);
--
2.39.2
If SFP module is not present, it is sensible to fail sfp_module_eeprom()
and sfp_module_eeprom_by_page() early to avoid excessive I2C transfers
which are garanteed to fail.
Suggested-by: Andrew Lunn <[email protected]>
Signed-off-by: Ivan Bornyakov <[email protected]>
---
drivers/net/phy/sfp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 5663a184644d..6f32c2ab415d 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -2481,6 +2481,9 @@ static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
unsigned int first, last, len;
int ret;
+ if (!(sfp->state & SFP_F_PRESENT))
+ return -ENODEV;
+
if (ee->len == 0)
return -EINVAL;
@@ -2513,6 +2516,9 @@ static int sfp_module_eeprom_by_page(struct sfp *sfp,
const struct ethtool_module_eeprom *page,
struct netlink_ext_ack *extack)
{
+ if (!(sfp->state & SFP_F_PRESENT))
+ return -ENODEV;
+
if (page->bank) {
NL_SET_ERR_MSG(extack, "Banks not supported");
return -EOPNOTSUPP;
--
2.39.2
On Thu, Apr 06, 2023 at 04:08:33PM +0300, Ivan Bornyakov wrote:
> If SFP module is not present, it is sensible to fail sfp_module_eeprom()
> and sfp_module_eeprom_by_page() early to avoid excessive I2C transfers
> which are garanteed to fail.
>
> Suggested-by: Andrew Lunn <[email protected]>
> Signed-off-by: Ivan Bornyakov <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
On Thu, Apr 06, 2023 at 04:08:32PM +0300, Ivan Bornyakov wrote:
> sfp->i2c_block_size is initialized at SFP module insertion in
> sfp_sm_mod_probe(). Because of that, if SFP module was never inserted
> since boot, sfp_read() call will lead to zero-length I2C read attempt,
> and not all I2C controllers are happy with zero-length reads.
>
> One way to issue sfp_read() on empty SFP cage is to execute ethtool -m.
> If SFP module was never plugged since boot, there will be a zero-length
> I2C read attempt.
>
> # ethtool -m xge0
> i2c i2c-3: adapter quirk: no zero length (addr 0x0050, size 0, read)
> Cannot get Module EEPROM data: Operation not supported
>
> If SFP module was plugged then removed at least once,
> sfp->i2c_block_size will be initialized and ethtool -m will fail with
> different exit code and without I2C error
>
> # ethtool -m xge0
> Cannot get Module EEPROM data: Remote I/O error
>
> Fix this by initializing sfp->i2_block_size at struct sfp allocation
> stage so no wild sfp_read() could issue zero-length I2C read.
>
> Signed-off-by: Ivan Bornyakov <[email protected]>
> Fixes: 0d035bed2a4a ("net: sfp: VSOL V2801F / CarlitoxxPro CPGOS03-0490 v2.0 workaround")
> Cc: [email protected]
Reviewed-by: Andrew Lunn <[email protected]>
Andrew
Hello:
This series was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:
On Thu, 6 Apr 2023 16:08:31 +0300 you wrote:
> The patchset is to improve EEPROM read requests when SFP module is
> absent.
>
> ChangeLog:
> v1:
> https://lore.kernel.org/netdev/[email protected]/
> v2:
> * reword commit message of "net: sfp: initialize sfp->i2c_block_size
> at sfp allocation"
> * add second patch to eliminate excessive I2C transfers in
> sfp_module_eeprom() and sfp_module_eeprom_by_page()
>
> [...]
Here is the summary with links:
- [net,v2,1/2] net: sfp: initialize sfp->i2c_block_size at sfp allocation
https://git.kernel.org/netdev/net/c/813c2dd78618
- [net,v2,2/2] net: sfp: avoid EEPROM read of absent SFP module
https://git.kernel.org/netdev/net/c/bef227c1537c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html