[PATCH net] net: txgbe: change hw reset mode

[PATCH net] net: txgbe: change hw reset mode

[Jiawen Wu]

The old way to do hardware reset is sending reset command to firmware.
In order to adapt to the new firmware, driver directly write register
of LAN reset instead of the old way. And remove the redundant functions
wx_reset_hostif() and wx_calculate_checksum().

Fixes: b08012568ebb ("net: txgbe: Reset hardware")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/libwx/wx_hw.c    | 65 -------------------
 drivers/net/ethernet/wangxun/libwx/wx_hw.h    |  1 -
 drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c |  8 +--
 3 files changed, 4 insertions(+), 70 deletions(-)

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
index ca409b4054d0..6cf49db55938 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
+++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
@@ -431,71 +431,6 @@ int wx_read_ee_hostif_buffer(struct wx *wx,
 }
 EXPORT_SYMBOL(wx_read_ee_hostif_buffer);
 
-/**
- *  wx_calculate_checksum - Calculate checksum for buffer
- *  @buffer: pointer to EEPROM
- *  @length: size of EEPROM to calculate a checksum for
- *  Calculates the checksum for some buffer on a specified length.  The
- *  checksum calculated is returned.
- **/
-static u8 wx_calculate_checksum(u8 *buffer, u32 length)
-{
-	u8 sum = 0;
-	u32 i;
-
-	if (!buffer)
-		return 0;
-
-	for (i = 0; i < length; i++)
-		sum += buffer[i];
-
-	return (u8)(0 - sum);
-}
-
-/**
- *  wx_reset_hostif - send reset cmd to fw
- *  @wx: pointer to hardware structure
- *
- *  Sends reset cmd to firmware through the manageability
- *  block.
- **/
-int wx_reset_hostif(struct wx *wx)
-{
-	struct wx_hic_reset reset_cmd;
-	int ret_val = 0;
-	int i;
-
-	reset_cmd.hdr.cmd = FW_RESET_CMD;
-	reset_cmd.hdr.buf_len = FW_RESET_LEN;
-	reset_cmd.hdr.cmd_or_resp.cmd_resv = FW_CEM_CMD_RESERVED;
-	reset_cmd.lan_id = wx->bus.func;
-	reset_cmd.reset_type = (u16)wx->reset_type;
-	reset_cmd.hdr.checksum = 0;
-	reset_cmd.hdr.checksum = wx_calculate_checksum((u8 *)&reset_cmd,
-						       (FW_CEM_HDR_LEN +
-							reset_cmd.hdr.buf_len));
-
-	for (i = 0; i <= FW_CEM_MAX_RETRIES; i++) {
-		ret_val = wx_host_interface_command(wx, (u32 *)&reset_cmd,
-						    sizeof(reset_cmd),
-						    WX_HI_COMMAND_TIMEOUT,
-						    true);
-		if (ret_val != 0)
-			continue;
-
-		if (reset_cmd.hdr.cmd_or_resp.ret_status ==
-		    FW_CEM_RESP_STATUS_SUCCESS)
-			ret_val = 0;
-		else
-			ret_val = -EFAULT;
-
-		break;
-	}
-
-	return ret_val;
-}
-EXPORT_SYMBOL(wx_reset_hostif);
-
 /**
  *  wx_init_eeprom_params - Initialize EEPROM params
  *  @wx: pointer to hardware structure
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.h b/drivers/net/ethernet/wangxun/libwx/wx_hw.h
index c173c56f0ab5..9faacf0c51d1 100644
--- a/drivers/net/ethernet/wangxun/libwx/wx_hw.h
+++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.h
@@ -14,7 +14,6 @@ int wx_host_interface_command(struct wx *wx, u32 *buffer,
 int wx_read_ee_hostif(struct wx *wx, u16 offset, u16 *data);
 int wx_read_ee_hostif_buffer(struct wx *wx,
 			     u16 offset, u16 words, u16 *data);
-int wx_reset_hostif(struct wx *wx);
 void wx_init_eeprom_params(struct wx *wx);
 void wx_get_mac_addr(struct wx *wx, u8 *mac_addr);
 void wx_init_rx_addrs(struct wx *wx);
diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
index ebc46f3be056..e571f494bb4a 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
@@ -270,16 +270,16 @@ static void txgbe_reset_misc(struct wx *wx)
 int txgbe_reset_hw(struct wx *wx)
 {
 	int status;
+	u32 val;
 
 	/* Call adapter stop to disable tx/rx and clear interrupts */
 	status = wx_stop_adapter(wx);
 	if (status != 0)
 		return status;
 
-	if (!(((wx->subsystem_device_id & WX_NCSI_MASK) == WX_NCSI_SUP) ||
-	      ((wx->subsystem_device_id & WX_WOL_MASK) == WX_WOL_SUP)))
-		wx_reset_hostif(wx);
-
+	val = WX_MIS_RST_LAN_RST(wx->bus.func);
+	wr32(wx, WX_MIS_RST, val | rd32(wx, WX_MIS_RST));
+	WX_WRITE_FLUSH(wx);
 	usleep_range(10, 100);
 
 	status = wx_check_flash_load(wx, TXGBE_SPI_ILDR_STATUS_LAN_SW_RST(wx->bus.func));
-- 
2.27.0

Re: [PATCH net] net: txgbe: change hw reset mode

[Jakub Kicinski]

On Wed, 21 Jun 2023 17:06:45 +0800 Jiawen Wu wrote:
> The old way to do hardware reset is sending reset command to firmware.
> In order to adapt to the new firmware, driver directly write register
> of LAN reset instead of the old way.

Which versions of the FW use one method vs the other? Why is it okay 
to change the driver for new FW, are there no devices running old FW
in the wild? Or the new method is safe for both?

