Message-Id: <7090F997-454D-4198-A076-72604C7AF0E1@net-swift.com>
Date: Fri, 20 Jun 2025 18:27:37 +0800
From: "mengyuanlou@...-swift.com" <mengyuanlou@...-swift.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: netdev@...r.kernel.org,
 kuba@...nel.org,
 pabeni@...hat.com,
 horms@...nel.org,
 andrew+netdev@...n.ch,
 duanqiangwen@...-swift.com,
 linglingzhang@...stnetic.com,
 jiawenwu@...-swift.com
Subject: Re: [PATCH net-next 02/12] net: libwx: add base vf api for vf drivers



> 2025年6月18日 19:28，Michal Swiatkowski <michal.swiatkowski@...ux.intel.com> 写道：
> 
> On Wed, Jun 11, 2025 at 04:35:49PM +0800, Mengyuan Lou wrote:
>> Implement mbox_write_and_read_ack functions which are
>> used to set basic functions like set_mac, get_link.etc
>> for vf.
>> 
>> Signed-off-by: Mengyuan Lou <mengyuanlou@...-swift.com>
>> ---
>> drivers/net/ethernet/wangxun/libwx/Makefile  |   1 +
>> drivers/net/ethernet/wangxun/libwx/wx_hw.c   |   2 +-
>> drivers/net/ethernet/wangxun/libwx/wx_hw.h   |   1 +
>> drivers/net/ethernet/wangxun/libwx/wx_type.h |   1 +
>> drivers/net/ethernet/wangxun/libwx/wx_vf.c   | 521 +++++++++++++++++++
>> drivers/net/ethernet/wangxun/libwx/wx_vf.h   |  61 +++
>> 6 files changed, 586 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/net/ethernet/wangxun/libwx/wx_vf.c
>> create mode 100644 drivers/net/ethernet/wangxun/libwx/wx_vf.h
>> 
>> diff --git a/drivers/net/ethernet/wangxun/libwx/Makefile b/drivers/net/ethernet/wangxun/libwx/Makefile
>> index 9b78b604a94e..ddf0bb921676 100644
>> --- a/drivers/net/ethernet/wangxun/libwx/Makefile
>> +++ b/drivers/net/ethernet/wangxun/libwx/Makefile
>> @@ -5,3 +5,4 @@
>> obj-$(CONFIG_LIBWX) += libwx.o
>> 
>> libwx-objs := wx_hw.o wx_lib.o wx_ethtool.o wx_ptp.o wx_mbx.o wx_sriov.o
>> +libwx-objs += wx_vf.o
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.c b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
>> index 0f4be72116b8..82dd76f0326e 100644
>> --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.c
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.c
>> @@ -1107,7 +1107,7 @@ static int wx_write_uc_addr_list(struct net_device *netdev, int pool)
>>  *  by the MO field of the MCSTCTRL. The MO field is set during initialization
>>  *  to mc_filter_type.
>>  **/
>> -static u32 wx_mta_vector(struct wx *wx, u8 *mc_addr)
>> +u32 wx_mta_vector(struct wx *wx, u8 *mc_addr)
>> {
>> u32 vector = 0;
>> 
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_hw.h b/drivers/net/ethernet/wangxun/libwx/wx_hw.h
>> index 26a56cba60b9..718015611da6 100644
>> --- a/drivers/net/ethernet/wangxun/libwx/wx_hw.h
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_hw.h
>> @@ -29,6 +29,7 @@ void wx_mac_set_default_filter(struct wx *wx, u8 *addr);
>> int wx_add_mac_filter(struct wx *wx, u8 *addr, u16 pool);
>> int wx_del_mac_filter(struct wx *wx, u8 *addr, u16 pool);
>> void wx_flush_sw_mac_table(struct wx *wx);
>> +u32 wx_mta_vector(struct wx *wx, u8 *mc_addr);
>> int wx_set_mac(struct net_device *netdev, void *p);
>> void wx_disable_rx(struct wx *wx);
>> int wx_set_vf_spoofchk(struct net_device *netdev, int vf, bool setting);
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
>> index f2061c893358..d2d0764792d4 100644
>> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
>> @@ -1213,6 +1213,7 @@ struct wx {
>> 
>> void *priv;
>> u8 __iomem *hw_addr;
>> + u8 __iomem *b4_addr; /* vf only */
>> struct pci_dev *pdev;
>> struct net_device *netdev;
>> struct wx_bus_info bus;
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf.c b/drivers/net/ethernet/wangxun/libwx/wx_vf.c
>> new file mode 100644
>> index 000000000000..a211329fd71a
>> --- /dev/null
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_vf.c
>> @@ -0,0 +1,521 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2015 - 2025 Beijing WangXun Technology Co., Ltd. */
>> +
>> +#include <linux/etherdevice.h>
>> +#include <linux/pci.h>
>> +
>> +#include "wx_type.h"
>> +#include "wx_hw.h"
>> +#include "wx_mbx.h"
>> +#include "wx_vf.h"
>> +
>> +static void wx_virt_clr_reg(struct wx *wx)
>> +{
>> + u32 vfsrrctl;
>> + int i;
>> +
>> + /* VRSRRCTL default values (BSIZEPACKET = 2048, BSIZEHEADER = 256) */
>> + vfsrrctl = WX_VXRXDCTL_HDRSZ(wx_hdr_sz(WX_RX_HDR_SIZE));
>> + vfsrrctl |= WX_VXRXDCTL_BUFSZ(wx_buf_sz(WX_RX_BUF_SIZE));
>> +
>> + /* clear all rxd ctl */
>> + for (i = 0; i < 7; i++) {
> 
> Define for 7 will be usefull.
> 
>> + wr32m(wx, WX_VXRXDCTL(i),
>> +      WX_VXRXDCTL_HDRSZ_MASK | WX_VXRXDCTL_BUFSZ_MASK,
>> +      vfsrrctl);
>> + }
> 
> Drop {, } as it is one line.
> 
>> +
>> + rd32(wx, WX_VXSTATUS);
>> +}
>> +
>> +/**
>> + *  wx_start_hw_vf - Prepare hardware for Tx/Rx
>> + *  @wx: pointer to hardware structure
>> + *
>> + *  Starts the hardware by filling the bus info structure and media type, clears
>> + *  all on chip counters, initializes receive address registers, multicast
>> + *  table, VLAN filter table, calls routine to set up link and flow control
>> + *  settings, and leaves transmit and receive units disabled and uninitialized
>> + **/
>> +void wx_start_hw_vf(struct wx *wx)
>> +{
>> + /* Clear wx stopped flag */
>> + wx->adapter_stopped = false;
> 
> From the description it should do a lot more. Maybe move the description
> to the patch where the function is filled.
> 
>> +}
>> +EXPORT_SYMBOL(wx_start_hw_vf);
>> +
>> +/**
>> + *  wx_init_hw_vf - virtual function hardware initialization
>> + *  @wx: pointer to hardware structure
>> + *
>> + *  Initialize the hardware by resetting the hardware and then starting
>> + *  the hardware
> 
> Now it is just setting the flag and reading MAC address.
> 
>> + **/
>> +void wx_init_hw_vf(struct wx *wx)
>> +{
>> + wx_start_hw_vf(wx);
>> + wx_get_mac_addr_vf(wx, wx->mac.addr);
>> +}
>> +EXPORT_SYMBOL(wx_init_hw_vf);
>> +
>> +static int wx_mbx_write_and_read_reply(struct wx *wx, u32 *req_buf,
>> +       u32 *resp_buf, u16 size)
>> +{
>> + int ret;
>> +
>> + ret = wx_write_posted_mbx(wx, req_buf, size);
>> + if (unlikely(ret))
> 
> Why unlikely? It isn't usuall to add in error handling outside hot path.
> 
>> + return ret;
>> +
>> + return wx_read_posted_mbx(wx, resp_buf, size);
>> +}
>> +
>> +/**
>> + *  wx_reset_hw_vf - Performs hardware reset
>> + *  @wx: pointer to hardware structure
>> + *
>> + *  Resets the hardware by resetting the transmit and receive units, masks and
>> + *  clears all interrupts.
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_reset_hw_vf(struct wx *wx)
>> +{
>> + struct wx_mbx_info *mbx = &wx->mbx;
>> + u32 msgbuf[4] = {WX_VF_RESET};
>> + u8 *addr = (u8 *)(&msgbuf[1]);
>> + u32 b4_buf[16] = {0};
>> + u32 timeout = 200;
>> + int ret;
>> + u32 i;
>> +
>> + /* Call wx stop to disable tx/rx and clear interrupts */
>> + wx_stop_adapter_vf(wx);
>> +
>> + /* reset the api version */
>> + wx->vfinfo->vf_api = wx_mbox_api_null;
>> +
>> + /* backup msix vectors */
>> + if (wx->b4_addr) {
> 
> Where this address is being set? I wonder if 0 is always invalid (as 0
> sounds like valid address) and also if it shouldn't return error in case
> wx->b4_addr is invalid.

It is inited in probe.
Because there are two different macs, one of them requires manual saving of the value of bar4.
Another one is unnecessary.
> 
>> + for (i = 0; i < 16; i++)
>> + b4_buf[i] = readl(wx->b4_addr + i * 4);
>> + }
>> +
>> + wr32m(wx, WX_VXCTRL, WX_VXCTRL_RST, WX_VXCTRL_RST);
>> + rd32(wx, WX_VXSTATUS);
>> +
>> + /* we cannot reset while the RSTI / RSTD bits are asserted */
>> + while (!wx_check_for_rst_vf(wx) && timeout) {
>> + timeout--;
>> + udelay(5);
>> + }
>> +
>> + /* restore msix vectors */
>> + if (wx->b4_addr) {
>> + for (i = 0; i < 16; i++)
>> + writel(b4_buf[i], wx->b4_addr + i * 4);
>> + }
>> +
>> + /* amlite: bme */
>> +#define WX_VX_PF_BME 0x4B8
> 
> In my opinion it should go to the header where there are other
> register definitions.
> 
>> + if (wx->mac.type == wx_mac_aml || wx->mac.type == wx_mac_aml40)
>> + wr32(wx, WX_VX_PF_BME, BIT(0));
> 
> BIT(0) can be also described there.
> 
>> +
>> + if (!timeout)
>> + return -EBUSY;
>> +
>> + /* Reset VF registers to initial values */
>> + wx_virt_clr_reg(wx);
>> +
>> + /* mailbox timeout can now become active */
>> + mbx->timeout = 2000;
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> +
>> + if (msgbuf[0] != (WX_VF_RESET | WX_VT_MSGTYPE_ACK) &&
>> +    msgbuf[0] != (WX_VF_RESET | WX_VT_MSGTYPE_NACK))
>> + return -EINVAL;
>> +
>> + if (msgbuf[0] == (WX_VF_RESET | WX_VT_MSGTYPE_ACK))
>> + ether_addr_copy(wx->mac.perm_addr, addr);
>> +
>> + wx->mac.mc_filter_type = msgbuf[3];
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(wx_reset_hw_vf);
>> +
>> +/**
>> + *  wx_stop_adapter_vf - Generic stop Tx/Rx units
>> + *  @wx: pointer to hardware structure
>> + *
>> + *  Sets the adapter_stopped flag within wx struct. Clears interrupts,
>> + *  disables transmit and receive units. The adapter_stopped flag is used by
>> + *  the shared code and drivers to determine if the wx is in a stopped
>> + *  state and should not touch the hardware.
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_stop_adapter_vf(struct wx *wx)
>> +{
>> + u32 reg_val;
>> + u16 i;
>> +
>> + wx->adapter_stopped = true;
>> +
>> + /* Clear interrupt mask to stop from interrupts being generated */
>> + wr32(wx, WX_VXIMS, WX_VF_IRQ_CLEAR_MASK);
>> +
>> + /* Clear any pending interrupts, flush previous writes */
>> + wr32(wx, WX_VXICR, U32_MAX);
>> +
>> + /* Disable the transmit unit.  Each queue must be disabled. */
>> + for (i = 0; i < wx->mac.max_tx_queues; i++)
>> + wr32(wx, WX_VXTXDCTL(i), WX_VXTXDCTL_FLUSH);
>> +
>> + /* Disable the receive unit by stopping each queue */
>> + for (i = 0; i < wx->mac.max_rx_queues; i++) {
>> + reg_val = rd32(wx, WX_VXRXDCTL(i));
>> + reg_val &= ~WX_VXRXDCTL_ENABLE;
>> + wr32(wx, WX_VXRXDCTL(i), reg_val);
>> + }
>> + /* Clear packet split and pool config */
>> + wr32(wx, WX_VXMRQC, 0);
>> +
>> + /* flush all queues disables */
>> + rd32(wx, WX_VXSTATUS);
>> +
>> + return 0;
> 
> Always returns 0, should be void.

> 
>> +}
>> +EXPORT_SYMBOL(wx_stop_adapter_vf);
>> +
>> +/**
>> + *  wx_set_rar_vf - set device MAC address
>> + *  @wx: pointer to hardware structure
>> + *  @index: Receive address register to write
>> + *  @addr: Address to put into receive address register
>> + *  @enable_addr: set flag that address is active
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_set_rar_vf(struct wx *wx, u32 index, u8 *addr, u32 enable_addr)
>> +{
>> + u32 msgbuf[3] = {WX_VF_SET_MAC_ADDR};
>> + u8 *msg_addr = (u8 *)(&msgbuf[1]);
>> + int ret;
>> +
>> + memcpy(msg_addr, addr, ETH_ALEN);
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> + msgbuf[0] &= ~WX_VT_MSGTYPE_CTS;
>> +
>> + /* if nacked the address was rejected, use "perm_addr" */
>> + if (msgbuf[0] == (WX_VF_SET_MAC_ADDR | WX_VT_MSGTYPE_NACK)) {
>> + wx_get_mac_addr_vf(wx, wx->mac.addr);
>> + return -EINVAL;
>> + }
>> +
>> + return ret;
> 
> nit, ret is always 0 here, so return 0; should be more readable.
> 
>> +}
>> +EXPORT_SYMBOL(wx_set_rar_vf);
>> +
>> +/**
>> + *  wx_update_mc_addr_list_vf - Update Multicast addresses
>> + *  @wx: pointer to the HW structure
>> + *  @netdev: pointer to the net device structure
>> + *
>> + *  Updates the Multicast Table Array.
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_update_mc_addr_list_vf(struct wx *wx, struct net_device *netdev)
>> +{
>> + u32 msgbuf[WX_VXMAILBOX_SIZE] = {0};
> 
> WX_VXMAILBOX_SIZE is 15, but 4B is opcode. There is 14 * 4B for payload
> = 56B, so it is 28 16 bit words, not 30, am I correct? Please add it as
> define instead of just the number.

I check the spec, find the hw size is 16. And last 4B is reserve.

> 
>> + u16 *vector_l = (u16 *)&msgbuf[1];
>> + struct netdev_hw_addr *ha;
>> + u32 cnt, i;
>> +
>> + /* Each entry in the list uses 1 16 bit word.  We have 30
>> + * 16 bit words available in our HW msgbuf buffer (minus 1 for the
>> + * msgbuf type).  That's 30 hash values if we pack 'em right. If
>> + * there are more than 30 MC addresses to add then punt the
>> + * extras for now and then add code to handle more than 30 later.
>> + * It would be unusual for a server to request that many multi-cast
>> + * addresses except for in large enterprise network environments.
>> + */
> 
> I think you should handle more than 30 here, instead of writting a
> comment about adding it later. It is just about sending anoither
> command, yes?
> }

