diff mbox series

[net-next,v3] net: txgbe: change LAN reset mode

Message ID 20230711062623.3058-1-jiawenwu@trustnetic.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net-next,v3] net: txgbe: change LAN reset mode | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1341 this patch: 1341
netdev/cc_maintainers fail 1 blamed authors not CCed: davem@davemloft.net; 4 maintainers not CCed: kuba@kernel.org edumazet@google.com davem@davemloft.net pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 1364 this patch: 1364
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1364 this patch: 1364
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 98 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiawen Wu July 11, 2023, 6:26 a.m. UTC
The old way to do LAN reset is sending reset command to firmware. Once
firmware performs reset, it reconfigures what it needs.

In the new firmware versions, veto bit is introduced for NCSI/LLDP to
block PHY domain in LAN reset. At this point, writing register of LAN
reset directly makes the same effect as the old way. And it does not
reset MNG domain, so that veto bit does not change.

And this change is compatible with old firmware versions, since veto
bit was never used.

Fixes: b08012568ebb ("net: txgbe: Reset hardware")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
v2 -> v3:
- post to net-next
v1 -> v2:
- detail commit log
---
 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(-)

Comments

Simon Horman July 12, 2023, 1:28 p.m. UTC | #1
On Tue, Jul 11, 2023 at 02:26:23PM +0800, Jiawen Wu wrote:
> The old way to do LAN reset is sending reset command to firmware. Once
> firmware performs reset, it reconfigures what it needs.
> 
> In the new firmware versions, veto bit is introduced for NCSI/LLDP to
> block PHY domain in LAN reset. At this point, writing register of LAN
> reset directly makes the same effect as the old way. And it does not
> reset MNG domain, so that veto bit does not change.
> 
> And this change is compatible with old firmware versions, since veto
> bit was never used.

Can I confirm that:

1. Old firmware works both with and without this patch;
2. New firmware works with this patch but not without it?

> Fixes: b08012568ebb ("net: txgbe: Reset hardware")
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

This feels more like an enhancement, to support new firmware,
than a fix: old firmware works fine (wrt to this feature).

I suggest dropping drop the Fixes tag.

...
Paolo Abeni July 13, 2023, 12:49 p.m. UTC | #2
Hi,

On Tue, 2023-07-11 at 14:26 +0800, Jiawen Wu wrote:
> The old way to do LAN reset is sending reset command to firmware. Once
> firmware performs reset, it reconfigures what it needs.
> 
> In the new firmware versions, veto bit is introduced for NCSI/LLDP to
> block PHY domain in LAN reset. At this point, writing register of LAN
> reset directly makes the same effect as the old way. And it does not
> reset MNG domain, so that veto bit does not change.
> 
> And this change is compatible with old firmware versions, since veto
> bit was never used.

As the current commit message wording still raises questions, could you
please explicitly the level of compatibility of both the old and new
firmware pre/after this change?

And please drop the fixes tag, thanks!

Paolo
Jiawen Wu July 14, 2023, 2:06 a.m. UTC | #3
On Thursday, July 13, 2023 8:49 PM, Paolo Abeni wrote:
> Hi,
> 
> On Tue, 2023-07-11 at 14:26 +0800, Jiawen Wu wrote:
> > The old way to do LAN reset is sending reset command to firmware. Once
> > firmware performs reset, it reconfigures what it needs.
> >
> > In the new firmware versions, veto bit is introduced for NCSI/LLDP to
> > block PHY domain in LAN reset. At this point, writing register of LAN
> > reset directly makes the same effect as the old way. And it does not
> > reset MNG domain, so that veto bit does not change.
> >
> > And this change is compatible with old firmware versions, since veto
> > bit was never used.
> 
> As the current commit message wording still raises questions, could you
> please explicitly the level of compatibility of both the old and new
> firmware pre/after this change?

The old firmware is compatible with the driver before and after this
change. The new firmware needs to use with the driver after this change if
it wants to implement the new feature, otherwise it is the same as the old
firmware.

Does this explain make sense?

> 
> And please drop the fixes tag, thanks!
>
Simon Horman July 15, 2023, 10:57 a.m. UTC | #4
On Fri, Jul 14, 2023 at 10:06:59AM +0800, Jiawen Wu wrote:
> On Thursday, July 13, 2023 8:49 PM, Paolo Abeni wrote:
> > Hi,
> > 
> > On Tue, 2023-07-11 at 14:26 +0800, Jiawen Wu wrote:
> > > The old way to do LAN reset is sending reset command to firmware. Once
> > > firmware performs reset, it reconfigures what it needs.
> > >
> > > In the new firmware versions, veto bit is introduced for NCSI/LLDP to
> > > block PHY domain in LAN reset. At this point, writing register of LAN
> > > reset directly makes the same effect as the old way. And it does not
> > > reset MNG domain, so that veto bit does not change.
> > >
> > > And this change is compatible with old firmware versions, since veto
> > > bit was never used.
> > 
> > As the current commit message wording still raises questions, could you
> > please explicitly the level of compatibility of both the old and new
> > firmware pre/after this change?
> 
> The old firmware is compatible with the driver before and after this
> change. The new firmware needs to use with the driver after this change if
> it wants to implement the new feature, otherwise it is the same as the old
> firmware.
> 
> Does this explain make sense?

Yes, that makes sense to me.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
index 39a9aeee7aab..8f5bba0778c6 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 1f93ca32c921..0c4756e6ee06 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 0772eb14eabf..6e130d1f7a7b 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_hw.c
@@ -257,16 +257,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));