[PATCH v2] net: usb: sr9700: fix incorrect command used to write single register

[PATCH v2] net: usb: sr9700: fix incorrect command used to write single register

[Ethan Nelson-Moore]

This fixes the device failing to initialize with "error reading MAC
address" for me, probably because the incorrect write of NCR_RST to
SR_NCR is not actually resetting the device.

Fixes: c9b37458e95629b1d1171457afdcc1bf1eb7881d ("USB2NET : SR9700 : One chip USB 1.1 USB2NET SR9700Device Driver Support")
Cc: stable@vger.kernel.org
Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
---
 drivers/net/usb/sr9700.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
index 091bc2aca7e8..5d97e95a17b0 100644
--- a/drivers/net/usb/sr9700.c
+++ b/drivers/net/usb/sr9700.c
@@ -52,7 +52,7 @@ static int sr_read_reg(struct usbnet *dev, u8 reg, u8 *value)
 
 static int sr_write_reg(struct usbnet *dev, u8 reg, u8 value)
 {
-	return usbnet_write_cmd(dev, SR_WR_REGS, SR_REQ_WR_REG,
+	return usbnet_write_cmd(dev, SR_WR_REG, SR_REQ_WR_REG,
 				value, reg, NULL, 0);
 }
 
@@ -65,7 +65,7 @@ static void sr_write_async(struct usbnet *dev, u8 reg, u16 length,
 
 static void sr_write_reg_async(struct usbnet *dev, u8 reg, u8 value)
 {
-	usbnet_write_cmd_async(dev, SR_WR_REGS, SR_REQ_WR_REG,
+	usbnet_write_cmd_async(dev, SR_WR_REG, SR_REQ_WR_REG,
 			       value, reg, NULL, 0);
 }
 
-- 
2.43.0

Re: [PATCH v2] net: usb: sr9700: fix incorrect command used to write single register

[Andrew Lunn]

On Sun, Dec 21, 2025 at 12:24:00AM -0800, Ethan Nelson-Moore wrote:
> This fixes the device failing to initialize with "error reading MAC
> address" for me, probably because the incorrect write of NCR_RST to
> SR_NCR is not actually resetting the device.
> 
> Fixes: c9b37458e95629b1d1171457afdcc1bf1eb7881d ("USB2NET : SR9700 : One chip USB 1.1 USB2NET SR9700Device Driver Support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
> ---
>  drivers/net/usb/sr9700.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
> index 091bc2aca7e8..5d97e95a17b0 100644
> --- a/drivers/net/usb/sr9700.c
> +++ b/drivers/net/usb/sr9700.c
> @@ -52,7 +52,7 @@ static int sr_read_reg(struct usbnet *dev, u8 reg, u8 *value)
>  
>  static int sr_write_reg(struct usbnet *dev, u8 reg, u8 value)
>  {
> -	return usbnet_write_cmd(dev, SR_WR_REGS, SR_REQ_WR_REG,
> +	return usbnet_write_cmd(dev, SR_WR_REG, SR_REQ_WR_REG,
>  				value, reg, NULL, 0);
>  }
>  
> @@ -65,7 +65,7 @@ static void sr_write_async(struct usbnet *dev, u8 reg, u16 length,
>  
>  static void sr_write_reg_async(struct usbnet *dev, u8 reg, u8 value)
>  {
> -	usbnet_write_cmd_async(dev, SR_WR_REGS, SR_REQ_WR_REG,
> +	usbnet_write_cmd_async(dev, SR_WR_REG, SR_REQ_WR_REG,
>  			       value, reg, NULL, 0);
>  }

I don't know anything about this hardware, but there are four calls using SR_WR_REG:

https://elixir.bootlin.com/linux/v6.18.2/source/drivers/net/usb/sr9700.h#L157

You only change two here? Are the other two correct?

It might be worth while also changing the name of one of these:

#define	SR_WR_REGS		0x01
#define	SR_WR_REG		0x03

to make it clearer what each is actually used for, so they don't get
used wrongly again.

	Andrew

Re: [PATCH v2] net: usb: sr9700: fix incorrect command used to write single register

[Ethan Nelson-Moore]

Hi, Andrew,

The other two are correct because they intend to write multiple
registers - they are used with a length parameter.

Ethan

On Sun, Dec 21, 2025 at 4:34 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sun, Dec 21, 2025 at 12:24:00AM -0800, Ethan Nelson-Moore wrote:
> > This fixes the device failing to initialize with "error reading MAC
> > address" for me, probably because the incorrect write of NCR_RST to
> > SR_NCR is not actually resetting the device.
> >
> > Fixes: c9b37458e95629b1d1171457afdcc1bf1eb7881d ("USB2NET : SR9700 : One chip USB 1.1 USB2NET SR9700Device Driver Support")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
> > ---
> >  drivers/net/usb/sr9700.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/usb/sr9700.c b/drivers/net/usb/sr9700.c
> > index 091bc2aca7e8..5d97e95a17b0 100644
> > --- a/drivers/net/usb/sr9700.c
> > +++ b/drivers/net/usb/sr9700.c
> > @@ -52,7 +52,7 @@ static int sr_read_reg(struct usbnet *dev, u8 reg, u8 *value)
> >
> >  static int sr_write_reg(struct usbnet *dev, u8 reg, u8 value)
> >  {
> > -     return usbnet_write_cmd(dev, SR_WR_REGS, SR_REQ_WR_REG,
> > +     return usbnet_write_cmd(dev, SR_WR_REG, SR_REQ_WR_REG,
> >                               value, reg, NULL, 0);
> >  }
> >
> > @@ -65,7 +65,7 @@ static void sr_write_async(struct usbnet *dev, u8 reg, u16 length,
> >
> >  static void sr_write_reg_async(struct usbnet *dev, u8 reg, u8 value)
> >  {
> > -     usbnet_write_cmd_async(dev, SR_WR_REGS, SR_REQ_WR_REG,
> > +     usbnet_write_cmd_async(dev, SR_WR_REG, SR_REQ_WR_REG,
> >                              value, reg, NULL, 0);
> >  }
>
> I don't know anything about this hardware, but there are four calls using SR_WR_REG:
>
> https://elixir.bootlin.com/linux/v6.18.2/source/drivers/net/usb/sr9700.h#L157
>
> You only change two here? Are the other two correct?
>
> It might be worth while also changing the name of one of these:
>
> #define SR_WR_REGS              0x01
> #define SR_WR_REG               0x03
>
> to make it clearer what each is actually used for, so they don't get
> used wrongly again.
>
>         Andrew

Re: [PATCH v2] net: usb: sr9700: fix incorrect command used to write single register

[Andrew Lunn]

On Sun, Dec 21, 2025 at 03:42:58PM -0800, Ethan Nelson-Moore wrote:
> Hi, Andrew,
> 
> The other two are correct because they intend to write multiple registers -
> they are used with a length parameter. 

Please don't top post.

How finished do you think this driver is? Are there likely to be more
instances of SR_WR_REG/SR_WR_REGS added in the future? If so, it might
make sense to change the code to make this sort of error less likely.

SR_WR_MULTIPLE_REG and SR_WR_ONE_REG?

		   Andrew

Re: [PATCH v2] net: usb: sr9700: fix incorrect command used to write single register

[Ethan Nelson-Moore]

On Mon, Dec 22, 2025 at 1:07 AM Andrew Lunn <andrew@lunn.ch> wrote:
> Please don't top post.

Sorry. I will keep that in mind in the future.

> How finished do you think this driver is? Are there likely to be more
> instances of SR_WR_REG/SR_WR_REGS added in the future? If so, it might
> make sense to change the code to make this sort of error less likely.
>
> SR_WR_MULTIPLE_REG and SR_WR_ONE_REG?

That's a good idea. This driver has significant functional issues
because it is a copy of the dm9601 driver, but the hardware, while
generally a DM9601 clone, does not support some features of the DM9601
(for example, it has no MII or multicast filter registers). I am
probing the registers to check which registers/bits are present and
work as expected (going by the DM9601 datasheet). When I finish doing
so and submit a patch to fix these issues, I will change the names of
those constants.

Re: [PATCH v2] net: usb: sr9700: fix incorrect command used to write single register

[patchwork-bot+netdevbpf]

Hello:

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

On Sun, 21 Dec 2025 00:24:00 -0800 you wrote:
> This fixes the device failing to initialize with "error reading MAC
> address" for me, probably because the incorrect write of NCR_RST to
> SR_NCR is not actually resetting the device.
> 
> Fixes: c9b37458e95629b1d1171457afdcc1bf1eb7881d ("USB2NET : SR9700 : One chip USB 1.1 USB2NET SR9700Device Driver Support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ethan Nelson-Moore <enelsonmoore@gmail.com>
> 
> [...]

Here is the summary with links:
  - [v2] net: usb: sr9700: fix incorrect command used to write single register
    https://git.kernel.org/netdev/net/c/fa0b198be1c6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html