Hi Simon,

On 23/05/24 12:24 am, Simon Horman wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

On Wed, May 22, 2024 at 07:38:17PM +0530, Parthiban Veerasooran wrote:

quoted

By default, LAN9500A configures the external LEDs to the below function.
nSPD_LED -> Speed Indicator
nLNKA_LED -> Link and Activity Indicator
nFDX_LED -> Full Duplex Link Indicator

But, EVB-LAN8670-USB uses the below external LEDs function which can be
enabled by writing 1 to the LED Select (LED_SEL) bit in the LAN9500A.
nSPD_LED -> Speed Indicator
nLNKA_LED -> Link Indicator
nFDX_LED -> Activity Indicator

Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com>
---
  drivers/net/usb/smsc95xx.c | 12 ++++++++++++
  drivers/net/usb/smsc95xx.h |  1 +
  2 files changed, 13 insertions(+)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index cbea24666479..05975461bf10 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c

@@ -1006,6 +1006,18 @@ static int smsc95xx_reset(struct usbnet *dev)
       /* Configure GPIO pins as LED outputs */
       write_buf = LED_GPIO_CFG_SPD_LED | LED_GPIO_CFG_LNK_LED |
               LED_GPIO_CFG_FDX_LED;
+
+     /* Set LED Select (LED_SEL) bit for the external LED pins functionality
+      * in the Microchip's EVB-LAN8670-USB 10BASE-T1S Ethernet device which
+      * uses the below LED function.
+      * nSPD_LED -> Speed Indicator
+      * nLNKA_LED -> Link Indicator
+      * nFDX_LED -> Activity Indicator
+      */
+     if (dev->udev->descriptor.idVendor == 0x184F &&
+         dev->udev->descriptor.idProduct == 0x0051)

Hi Parthiban,

There seems to be an endian missmatch here.
The type of .idVendor and .idProduct is __le16,
but here they are compared against host byte-order integers.

Sorry, somehow I missed to check this. Will try to avoid this in the 
future. Thanks for letting me know.

But I am not going to proceed further with this patch as Woojung pointed 
out EEPROM approach can be used instead of updating the driver. So 
please discard this patch. But the EEPROM approach mentioned by Woojung 
is needed a fix in the driver to work properly. I will send out that fix 
patch separately. Please review it.

Thanks for your understanding.

Best regards,
Parthiban V

Flagged by Sparse.

quoted

+             write_buf |= LED_GPIO_CFG_LED_SEL;
+
       ret = smsc95xx_write_reg(dev, LED_GPIO_CFG, write_buf);
       if (ret < 0)
               return ret;

...

--
pw-bot: changes-requested