This comment is confused.
This mailbox cmd is tell pf should open multicast when netdev_mc_count > 30.

Set MAC address is another cmd.


>> +
>> + cnt = netdev_mc_count(netdev);
>> + if (cnt > 30)
>> + cnt = 30;
>> + msgbuf[0] = WX_VF_SET_MULTICAST;
>> + msgbuf[0] |= cnt << WX_VT_MSGINFO_SHIFT;
>> +
>> + i = 0;
>> + netdev_for_each_mc_addr(ha, netdev) {
>> + if (i == cnt)
>> + break;
>> + if (is_link_local_ether_addr(ha->addr))
>> + continue;
>> +
>> + vector_l[i++] = wx_mta_vector(wx, ha->addr);
> 
> What about endianess, don't you need a __cpu_to_le16/be16 conversion? 
> 
>> + }
>> +
>> + return wx_write_posted_mbx(wx, msgbuf, WX_VXMAILBOX_SIZE);
> 
> Shouldn't the size be cut to the real number of address?

Your are right.
> 
>> +}
>> +EXPORT_SYMBOL(wx_update_mc_addr_list_vf);
>> +
>> +/**
>> + *  wx_update_xcast_mode_vf - Update Multicast mode
>> + *  @wx: pointer to the HW structure
>> + *  @xcast_mode: new multicast mode
>> + *
>> + *  Updates the Multicast Mode of VF.
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_update_xcast_mode_vf(struct wx *wx, int xcast_mode)
>> +{
>> + u32 msgbuf[2] = {WX_VF_UPDATE_XCAST_MODE, xcast_mode};
>> + int ret = 0;
>> +
>> + switch (wx->vfinfo->vf_api) {
>> + case wx_mbox_api_13:
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
> 
> if (api != vx_mbox_api_13)
> return -EINVAL;
> 
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> +
>> + msgbuf[0] &= ~WX_VT_MSGTYPE_CTS;
>> + if (msgbuf[0] == (WX_VF_UPDATE_XCAST_MODE | WX_VT_MSGTYPE_NACK))
>> + return -EINVAL;
>> +
>> + return ret;
> 
> return 0;
> 
>> +}
>> +EXPORT_SYMBOL(wx_update_xcast_mode_vf);
>> +
>> +/**
>> + * wx_get_link_state_vf - Get VF link state from PF
>> + * @wx: pointer to the HW structure
>> + * @link_state: link state storage
>> + *
>> + * Return: return state of the operation error or success.
>> + **/
>> +int wx_get_link_state_vf(struct wx *wx, u16 *link_state)
>> +{
>> + u32 msgbuf[2] = {WX_VF_GET_LINK_STATE};
>> + int ret = 0;
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> +
>> + if (msgbuf[0] & WX_VT_MSGTYPE_NACK)
>> + return -EINVAL;
>> +
>> + *link_state = msgbuf[1];
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(wx_get_link_state_vf);
>> +
>> +/**
>> + *  wx_set_vfta_vf - Set/Unset vlan filter table address
>> + *  @wx: pointer to the HW structure
>> + *  @vlan: 12 bit VLAN ID
>> + *  @vind: unused by VF drivers
>> + *  @vlan_on: if true then set bit, else clear bit
>> + *  @vlvf_bypass: boolean flag indicating updating default pool is okay
>> + *
>> + *  Turn on/off specified VLAN in the VLAN filter table.
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_set_vfta_vf(struct wx *wx, u32 vlan, u32 vind, bool vlan_on,
>> +   bool vlvf_bypass)
>> +{
>> + u32 msgbuf[2] = {WX_VF_SET_VLAN, vlan};
>> + bool vlan_offload = false;
>> + int ret = 0;
>> +
>> + /* Setting the 8 bit field MSG INFO to TRUE indicates "add" */
>> + msgbuf[0] |= vlan_on << WX_VT_MSGINFO_SHIFT;
>> + /* if vf vlan offload is disabled, allow to create vlan under pf port vlan */
>> + msgbuf[0] |= BIT(vlan_offload);
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> +
>> + if (msgbuf[0] & WX_VT_MSGTYPE_ACK)
>> + return 0;
>> +
>> + return msgbuf[0] & WX_VT_MSGTYPE_NACK;
> 
> This will be positive on fail. Reword the kdoc.
> Is it even possible to not have ACK and not have NACK?
> Maybe return -EIO; here is enough? Or even better return error if no ACK
> and return 0; at the end.

If wx_mbx_write_and_read_reply is passed, the msgbuf definitely received successfully.

So I think if it not have ACK and not have NACK, it will return before.


> 
>> +}
>> +EXPORT_SYMBOL(wx_set_vfta_vf);
>> +
>> +void wx_get_mac_addr_vf(struct wx *wx, u8 *mac_addr)
>> +{
>> + ether_addr_copy(mac_addr, wx->mac.perm_addr);
>> +}
>> +EXPORT_SYMBOL(wx_get_mac_addr_vf);
>> +
>> +int wx_get_fw_version_vf(struct wx *wx)
>> +{
>> + u32 msgbuf[2] = {WX_VF_GET_FW_VERSION};
>> + int ret;
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> +
>> + if (msgbuf[0] & WX_VT_MSGTYPE_NACK)
>> + return -EINVAL;
>> + snprintf(wx->eeprom_id, 32, "0x%08x", msgbuf[1]);
>> +
>> + return ret;
> 
> It will be great to stick to one style of checking if it is possible.
> Here is the best, if nack returns error, just return 0 at the end, as
> ret is always 0 here.
> 
>> +}
>> +EXPORT_SYMBOL(wx_get_fw_version_vf);
>> +
>> +int wx_set_uc_addr_vf(struct wx *wx, u32 index, u8 *addr)
>> +{
>> + u32 msgbuf[3] = {WX_VF_SET_MACVLAN};
>> + u8 *msg_addr = (u8 *)(&msgbuf[1]);
>> + int ret;
>> +
>> + /* If index is one then this is the start of a new list and needs
>> + * indication to the PF so it can do it's own list management.
>> + * If it is zero then that tells the PF to just clear all of
>> + * this VF's macvlans and there is no new list.
>> + */
>> + msgbuf[0] |= index << WX_VT_MSGINFO_SHIFT;
>> + if (addr)
>> + memcpy(msg_addr, addr, 6);
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> +
>> + msgbuf[0] &= ~WX_VT_MSGTYPE_CTS;
>> +
>> + if (msgbuf[0] == (WX_VF_SET_MACVLAN | WX_VT_MSGTYPE_NACK))
>> + return -EINVAL;
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(wx_set_uc_addr_vf);
>> +
>> +/**
>> + *  wx_rlpml_set_vf - Set the maximum receive packet length
>> + *  @wx: pointer to the HW structure
>> + *  @max_size: value to assign to max frame size
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_rlpml_set_vf(struct wx *wx, u16 max_size)
>> +{
>> + u32 msgbuf[2] = {WX_VF_SET_LPE, max_size};
>> + int ret;
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> + if ((msgbuf[0] & WX_VF_SET_LPE) &&
>> +    (msgbuf[0] & WX_VT_MSGTYPE_NACK))
>> + return -EINVAL;
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(wx_rlpml_set_vf);
>> +
>> +/**
>> + *  wx_negotiate_api_version - Negotiate supported API version
>> + *  @wx: pointer to the HW structure
>> + *  @api: integer containing requested API version
>> + *
>> + *  Return: returns 0 on success, negative error code on failure
>> + **/
>> +int wx_negotiate_api_version(struct wx *wx, int api)
>> +{
>> + u32 msgbuf[2] = {WX_VF_API_NEGOTIATE, api};
>> + int ret;
>> +
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> +
>> + msgbuf[0] &= ~WX_VT_MSGTYPE_CTS;
>> +
>> + /* Store value and return 0 on success */
>> + if (msgbuf[0] == (WX_VF_API_NEGOTIATE | WX_VT_MSGTYPE_NACK))
>> + return -EINVAL;
>> + wx->vfinfo->vf_api = api;
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(wx_negotiate_api_version);
>> +
>> +int wx_get_queues_vf(struct wx *wx, u32 *num_tcs, u32 *default_tc)
>> +{
>> + u32 msgbuf[5] = {WX_VF_GET_QUEUES};
>> + int ret = 0;
> 
> int ret; is enough.
> 
>> +
>> + /* do nothing if API doesn't support wx_get_queues */
>> + switch (wx->vfinfo->vf_api) {
>> + case wx_mbox_api_13:
>> + break;
>> + default:
>> + return 0;
> 
> if() instead
> 
>> + }
>> +
>> + /* Fetch queue configuration from the PF */
>> + ret = wx_mbx_write_and_read_reply(wx, msgbuf, msgbuf,
>> +  ARRAY_SIZE(msgbuf));
>> + if (ret)
>> + return ret;
>> + msgbuf[0] &= ~WX_VT_MSGTYPE_CTS;
>> +
>> + /* if we didn't get an ACK there must have been
>> + * some sort of mailbox error so we should treat it
>> + * as such
>> + */
>> + if (msgbuf[0] != (WX_VF_GET_QUEUES | WX_VT_MSGTYPE_ACK))
>> + return -EINVAL;
>> + /* record and validate values from message */
>> + wx->mac.max_tx_queues = msgbuf[WX_VF_TX_QUEUES];
>> + if (wx->mac.max_tx_queues == 0 ||
>> +    wx->mac.max_tx_queues > WX_VF_MAX_TX_QUEUES)
>> + wx->mac.max_tx_queues = WX_VF_MAX_TX_QUEUES;
>> +
>> + wx->mac.max_rx_queues = msgbuf[WX_VF_RX_QUEUES];
>> + if (wx->mac.max_rx_queues == 0 ||
>> +    wx->mac.max_rx_queues > WX_VF_MAX_RX_QUEUES)
>> + wx->mac.max_rx_queues = WX_VF_MAX_RX_QUEUES;
>> +
>> + *num_tcs = msgbuf[WX_VF_TRANS_VLAN];
>> + /* in case of unknown state assume we cannot tag frames */
>> + if (*num_tcs > wx->mac.max_rx_queues)
>> + *num_tcs = 1;
>> + *default_tc = msgbuf[WX_VF_DEF_QUEUE];
>> + /* default to queue 0 on out-of-bounds queue number */
>> + if (*default_tc >= wx->mac.max_tx_queues)
>> + *default_tc = 0;
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL(wx_get_queues_vf);
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf.h b/drivers/net/ethernet/wangxun/libwx/wx_vf.h
>> new file mode 100644
>> index 000000000000..eb40048f46eb
>> --- /dev/null
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_vf.h
>> @@ -0,0 +1,61 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright (c) 2015 - 2025 Beijing WangXun Technology Co., Ltd. */
>> +
>> +#ifndef _WX_VF_H_
>> +#define _WX_VF_H_
>> +
>> +#define WX_VXSTATUS              0x4
>> +#define WX_VXCTRL                0x8
>> +#define WX_VXCTRL_RST            BIT(0)
>> +
>> +#define WX_VXMRQC                0x78
>> +#define WX_VXICR                 0x100
>> +#define WX_VXIMS                 0x108
>> +#define WX_VF_IRQ_CLEAR_MASK     7
>> +#define WX_VF_MAX_TX_QUEUES      4
>> +#define WX_VF_MAX_RX_QUEUES      4
>> +#define WX_VXTXDCTL(r)           (0x3010 + (0x40 * (r)))
>> +#define WX_VXRXDCTL(r)           (0x1010 + (0x40 * (r)))
>> +#define WX_VXRXDCTL_ENABLE       BIT(0)
>> +#define WX_VXTXDCTL_FLUSH        BIT(26)
>> +
>> +#define WX_VXRXDCTL_RSCMAX(f)    FIELD_PREP(GENMASK(24, 23), f)
>> +#define WX_VXRXDCTL_BUFLEN(f)    FIELD_PREP(GENMASK(6, 1), f)
>> +#define WX_VXRXDCTL_BUFSZ(f)     FIELD_PREP(GENMASK(11, 8), f)
>> +#define WX_VXRXDCTL_HDRSZ(f)     FIELD_PREP(GENMASK(15, 12), f)
>> +
>> +#define WX_VXRXDCTL_RSCMAX_MASK  GENMASK(24, 23)
>> +#define WX_VXRXDCTL_BUFLEN_MASK  GENMASK(6, 1)
>> +#define WX_VXRXDCTL_BUFSZ_MASK   GENMASK(11, 8)
>> +#define WX_VXRXDCTL_HDRSZ_MASK   GENMASK(15, 12)
>> +
>> +#define wx_conf_size(v, mwidth, uwidth) ({ \
>> + typeof(v) _v = (v); \
>> + (_v == 2 << (mwidth) ? 0 : _v >> (uwidth)); \
>> +})
>> +#define wx_buf_len(v)            wx_conf_size(v, 13, 7)
>> +#define wx_hdr_sz(v)             wx_conf_size(v, 10, 6)
>> +#define wx_buf_sz(v)             wx_conf_size(v, 14, 10)
>> +#define wx_pkt_thresh(v)         wx_conf_size(v, 4, 0)
>> +
>> +#define WX_RX_HDR_SIZE           256
>> +#define WX_RX_BUF_SIZE           2048
>> +
>> +void wx_start_hw_vf(struct wx *wx);
>> +void wx_init_hw_vf(struct wx *wx);
>> +int wx_reset_hw_vf(struct wx *wx);
>> +void wx_get_mac_addr_vf(struct wx *wx, u8 *mac_addr);
>> +int wx_stop_adapter_vf(struct wx *wx);
>> +int wx_get_fw_version_vf(struct wx *wx);
>> +int wx_set_rar_vf(struct wx *wx, u32 index, u8 *addr, u32 enable_addr);
>> +int wx_update_mc_addr_list_vf(struct wx *wx, struct net_device *netdev);
>> +int wx_set_uc_addr_vf(struct wx *wx, u32 index, u8 *addr);
>> +int wx_rlpml_set_vf(struct wx *wx, u16 max_size);
>> +int wx_negotiate_api_version(struct wx *wx, int api);
>> +int wx_get_queues_vf(struct wx *wx, u32 *num_tcs, u32 *default_tc);
>> +int wx_update_xcast_mode_vf(struct wx *wx, int xcast_mode);
>> +int wx_get_link_state_vf(struct wx *wx, u16 *link_state);
>> +int wx_set_vfta_vf(struct wx *wx, u32 vlan, u32 vind, bool vlan_on,
>> +   bool vlvf_bypass);
>> +
>> +#endif /* _WX_VF_H_ */
> 
> In general it will be great to define structures for each command that
> is used in VF communication. Filling it should be also easier.
> 
> Thanks
> 
>> -- 
>> 2.30.1
> 

Powered by blists - more mailing lists