[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <C5DDDB438F5F4EF9+20250813074622.GE944516@nic-Precision-5820-Tower>
Date: Wed, 13 Aug 2025 15:46:22 +0800
From: Yibo Dong <dong100@...se.com>
To: "Anwar, Md Danish" <a0501179@...com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
corbet@....net, gur.stavi@...wei.com, maddy@...ux.ibm.com,
mpe@...erman.id.au, danishanwar@...com, lee@...ger.us,
gongfan1@...wei.com, lorenzo@...nel.org, geert+renesas@...der.be,
Parthiban.Veerasooran@...rochip.com, lukas.bulwahn@...hat.com,
alexanderduyck@...com, richardcochran@...il.com,
netdev@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support
On Tue, Aug 12, 2025 at 10:00:57PM +0530, Anwar, Md Danish wrote:
> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> > Initialize basic mbx function.
> >
> > Signed-off-by: Dong Yibo <dong100@...se.com>
> > ---
> > drivers/net/ethernet/mucse/rnpgbe/Makefile | 3 +-
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h | 37 ++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c | 5 +
> > drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h | 2 +
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c | 443 ++++++++++++++++++
> > .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h | 31 ++
> > 6 files changed, 520 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> >
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > index 42c359f459d9..5fc878ada4b1 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > @@ -6,4 +6,5 @@
> >
> > obj-$(CONFIG_MGBE) += rnpgbe.o
> > rnpgbe-objs := rnpgbe_main.o\
> > - rnpgbe_chip.o
> > + rnpgbe_chip.o\
> > + rnpgbe_mbx.o
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 0dd3d3cb2a4d..05830bb73d3e 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > @@ -5,6 +5,7 @@
> > #define _RNPGBE_H
> >
> > #include <linux/types.h>
> > +#include <linux/mutex.h>
> >
> > extern const struct rnpgbe_info rnpgbe_n500_info;
> > extern const struct rnpgbe_info rnpgbe_n210_info;
> > @@ -40,7 +41,43 @@ struct mucse_mac_info {
> > void *back;
> > };
> >
> > +struct mucse_hw;
> > +
> > +struct mucse_mbx_operations {
> > + void (*init_params)(struct mucse_hw *hw);
> > + int (*read)(struct mucse_hw *hw, u32 *msg,
> > + u16 size);
> > + int (*write)(struct mucse_hw *hw, u32 *msg,
> > + u16 size);
> > + int (*read_posted)(struct mucse_hw *hw, u32 *msg,
> > + u16 size);
> > + int (*write_posted)(struct mucse_hw *hw, u32 *msg,
> > + u16 size);
> > + int (*check_for_msg)(struct mucse_hw *hw);
> > + int (*check_for_ack)(struct mucse_hw *hw);
> > + void (*configure)(struct mucse_hw *hw, int num_vec,
> > + bool enable);
> > +};
> > +
> > +struct mucse_mbx_stats {
> > + u32 msgs_tx;
> > + u32 msgs_rx;
> > + u32 acks;
> > + u32 reqs;
> > + u32 rsts;
> > +};
> > +
> > struct mucse_mbx_info {
> > + const struct mucse_mbx_operations *ops;
> > + struct mucse_mbx_stats stats;
> > + u32 timeout;
> > + u32 usec_delay;
> > + u16 size;
> > + u16 fw_req;
> > + u16 fw_ack;
> > + /* lock for only one use mbx */
> > + struct mutex lock;
> > + bool irq_enabled;
> > /* fw <--> pf mbx */
> > u32 fw_pf_shm_base;
> > u32 pf2fw_mbox_ctrl;
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > index 20ec67c9391e..16d0a76114b5 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -1,8 +1,11 @@
> > // SPDX-License-Identifier: GPL-2.0
> > /* Copyright(c) 2020 - 2025 Mucse Corporation. */
> >
> > +#include <linux/string.h>
> > +
> > #include "rnpgbe.h"
> > #include "rnpgbe_hw.h"
> > +#include "rnpgbe_mbx.h"
> >
> > /**
> > * rnpgbe_init_common - Setup common attribute
> > @@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
> >
> > mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
> > mac->back = hw;
> > +
> > + hw->mbx.ops = &mucse_mbx_ops_generic;
> > }
> >
> > /**
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > index fc57258537cf..aee037e3219d 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > @@ -7,6 +7,8 @@
> > #define RNPGBE_RING_BASE 0x1000
> > #define RNPGBE_MAC_BASE 0x20000
> > #define RNPGBE_ETH_BASE 0x10000
> > +/**************** DMA Registers ****************************/
> > +#define RNPGBE_DMA_DUMY 0x000c
> > /**************** CHIP Resource ****************************/
> > #define RNPGBE_MAX_QUEUES 8
> > #endif /* _RNPGBE_HW_H */
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > new file mode 100644
> > index 000000000000..1195cf945ad1
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > @@ -0,0 +1,443 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> > +
> > +#include <linux/pci.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +#include <linux/iopoll.h>
> > +
> > +#include "rnpgbe.h"
> > +#include "rnpgbe_mbx.h"
> > +#include "rnpgbe_hw.h"
> > +
> > +/**
> > + * mucse_read_mbx - Reads a message from the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > + /* limit read size */
> > + min(size, mbx->size);
> > + return mbx->ops->read(hw, msg, size);
> > +}
>
> What's the purpose of min() here if you are anyways passing size to read()?
>
> The min() call needs to be assigned to size, e.g.: size = min(size,
> mbx->size);
>
Yes, I will fix this error.
> > +
> > +/**
> > + * mucse_write_mbx - Write a message to the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
>
> > +
> > +/**
> > + * mucse_mbx_reset - Reset mbx info, sync info from regs
> > + * @hw: pointer to the HW structure
> > + *
> > + * This function reset all mbx variables to default.
> > + **/
> > +static void mucse_mbx_reset(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + int v;
> > +
>
> Variable 'v' should be declared as u32 to match the register read.
>
Got it, I will fix it.
> > + v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > + hw->mbx.fw_req = v & GENMASK(15, 0);
> > + hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > + mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > + mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > +}
> > +
> > +/**
> > + * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
> > + * @hw: pointer to the HW structure
> > + * @nr_vec: vector number for mbx
> > + * @enable: TRUE for enable, FALSE for disable
> > + *
> > + * This function configure mbx to use interrupt nr_vec.
> > + **/
> > +static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
> > + bool enable)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > + u32 v;
> > +
> > + if (enable) {
> > + v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > + hw->mbx.fw_req = v & GENMASK(15, 0);
> > + hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > + mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > + mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
> > + mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > + } else {
> > + mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
> > + mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > + mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
> > + }
> > +}
> > +
> > +/**
> > + * mucse_init_mbx_params_pf - Set initial values for pf mailbox
> > + * @hw: pointer to the HW structure
> > + *
> > + * Initializes the hw->mbx struct to correct values for pf mailbox
> > + */
> > +static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> > +{
> > + struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > + mbx->usec_delay = 100;
> > + mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;
>
> Use appropriate constants like USEC_PER_SEC instead of hardcoded values.
>
Ok, I will update it.
> > + mbx->stats.msgs_tx = 0;
> > + mbx->stats.msgs_rx = 0;
>
>
> --
> Thanks and Regards,
> Md Danish Anwar
>
>
Thanks for your feedback.
Powered by blists - more mailing lists