diff mbox series

net: usb: smsc95xx: fix changing LED_SEL bit value updated from EEPROM

Message ID 20240523085314.167650-1-Parthiban.Veerasooran@microchip.com (mailing list archive)
State Accepted
Commit 52a2f0608366a629d43dacd3191039c95fef74ba
Headers show
Series net: usb: smsc95xx: fix changing LED_SEL bit value updated from EEPROM | expand

Commit Message

Parthiban Veerasooran May 23, 2024, 8:53 a.m. UTC 
LED Select (LED_SEL) bit in the LED General Purpose IO Configuration
register is used to determine the functionality of external LED pins
(Speed Indicator, Link and Activity Indicator, Full Duplex Link
Indicator). The default value for this bit is 0 when no EEPROM is
present. If a EEPROM is present, the default value is the value of the
LED Select bit in the Configuration Flags of the EEPROM. A USB Reset or
Lite Reset (LRST) will cause this bit to be restored to the image value
last loaded from EEPROM, or to be set to 0 if no EEPROM is present.

While configuring the dual purpose GPIO/LED pins to LED outputs in the
LED General Purpose IO Configuration register, the LED_SEL bit is changed
as 0 and resulting the configured value from the EEPROM is cleared. The
issue is fixed by using read-modify-write approach.

Fixes: f293501c61c5 ("smsc95xx: configure LED outputs")
Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
---
 drivers/net/usb/smsc95xx.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)


base-commit: 4b377b4868ef17b040065bd468668c707d2477a5

Comments

Simon Horman May 23, 2024, 2:40 p.m. UTC | #1 
On Thu, May 23, 2024 at 02:23:14PM +0530, Parthiban Veerasooran wrote:
> LED Select (LED_SEL) bit in the LED General Purpose IO Configuration
> register is used to determine the functionality of external LED pins
> (Speed Indicator, Link and Activity Indicator, Full Duplex Link
> Indicator). The default value for this bit is 0 when no EEPROM is
> present. If a EEPROM is present, the default value is the value of the
> LED Select bit in the Configuration Flags of the EEPROM. A USB Reset or
> Lite Reset (LRST) will cause this bit to be restored to the image value
> last loaded from EEPROM, or to be set to 0 if no EEPROM is present.
> 
> While configuring the dual purpose GPIO/LED pins to LED outputs in the
> LED General Purpose IO Configuration register, the LED_SEL bit is changed
> as 0 and resulting the configured value from the EEPROM is cleared. The
> issue is fixed by using read-modify-write approach.
> 
> Fixes: f293501c61c5 ("smsc95xx: configure LED outputs")
> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
> ---
>  drivers/net/usb/smsc95xx.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Simon Horman <horms@kernel.org>
Woojung Huh May 23, 2024, 4:17 p.m. UTC | #2 
> -----Original Message-----
> From: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
> Sent: Thursday, May 23, 2024 4:53 AM
> To: steve.glendinning@shawell.net; UNGLinuxDriver
> <UNGLinuxDriver@microchip.com>; davem@davemloft.net; edumazet@google.com;
> kuba@kernel.org; pabeni@redhat.com
> Cc: netdev@vger.kernel.org; linux-usb@vger.kernel.org; linux-
> kernel@vger.kernel.org; Parthiban Veerasooran - I17164
> <Parthiban.Veerasooran@microchip.com>
> Subject: [PATCH] net: usb: smsc95xx: fix changing LED_SEL bit value updated
> from EEPROM
> 
> LED Select (LED_SEL) bit in the LED General Purpose IO Configuration
> register is used to determine the functionality of external LED pins
> (Speed Indicator, Link and Activity Indicator, Full Duplex Link
> Indicator). The default value for this bit is 0 when no EEPROM is
> present. If a EEPROM is present, the default value is the value of the
> LED Select bit in the Configuration Flags of the EEPROM. A USB Reset or
> Lite Reset (LRST) will cause this bit to be restored to the image value
> last loaded from EEPROM, or to be set to 0 if no EEPROM is present.
> 
> While configuring the dual purpose GPIO/LED pins to LED outputs in the
> LED General Purpose IO Configuration register, the LED_SEL bit is changed
> as 0 and resulting the configured value from the EEPROM is cleared. The
> issue is fixed by using read-modify-write approach.
> 
> Fixes: f293501c61c5 ("smsc95xx: configure LED outputs")
> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
> ---
>  drivers/net/usb/smsc95xx.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Woojung Huh <woojung.huh@microchip.com>
patchwork-bot+netdevbpf@kernel.org May 27, 2024, 11 a.m. UTC | #3 
Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Thu, 23 May 2024 14:23:14 +0530 you wrote:
> LED Select (LED_SEL) bit in the LED General Purpose IO Configuration
> register is used to determine the functionality of external LED pins
> (Speed Indicator, Link and Activity Indicator, Full Duplex Link
> Indicator). The default value for this bit is 0 when no EEPROM is
> present. If a EEPROM is present, the default value is the value of the
> LED Select bit in the Configuration Flags of the EEPROM. A USB Reset or
> Lite Reset (LRST) will cause this bit to be restored to the image value
> last loaded from EEPROM, or to be set to 0 if no EEPROM is present.
> 
> [...]

Here is the summary with links:
  - net: usb: smsc95xx: fix changing LED_SEL bit value updated from EEPROM
    https://git.kernel.org/netdev/net/c/52a2f0608366

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index cbea24666479..8e82184be5e7 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -879,7 +879,7 @@  static int smsc95xx_start_rx_path(struct usbnet *dev)
 static int smsc95xx_reset(struct usbnet *dev)
 {
 	struct smsc95xx_priv *pdata = dev->driver_priv;
-	u32 read_buf, write_buf, burst_cap;
+	u32 read_buf, burst_cap;
 	int ret = 0, timeout;
 
 	netif_dbg(dev, ifup, dev->net, "entering smsc95xx_reset\n");
@@ -1003,10 +1003,13 @@  static int smsc95xx_reset(struct usbnet *dev)
 		return ret;
 	netif_dbg(dev, ifup, dev->net, "ID_REV = 0x%08x\n", read_buf);
 
+	ret = smsc95xx_read_reg(dev, LED_GPIO_CFG, &read_buf);
+	if (ret < 0)
+		return ret;
 	/* Configure GPIO pins as LED outputs */
-	write_buf = LED_GPIO_CFG_SPD_LED | LED_GPIO_CFG_LNK_LED |
-		LED_GPIO_CFG_FDX_LED;
-	ret = smsc95xx_write_reg(dev, LED_GPIO_CFG, write_buf);
+	read_buf |= LED_GPIO_CFG_SPD_LED | LED_GPIO_CFG_LNK_LED |
+		    LED_GPIO_CFG_FDX_LED;
+	ret = smsc95xx_write_reg(dev, LED_GPIO_CFG, read_buf);
 	if (ret < 0)
 		return ret;