We need more information, we had a number of reports recently about
drivers breaking backwards compatibility and causing regressions for
users.

> And remove the redundant functions
> wx_reset_hostif() and wx_calculate_checksum().

You don't have to say that, it's a natural part of the change.
-- 
pw-bot: cr

Re: [PATCH net] net: txgbe: change hw reset mode

[mengyuanlou]

> 2023年6月23日 10:21，Jakub Kicinski <kuba@kernel.org> 写道：
> 
> On Wed, 21 Jun 2023 17:06:45 +0800 Jiawen Wu wrote:
>> The old way to do hardware reset is sending reset command to firmware.
>> In order to adapt to the new firmware, driver directly write register
>> of LAN reset instead of the old way.
> 
> Which versions of the FW use one method vs the other? Why is it okay 
> to change the driver for new FW, are there no devices running old FW
> in the wild? Or the new method is safe for both?

Lan reset contains of phy reset and dma reset.
New FW versions will support NCSI/LLDP which needs phy not to down.
When drivers do lan reset, fw can set a veto bit to block phy reset and
still to do dma reset.

> 
> We need more information, we had a number of reports recently about
> drivers breaking backwards compatibility and causing regressions for
> users.
> 
>> And remove the redundant functions
>> wx_reset_hostif() and wx_calculate_checksum().
> 
> You don't have to say that, it's a natural part of the change.
> -- 
> pw-bot: cr
> 
> 

Re: [PATCH net] net: txgbe: change hw reset mode

[Andrew Lunn]

On Sun, Jun 25, 2023 at 04:31:01PM +0800, mengyuanlou@net-swift.com wrote:
> 
> 
> > 2023年6月23日 10:21，Jakub Kicinski <kuba@kernel.org> 写道：
> > 
> > On Wed, 21 Jun 2023 17:06:45 +0800 Jiawen Wu wrote:
> >> The old way to do hardware reset is sending reset command to firmware.
> >> In order to adapt to the new firmware, driver directly write register
> >> of LAN reset instead of the old way.
> > 
> > Which versions of the FW use one method vs the other? Why is it okay 
> > to change the driver for new FW, are there no devices running old FW
> > in the wild? Or the new method is safe for both?
> 
> Lan reset contains of phy reset and dma reset.
> New FW versions will support NCSI/LLDP which needs phy not to down.
> When drivers do lan reset, fw can set a veto bit to block phy reset and
> still to do dma reset.

That does not answer the question. Is this backwards compatible with
old firmware?

	Andrew

Re: [PATCH net] net: txgbe: change hw reset mode

[mengyuanlou]

> 2023年6月25日 23:40，Andrew Lunn <andrew@lunn.ch> 写道：
> 
> On Sun, Jun 25, 2023 at 04:31:01PM +0800, mengyuanlou@net-swift.com wrote:
>> 
>> 
>>> 2023年6月23日 10:21，Jakub Kicinski <kuba@kernel.org> 写道：
>>> 
>>> On Wed, 21 Jun 2023 17:06:45 +0800 Jiawen Wu wrote:
>>>> The old way to do hardware reset is sending reset command to firmware.
>>>> In order to adapt to the new firmware, driver directly write register
>>>> of LAN reset instead of the old way.
>>> 
>>> Which versions of the FW use one method vs the other? Why is it okay 
>>> to change the driver for new FW, are there no devices running old FW
>>> in the wild? Or the new method is safe for both?
>> 
>> Lan reset contains of phy reset and dma reset.
>> New FW versions will support NCSI/LLDP which needs phy not to down.
>> When drivers do lan reset, fw can set a veto bit to block phy reset and
>> still to do dma reset.
> 
> That does not answer the question. Is this backwards compatible with
> old firmware?

Yeah，the veto bit is not set in old firmware, so they have the same effect.
> 
> Andrew

Re: [PATCH net] net: txgbe: change hw reset mode

[Jakub Kicinski]

On Mon, 26 Jun 2023 09:56:32 +0800 mengyuanlou@net-swift.com wrote:
> > That does not answer the question. Is this backwards compatible with
> > old firmware?  
> 
> Yeah，the veto bit is not set in old firmware, so they have the same effect.

Why were you using the more complex FW command then rather than just 
the register write, previously then?

Re: [PATCH net] net: txgbe: change hw reset mode

[mengyuanlou]

> 2023年6月27日 01:24，Jakub Kicinski <kuba@kernel.org> 写道：
> 
> On Mon, 26 Jun 2023 09:56:32 +0800 mengyuanlou@net-swift.com wrote:
>>> That does not answer the question. Is this backwards compatible with
>>> old firmware?  
>> 
>> Yeah，the veto bit is not set in old firmware, so they have the same effect.
> 
> Why were you using the more complex FW command then rather than just 
> the register write, previously then?
> 

Using FW command can notify fw that lan reset has happened, then FW
can configure something which should reconfigure.

Drivers write the register, the FW will not know lan reset has happened.

Later, we found the things which FW need in NCSI/LLDP/WOL... is only the phy. 
So just block phy reset, and use simple the register write.

Re: [PATCH net] net: txgbe: change hw reset mode

[Jakub Kicinski]

On Tue, 27 Jun 2023 10:18:46 +0800 mengyuanlou@net-swift.com wrote:
> > Why were you using the more complex FW command then rather than just 
> > the register write, previously then?
> 
> Using FW command can notify fw that lan reset has happened, then FW
> can configure something which should reconfigure.

This sentence is not entirely clear to me.

> Drivers write the register, the FW will not know lan reset has happened.
> 
> Later, we found the things which FW need in NCSI/LLDP/WOL... is only the phy. 
> So just block phy reset, and use simple the register write.

Fair enough, please include the explanation of backward compatibility
in the commit message and post a v2.