HI xiaolong,
Reply inline.
-----Original Message-----
From: Ye, Xiaolong
Sent: Wednesday, August 21, 2019 2:16 PM
To: Pei, Andy <andy....@intel.com>
Cc: dev@dpdk.org; Xu, Rosen <rosen...@intel.com>; Zhang, Tianfei
<tianfei.zh...@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2] net/ipn3ke: setup MTU when HW init
On 08/09, Andy Pei wrote:
>set up mtu to the minimun in tx mtu, rx mtu and IPN3KE_MAC_FRAME_SIZE_MAX.
>
>Signed-off-by: Andy Pei <andy....@intel.com>
>---
>v2:
>* modify low bound and upper bound.
>
> drivers/net/ipn3ke/ipn3ke_ethdev.c | 97
> ++++++++++++++++++++++++++++++++++++++
> drivers/net/ipn3ke/ipn3ke_ethdev.h | 19 ++++++++
> 2 files changed, 116 insertions(+)
>
>diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c
>b/drivers/net/ipn3ke/ipn3ke_ethdev.c
>index c226d63..1a6c217 100644
>--- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
>+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
>@@ -209,6 +209,100 @@
> return 0;
> }
>
>+static void ipn3ke_10G_mtu_setup
>+(struct ipn3ke_hw *hw, uint32_t mac_num, uint32_t eth_group_sel)
The dpdk convention is:
static void
ipn3ke_10G_mtu_setup(xxx)
OK
>+{
>+ uint32_t tx;
>+ uint32_t rx;
>+ uint32_t tmp;
>+
>+ (*hw->f_mac_read)(hw,
>+ &tx,
>+ IPN3KE_10G_TX_FRAME_MAXLENGTH,
>+ mac_num,
>+ eth_group_sel);
Need to verify whether hw->f_mac_read is NULL or not.
ok
>+
>+ (*hw->f_mac_read)(hw,
>+ &rx,
>+ IPN3KE_10G_RX_FRAME_MAXLENGTH,
>+ mac_num,
>+ eth_group_sel);
>+
>+ tmp = (tx > rx) ? rx : tx;
Can use RTE_MIN
OK
>+ if (tmp < RTE_ETHER_MIN_MTU)
>+ tmp = RTE_ETHER_MIN_MTU;
>+ if (tmp > IPN3KE_MAC_FRAME_SIZE_MAX - IPN3KE_ETH_OVERHEAD)
>+ tmp = IPN3KE_MAC_FRAME_SIZE_MAX - IPN3KE_ETH_OVERHEAD;
>+
>+ (*hw->f_mac_write)(hw,
>+ tmp,
>+ IPN3KE_10G_TX_FRAME_MAXLENGTH,
>+ mac_num,
>+ eth_group_sel);
>+
>+ (*hw->f_mac_write)(hw,
>+ tmp,
>+ IPN3KE_10G_RX_FRAME_MAXLENGTH,
>+ mac_num,
>+ eth_group_sel);
Need to check f_mac_write is not NULL before calling it.
ok
>+}
>+
>+static void ipn3ke_25G_mtu_setup
>+(struct ipn3ke_hw *hw, uint32_t mac_num, uint32_t eth_group_sel) {
>+ uint32_t tx;
>+ uint32_t rx;
>+ uint32_t tmp;
>+
>+ (*hw->f_mac_read)(hw,
>+ &tx,
>+ IPN3KE_25G_MAX_TX_SIZE_CONFIG,
>+ mac_num,
>+ eth_group_sel);
>+
>+ (*hw->f_mac_read)(hw,
>+ &rx,
>+ IPN3KE_25G_MAX_RX_SIZE_CONFIG,
>+ mac_num,
>+ eth_group_sel);
>+
>+ tmp = (tx > rx) ? rx : tx;
>+ if (tmp < RTE_ETHER_MIN_MTU)
>+ tmp = RTE_ETHER_MIN_MTU;
>+ if (tmp > IPN3KE_MAC_FRAME_SIZE_MAX - IPN3KE_ETH_OVERHEAD)
>+ tmp = IPN3KE_MAC_FRAME_SIZE_MAX - IPN3KE_ETH_OVERHEAD;
>+
>+ (*hw->f_mac_write)(hw,
>+ tmp,
>+ IPN3KE_25G_MAX_TX_SIZE_CONFIG,
>+ mac_num,
>+ eth_group_sel);
>+
>+ (*hw->f_mac_write)(hw,
>+ tmp,
>+ IPN3KE_25G_MAX_RX_SIZE_CONFIG,
>+ mac_num,
>+ eth_group_sel);
>+}
>+
ipn3ke_10G_mtu_setup and ipn3ke_25G_mtu_setup share most of the code, please
try to refactor to avoid duplicate code.
ok
>+static void
>+ipn3ke_mtu_setup(struct ipn3ke_hw *hw) {
>+ int i;
>+ if (hw->retimer.mac_type == IFPGA_RAWDEV_RETIMER_MAC_TYPE_10GE_XFI) {
>+ for (i = 0; i < hw->port_num; i++) {
>+ ipn3ke_10G_mtu_setup(hw, i, 0);
>+ ipn3ke_10G_mtu_setup(hw, i, 1);
>+ }
>+ } else if (hw->retimer.mac_type ==
>+ IFPGA_RAWDEV_RETIMER_MAC_TYPE_25GE_25GAUI) {
>+ for (i = 0; i < hw->port_num; i++) {
>+ ipn3ke_25G_mtu_setup(hw, i, 0);
>+ ipn3ke_25G_mtu_setup(hw, i, 1);
>+ }
>+ }
>+}
>+
> static int
> ipn3ke_hw_init(struct rte_afu_device *afu_dev,
> struct ipn3ke_hw *hw)
>@@ -303,6 +397,9 @@
> }
> }
>
>+ /* init mtu */
>+ ipn3ke_mtu_setup(hw);
>+
> ret = rte_eth_switch_domain_alloc(&hw->switch_domain_id);
> if (ret)
> IPN3KE_AFU_PMD_WARN("failed to allocate switch domain for
> device
>%d", diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.h
>b/drivers/net/ipn3ke/ipn3ke_ethdev.h
>index c7b336b..6419da3 100644
>--- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
>+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
>@@ -654,6 +654,25 @@ static inline void _ipn3ke_indrct_write(struct
>ipn3ke_hw *hw, #define IPN3KE_MAC_RX_FRAME_MAXLENGTH_MASK \
> IPN3KE_MASK(0xFFFF, IPN3KE_MAC_RX_FRAME_MAXLENGTH_SHIFT)
>
>+/* Additional Feature Register */
>+#define ADD_PHY_CTRL 0x0
>+#define PHY_RESET BIT(0)
>+/* registers for 25G/40G mac */
>+#define MAC_CONFIG 0x310
>+#define MAC_RESET_MASK GENMASK(2, 0)
>+
>+#define IPN3KE_MAX_MTU 0xffff
>+
>+#define IPN3KE_25G_PHY_PMA_SLOOP 0x313
>+#define IPN3KE_25G_TX_FLOW_CTRL 0x640
>+#define IPN3KE_25G_MAX_TX_SIZE_CONFIG 0x407
>+#define IPN3KE_25G_MAX_RX_SIZE_CONFIG 0x506
>+
>+#define IPN3KE_10G_TX_PAUSE_FRAME_QUANTA 0x0042
>+#define IPN3KE_10G_TX_PAUSE_FRAME_HOLDOFF 0x0043
>+#define IPN3KE_10G_TX_FRAME_MAXLENGTH 0x002C
>+#define IPN3KE_10G_RX_FRAME_MAXLENGTH 0x00AE
>+
> #define IPN3KE_REGISTER_WIDTH 32
I saw some MACRO definitions are not used in this patch.
I copy these MACRO from ipn3ke kernel driver in case we will use later.
If not use currently, I can delete these MACRO.
And by the way, why there is no mtu_set ops for ipn3ke ethdev?
Mtu_set ops is in ipn3ke_representor.c
Thanks,
Xiaolong
>
> /*Bits[2:0]: Configuration of TX statistics counters:
>--
>1.8.3.1
>
