[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <74b2aab0-25a5-4bb0-ab98-a70fb25f7c54@intel.com>
Date: Wed, 5 Jun 2024 07:03:11 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Mengyuan Lou <mengyuanlou@...-swift.com>, <netdev@...r.kernel.org>
CC: <jiawenwu@...stnetic.com>, <duanqiangwen@...-swift.com>
Subject: Re: [PATCH net-next v4 1/6] net: libwx: Add malibox api for wangxun
pf drivers
On 6/4/24 17:57, Mengyuan Lou wrote:
> Implements the mailbox interfaces for wangxun pf drivers
> ngbe and txgbe.
>
> Signed-off-by: Mengyuan Lou <mengyuanlou@...-swift.com>
> ---
> drivers/net/ethernet/wangxun/libwx/Makefile | 2 +-
> drivers/net/ethernet/wangxun/libwx/wx_mbx.c | 189 +++++++++++++++++++
> drivers/net/ethernet/wangxun/libwx/wx_mbx.h | 32 ++++
> drivers/net/ethernet/wangxun/libwx/wx_type.h | 5 +
> 4 files changed, 227 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/wangxun/libwx/wx_mbx.c
> create mode 100644 drivers/net/ethernet/wangxun/libwx/wx_mbx.h
>
Please change your future submissions to have cover letter message-id
linked with actual patches, instead of two separate threads. Please also
post URLs to previous versions.
> diff --git a/drivers/net/ethernet/wangxun/libwx/Makefile b/drivers/net/ethernet/wangxun/libwx/Makefile
> index 42ccd6e4052e..913a978c9032 100644
> --- a/drivers/net/ethernet/wangxun/libwx/Makefile
> +++ b/drivers/net/ethernet/wangxun/libwx/Makefile
> @@ -4,4 +4,4 @@
>
> obj-$(CONFIG_LIBWX) += libwx.o
>
> -libwx-objs := wx_hw.o wx_lib.o wx_ethtool.o
> +libwx-objs := wx_hw.o wx_lib.o wx_ethtool.o wx_mbx.o
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_mbx.c b/drivers/net/ethernet/wangxun/libwx/wx_mbx.c
> new file mode 100644
> index 000000000000..e7d7178a1f13
> --- /dev/null
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_mbx.c
> @@ -0,0 +1,189 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2015 - 2024 Beijing WangXun Technology Co., Ltd. */
> +#include <linux/pci.h>
> +#include "wx_type.h"
> +#include "wx_mbx.h"
> +
> +/**
> + * wx_obtain_mbx_lock_pf - obtain mailbox lock
> + * @wx: pointer to the HW structure
> + * @vf: the VF index
> + *
> + * return: return SUCCESS if we obtained the mailbox lock
> + **/
> +static int wx_obtain_mbx_lock_pf(struct wx *wx, u16 vf)
> +{
> + int ret = -EBUSY;
> + int count = 5;
> + u32 mailbox;
> +
> + while (count--) {
> + /* Take ownership of the buffer */
> + wr32(wx, WX_PXMAILBOX(vf), WX_PXMAILBOX_PFU);
> +
> + /* reserve mailbox for vf use */
> + mailbox = rd32(wx, WX_PXMAILBOX(vf));
> + if (mailbox & WX_PXMAILBOX_PFU) {
> + ret = 0;
> + break;
> + }
> + udelay(10);
not needed delay on the last loop step
> + }
> +
> + if (ret)
> + wx_err(wx, "Failed to obtain mailbox lock for PF%d", vf);
> +
> + return ret;
> +}
> +
> +static int wx_check_for_bit_pf(struct wx *wx, u32 mask, int index)
> +{
> + u32 mbvficr = rd32(wx, WX_MBVFICR(index));
> + int ret = -EBUSY;
please invert the flow (and use this as general rule), so you will have:
if (err) {
error_handling();
return -ESTH;
}
normal_code();
return 0;
> +
> + if (mbvficr & mask) {
> + ret = 0;
> + wr32(wx, WX_MBVFICR(index), mask);
you are checking if any bit of mask is set, then writing
this value into the register; not an error per-se, but also not an
obvious thing
> + }
> +
> + return ret;
> +}
> +
> +/**
> + * wx_check_for_ack_pf - checks to see if the VF has ACKed
> + * @wx: pointer to the HW structure
> + * @vf: the VF index
> + *
> + * return: return SUCCESS if the VF has set the Status bit or else -EBUSY
> + **/
> +int wx_check_for_ack_pf(struct wx *wx, u16 vf)
> +{
> + u32 index = vf / 16, vf_bit = vf % 16;
> +
> + return wx_check_for_bit_pf(wx,
> + FIELD_PREP(WX_MBVFICR_VFACK_MASK, BIT(vf_bit)),
> + index);
> +}
> +EXPORT_SYMBOL(wx_check_for_ack_pf);
> +
> +/**
> + * wx_check_for_msg_pf - checks to see if the VF has sent mail
> + * @wx: pointer to the HW structure
> + * @vf: the VF index
> + *
> + * return: return SUCCESS if the VF has set the Status bit or else -EBUSY
> + **/
> +int wx_check_for_msg_pf(struct wx *wx, u16 vf)
> +{
> + u32 index = vf / 16, vf_bit = vf % 16;
> +
> + return wx_check_for_bit_pf(wx,
> + FIELD_PREP(WX_MBVFICR_VFREQ_MASK, BIT(vf_bit)),
> + index);
> +}
> +EXPORT_SYMBOL(wx_check_for_msg_pf);
> +
> +/**
> + * wx_write_mbx_pf - Places a message in the mailbox
> + * @wx: pointer to the HW structure
> + * @msg: The message buffer
> + * @size: Length of buffer
> + * @vf: the VF index
> + *
> + * return: return SUCCESS if it successfully copied message into the buffer
s/return:/Return:/
s/SUCCESS/0/ ;)
> + **/
> +int wx_write_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf)
> +{
> + struct wx_mbx_info *mbx = &wx->mbx;
> + int ret, i;
> +
> + if (size > mbx->size) {
> + wx_err(wx, "Invalid mailbox message size %d", size);
> + ret = -EINVAL;
> + goto out_no_write;
> + }
> +
> + /* lock the mailbox to prevent pf/vf race condition */
> + ret = wx_obtain_mbx_lock_pf(wx, vf);
> + if (ret)
> + goto out_no_write;
> +
> + /* flush msg and acks as we are overwriting the message buffer */
how many messages does fit into mbox?
> + wx_check_for_msg_pf(wx, vf);
> + wx_check_for_ack_pf(wx, vf);
> +
> + /* copy the caller specified message to the mailbox memory buffer */
> + for (i = 0; i < size; i++)
> + wr32a(wx, WX_PXMBMEM(vf), i, msg[i]);
> +
> + /* Interrupt VF to tell it a message has been sent and release buffer*/
> + /* set mirrored mailbox flags */
> + wr32a(wx, WX_PXMBMEM(vf), WX_VXMAILBOX_SIZE, WX_PXMAILBOX_STS);
> + wr32(wx, WX_PXMAILBOX(vf), WX_PXMAILBOX_STS);
> +
> +out_no_write:
> + return ret;
> +}
> +EXPORT_SYMBOL(wx_write_mbx_pf);
> +
> +/**
> + * wx_read_mbx_pf - Read a message from the mailbox
> + * @wx: pointer to the HW structure
> + * @msg: The message buffer
> + * @size: Length of buffer
> + * @vf: the VF index
> + *
> + * return: return SUCCESS if VF copy a message from the mailbox buffer.
> + **/
> +int wx_read_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf)
> +{
> + struct wx_mbx_info *mbx = &wx->mbx;
> + int ret;
> + u16 i;
> +
> + /* limit read to size of mailbox */
> + if (size > mbx->size)
> + size = mbx->size;
> +
> + /* lock the mailbox to prevent pf/vf race condition */
> + ret = wx_obtain_mbx_lock_pf(wx, vf);
> + if (ret)
> + goto out_no_read;
> +
> + /* copy the message to the mailbox memory buffer */
> + for (i = 0; i < size; i++)
> + msg[i] = rd32a(wx, WX_PXMBMEM(vf), i);
> +
> + /* Acknowledge the message and release buffer */
> + /* set mirrored mailbox flags */
> + wr32a(wx, WX_PXMBMEM(vf), WX_VXMAILBOX_SIZE, WX_PXMAILBOX_ACK);
> + wr32(wx, WX_PXMAILBOX(vf), WX_PXMAILBOX_ACK);
> +out_no_read:
> + return ret;
> +}
> +EXPORT_SYMBOL(wx_read_mbx_pf);
> +
> +/**
> + * wx_check_for_rst_pf - checks to see if the VF has reset
> + * @wx: pointer to the HW structure
> + * @vf: the VF index
> + *
> + * return: return SUCCESS if the VF has set the Status bit or else -EBUSY
> + **/
> +int wx_check_for_rst_pf(struct wx *wx, u16 vf)
> +{
> + u32 reg_offset = vf / 32;
> + u32 vf_shift = vf % 32;
> + int ret = -EBUSY;
> + u32 vflre = 0;
> +
> + vflre = rd32(wx, WX_VFLRE(reg_offset));
> +
> + if (vflre & BIT(vf_shift)) {
ditto error vs normal flow, please apply to whole series
> + ret = 0;
> + wr32(wx, WX_VFLREC(reg_offset), BIT(vf_shift));
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(wx_check_for_rst_pf);
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_mbx.h b/drivers/net/ethernet/wangxun/libwx/wx_mbx.h
> new file mode 100644
> index 000000000000..1579096fb6ad
> --- /dev/null
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_mbx.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2015 - 2024 Beijing WangXun Technology Co., Ltd. */
> +#ifndef _WX_MBX_H_
> +#define _WX_MBX_H_
> +
> +#define WX_VXMAILBOX_SIZE 15
> +
> +/* PF Registers */
> +#define WX_PXMAILBOX(i) (0x600 + (4 * (i))) /* i=[0,63] */
> +#define WX_PXMAILBOX_STS BIT(0) /* Initiate message send to VF */
> +#define WX_PXMAILBOX_ACK BIT(1) /* Ack message recv'd from VF */
> +#define WX_PXMAILBOX_PFU BIT(3) /* PF owns the mailbox buffer */
> +
> +#define WX_PXMBMEM(i) (0x5000 + (64 * (i))) /* i=[0,63] */
> +
> +#define WX_VFLRE(i) (0x4A0 + (4 * (i))) /* i=[0,1] */
> +#define WX_VFLREC(i) (0x4A8 + (4 * (i))) /* i=[0,1] */
> +
> +/* SR-IOV specific macros */
> +#define WX_MBVFICR(i) (0x480 + (4 * (i))) /* i=[0,3] */
> +#define WX_MBVFICR_VFREQ_MASK GENMASK(15, 0)
> +#define WX_MBVFICR_VFACK_MASK GENMASK(31, 16)
> +
> +#define WX_VT_MSGINFO_MASK GENMASK(23, 16)
> +
> +int wx_write_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf);
> +int wx_read_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf);
> +int wx_check_for_rst_pf(struct wx *wx, u16 mbx_id);
> +int wx_check_for_msg_pf(struct wx *wx, u16 mbx_id);
> +int wx_check_for_ack_pf(struct wx *wx, u16 mbx_id);
> +
> +#endif /* _WX_MBX_H_ */
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index 5aaf7b1fa2db..caa2f4157834 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -674,6 +674,10 @@ struct wx_bus_info {
> u16 device;
> };
>
> +struct wx_mbx_info {
> + u16 size;
> +};
> +
> struct wx_thermal_sensor_data {
> s16 temp;
> s16 alarm_thresh;
> @@ -995,6 +999,7 @@ struct wx {
> struct pci_dev *pdev;
> struct net_device *netdev;
> struct wx_bus_info bus;
> + struct wx_mbx_info mbx;
> struct wx_mac_info mac;
> enum em_mac_type mac_type;
> enum sp_media_type media_type;
Powered by blists - more mailing lists