diff mbox series

[net-next] net: sfp: Use smaller chunk size when reading I2C data

Message ID 1559330285-30246-3-git-send-email-hancock@sedsystems.ca
State Superseded
Delegated to: David Miller
Headers show
Series [net-next] net: sfp: Use smaller chunk size when reading I2C data | expand

Commit Message

Robert Hancock May 31, 2019, 7:18 p.m. UTC
The SFP driver was reading up to 256 bytes of I2C data from the SFP
module in a single chunk. However, some I2C controllers do not support
reading that many bytes in a single transaction. Change to use a more
compatible 16-byte chunk size, since this is not performance critical.

Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
---
 drivers/net/phy/sfp.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

Comments

Russell King (Oracle) May 31, 2019, 8:13 p.m. UTC | #1
On Fri, May 31, 2019 at 01:18:03PM -0600, Robert Hancock wrote:
> The SFP driver was reading up to 256 bytes of I2C data from the SFP
> module in a single chunk. However, some I2C controllers do not support
> reading that many bytes in a single transaction. Change to use a more
> compatible 16-byte chunk size, since this is not performance critical.

This is the wrong place to fix the problem.  We still end up reading
more than 16 bytes with this approach.  I already have a patch fixing
this the right way, which is in the queue to be sent.

> 
> Signed-off-by: Robert Hancock <hancock@sedsystems.ca>
> ---
>  drivers/net/phy/sfp.c | 38 ++++++++++++++++++++++++--------------
>  1 file changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 6b6c83d..23a40a7 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -1651,7 +1651,7 @@ static int sfp_module_info(struct sfp *sfp, struct ethtool_modinfo *modinfo)
>  static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
>  			     u8 *data)
>  {
> -	unsigned int first, last, len;
> +	unsigned int first, last;
>  	int ret;
>  
>  	if (ee->len == 0)
> @@ -1659,26 +1659,36 @@ static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
>  
>  	first = ee->offset;
>  	last = ee->offset + ee->len;
> -	if (first < ETH_MODULE_SFF_8079_LEN) {
> -		len = min_t(unsigned int, last, ETH_MODULE_SFF_8079_LEN);
> -		len -= first;
>  
> -		ret = sfp_read(sfp, false, first, data, len);
> +	while (first < last) {
> +		bool a2;
> +		unsigned int this_addr, len;
> +
> +		if (first < ETH_MODULE_SFF_8079_LEN) {
> +			len = min_t(unsigned int, last,
> +				    ETH_MODULE_SFF_8079_LEN);
> +			len -= first;
> +			a2 = false;
> +			this_addr = first;
> +		} else {
> +			len = min_t(unsigned int, last,
> +				    ETH_MODULE_SFF_8472_LEN);
> +			len -= first;
> +			a2 = true;
> +			this_addr = first - ETH_MODULE_SFF_8079_LEN;
> +		}
> +		/* Some I2C adapters cannot read 256 bytes in a single read.
> +		 * Use a smaller chunk size to ensure we are within limits.
> +		 */
> +		len = min_t(unsigned int, len, 16);
> +
> +		ret = sfp_read(sfp, a2, this_addr, data, len);
>  		if (ret < 0)
>  			return ret;
>  
>  		first += len;
>  		data += len;
>  	}
> -	if (first < ETH_MODULE_SFF_8472_LEN && last > ETH_MODULE_SFF_8079_LEN) {
> -		len = min_t(unsigned int, last, ETH_MODULE_SFF_8472_LEN);
> -		len -= first;
> -		first -= ETH_MODULE_SFF_8079_LEN;
> -
> -		ret = sfp_read(sfp, true, first, data, len);
> -		if (ret < 0)
> -			return ret;
> -	}
>  	return 0;
>  }
>  
> -- 
> 1.8.3.1
> 
>
diff mbox series

Patch

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 6b6c83d..23a40a7 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -1651,7 +1651,7 @@  static int sfp_module_info(struct sfp *sfp, struct ethtool_modinfo *modinfo)
 static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
 			     u8 *data)
 {
-	unsigned int first, last, len;
+	unsigned int first, last;
 	int ret;
 
 	if (ee->len == 0)
@@ -1659,26 +1659,36 @@  static int sfp_module_eeprom(struct sfp *sfp, struct ethtool_eeprom *ee,
 
 	first = ee->offset;
 	last = ee->offset + ee->len;
-	if (first < ETH_MODULE_SFF_8079_LEN) {
-		len = min_t(unsigned int, last, ETH_MODULE_SFF_8079_LEN);
-		len -= first;
 
-		ret = sfp_read(sfp, false, first, data, len);
+	while (first < last) {
+		bool a2;
+		unsigned int this_addr, len;
+
+		if (first < ETH_MODULE_SFF_8079_LEN) {
+			len = min_t(unsigned int, last,
+				    ETH_MODULE_SFF_8079_LEN);
+			len -= first;
+			a2 = false;
+			this_addr = first;
+		} else {
+			len = min_t(unsigned int, last,
+				    ETH_MODULE_SFF_8472_LEN);
+			len -= first;
+			a2 = true;
+			this_addr = first - ETH_MODULE_SFF_8079_LEN;
+		}
+		/* Some I2C adapters cannot read 256 bytes in a single read.
+		 * Use a smaller chunk size to ensure we are within limits.
+		 */
+		len = min_t(unsigned int, len, 16);
+
+		ret = sfp_read(sfp, a2, this_addr, data, len);
 		if (ret < 0)
 			return ret;
 
 		first += len;
 		data += len;
 	}
-	if (first < ETH_MODULE_SFF_8472_LEN && last > ETH_MODULE_SFF_8079_LEN) {
-		len = min_t(unsigned int, last, ETH_MODULE_SFF_8472_LEN);
-		len -= first;
-		first -= ETH_MODULE_SFF_8079_LEN;
-
-		ret = sfp_read(sfp, true, first, data, len);
-		if (ret < 0)
-			return ret;
-	}
 	return 0;
 }