Re: [PATCH] net/txgbe: fix the missing old mailbox interface calls

[Thomas Monjalon <thomas@monjalon.net>]

26/11/2025 07:27, Jiawen Wu:
> Hi, this fix patch is crucial for the new merged feature in this release.
> 
> > -----Original Message-----
> > From: Jiawen Wu <jiawenwu@trustnetic.com>
> > Sent: Monday, November 24, 2025 10:03 AM
> > To: 'Stephen Hemminger' <stephen@networkplumber.org>
> > Cc: dev@dpdk.org; zaiyuwang@trustnetic.com
> > Subject: RE: [PATCH] net/txgbe: fix the missing old mailbox interface calls
> > 
> > On Thu, Nov 20, 2025 2:25 PM, Jiawen Wu wrote:
> > > On Thu, Nov 20, 2025 1:48 PM, Stephen Hemminger wrote:
> > > > On Wed, 19 Nov 2025 16:56:24 +0800
> > > > Jiawen Wu <jiawenwu@trustnetic.com> wrote:
> > > >
> > > > > @@ -28,6 +29,9 @@ void txgbe_init_ops_aml40(struct txgbe_hw *hw)
> > > > >  	mac->init_mac_link_ops = txgbe_init_mac_link_ops_aml40;
> > > > >  	mac->get_link_capabilities = txgbe_get_link_capabilities_aml40;
> > > > >  	mac->check_link = txgbe_check_mac_link_aml40;
> > > > > +
> > > > > +	/* MAILBOX */
> > > > > +	mbx->host_interface_command = txgbe_host_interface_command_aml;
> > > > >  }
> > > >
> > > > Maybe that comment means something to you, but it seems like something
> > > > the next maintainer would not know what it means. Could you explain it more?
> > >
> > > For different devices, the mailbox flow between software and firmware is
> > > different. There are several mailbox command in the txgbe driver, but only
> > > txgbe_hic_sr_read() was changed to use the new flow in commit
> > > 6a139ade82e7 ("net/txgbe: add new SW-FW mailbox interface"). Because
> > > this function invoke the txgbe_hic_unlocked() directly without SW-FW
> > > semaphore, I guess.
> > >
> > > It lead to other mailbox commands timeout for Amber-Lite devices, which
> > > is required to use the new flow. So this patch fills in the missing part.
> > >
> > > For the sake of code tidy, txgbe_hic_sr_read() should change to use the
> > > locked function txgbe_host_interface_command(), and this function could
> > > be change to pointer in struct txgbe_mbx_info for different devices.
> > 
> > Hi Stephen Hemminger,
> > 
> > Does this explanation is sufficient? Should I send v2 patch with these logs?

Yes you should send a v2 with a better comment in the code,
and maybe a longer explanation in the commit log.