| Message ID | 20250103081013.1995939-1-jiawenwu@trustnetic.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8ce4f287524c74a118b0af1eebd4b24a8efca57a |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: libwx: fix firmware mailbox abnormal return | expand |
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Fri, 3 Jan 2025 16:10:13 +0800 you wrote: > The existing SW-FW interaction flow on the driver is wrong. Follow this > wrong flow, driver would never return error if there is a unknown command. > Since firmware writes back 'firmware ready' and 'unknown command' in the > mailbox message if there is an unknown command sent by driver. So reading > 'firmware ready' does not timeout. Then driver would mistakenly believe > that the interaction has completed successfully. > > [...] Here is the summary with links: - [net,v2] net: libwx: fix firmware mailbox abnormal return https://git.kernel.org/netdev/net/c/8ce4f287524c You are awesome, thank you!
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c index 1bf9c38e4125..deaf670c160e 100644 --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c @@ -334,27 +334,25 @@ int wx_host_interface_command(struct wx *wx, u32 *buffer, status = read_poll_timeout(rd32, hicr, hicr & WX_MNG_MBOX_CTL_FWRDY, 1000, timeout * 1000, false, wx, WX_MNG_MBOX_CTL); + buf[0] = rd32(wx, WX_MNG_MBOX); + if ((buf[0] & 0xff0000) >> 16 == 0x80) { + wx_err(wx, "Unknown FW command: 0x%x\n", buffer[0] & 0xff); + status = -EINVAL; + goto rel_out; + } + /* Check command completion */ if (status) { - wx_dbg(wx, "Command has failed with no status valid.\n"); - - buf[0] = rd32(wx, WX_MNG_MBOX); - if ((buffer[0] & 0xff) != (~buf[0] >> 24)) { - status = -EINVAL; - goto rel_out; - } - if ((buf[0] & 0xff0000) >> 16 == 0x80) { - wx_dbg(wx, "It's unknown cmd.\n"); - status = -EINVAL; - goto rel_out; - } - + wx_err(wx, "Command has failed with no status valid.\n"); wx_dbg(wx, "write value:\n"); for (i = 0; i < dword_len; i++) wx_dbg(wx, "%x ", buffer[i]); wx_dbg(wx, "read value:\n"); for (i = 0; i < dword_len; i++) wx_dbg(wx, "%x ", buf[i]); + wx_dbg(wx, "\ncheck: %x %x\n", buffer[0] & 0xff, ~buf[0] >> 24); + + goto rel_out; } if (!return_data)
The existing SW-FW interaction flow on the driver is wrong. Follow this wrong flow, driver would never return error if there is a unknown command. Since firmware writes back 'firmware ready' and 'unknown command' in the mailbox message if there is an unknown command sent by driver. So reading 'firmware ready' does not timeout. Then driver would mistakenly believe that the interaction has completed successfully. It tends to happen with the use of custom firmware. Move the check for 'unknown command' out of the poll timeout for 'firmware ready'. And adjust the debug log so that mailbox messages are always printed when commands timeout. Fixes: 1efa9bfe58c5 ("net: libwx: Implement interaction with firmware") Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> --- Changes in v2: - Detail the issue in the commit log. - Change log level from debug to error to print unknown cmd. - Always return an error when cmd timed out. --- drivers/net/ethernet/wangxun/libwx/wx_hw.c | 24 ++++++++++------------ 1 file changed, 11 insertions(+), 13 deletions(-)