Re: [PATCH net] net: libwx: fix firmware mailbox abnormal return

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 2 Jan 2025 14:42:30 +0800 Jiawen Wu wrote:
> > >  	/* 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_dbg(wx, "check: %x %x\n", buffer[0] & 0xff, ~buf[0] >> 24);
> > > +		if ((buffer[0] & 0xff) != (~buf[0] >> 24))
> > > +			goto rel_out;  
> > 
> > Inverse question here, I guess. Why is it only an error for FW not
> > to be ready if cmd doesn't match?  
> 
> It is to check if the cmd has been processed by FW. If it matches, it means
> that FW has already processed the cmd, but FWRDY timeout for some
> unknown reason.

Feels a little risky. AFAICT the value we're comparing is just 
a checksum (potentially always set to 0xff?) It doesn't seem to
identify the command very well. If the command timed out let's
always return an error.

The other explanations make sense, please include them in the commit
msg.