Hi, Andy

On 09/02, 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>
>---
>Cc: qi.z.zh...@intel.com
>Cc: ferruh.yi...@intel.com
>Cc: rosen...@intel.com
>Cc: xiaolong...@intel.com
>
>v2:
>modify low bound and upper bound.
>
>v3:
>modify according to community comments.

Better to put more details about your changes other than the vague description.

>
> drivers/net/ipn3ke/ipn3ke_ethdev.c | 109 +++++++++++++++++++++++++++++++++++++
> drivers/net/ipn3ke/ipn3ke_ethdev.h |  15 +++++
> 2 files changed, 124 insertions(+)
>
>diff --git a/drivers/net/ipn3ke/ipn3ke_ethdev.c 
>b/drivers/net/ipn3ke/ipn3ke_ethdev.c
>index c226d63..711d48e 100644
>--- a/drivers/net/ipn3ke/ipn3ke_ethdev.c
>+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.c
>@@ -209,6 +209,112 @@
>       return 0;
> }
> 
>+static uint32_t
>+ipn3ke_mtu_cal(uint32_t tx, uint32_t rx)
>+{
>+      uint32_t tmp;
>+      tmp = RTE_MIN(tx, rx);
>+      if (tmp < RTE_ETHER_MIN_MTU)
>+              tmp = RTE_ETHER_MIN_MTU;

        tmp = RTE_MAX(tmp, RTE_ETHER_MIN_MTU);

>+      if (tmp > IPN3KE_MAC_FRAME_SIZE_MAX - IPN3KE_ETH_OVERHEAD)
>+              tmp = IPN3KE_MAC_FRAME_SIZE_MAX - IPN3KE_ETH_OVERHEAD;

        tmp = RTE_MIN(tmp, IPN3KE_MAC_FRAME_SIZE_MAX - IPN3KE_ETH_OVERHEAD);
        
>+      return tmp;
>+}
>+
>+static void
>+ipn3ke_10G_mtu_setup
>+(struct ipn3ke_hw *hw, uint32_t mac_num, uint32_t eth_group_sel)
>+{
>+      uint32_t tx;
>+      uint32_t rx;
>+      uint32_t tmp;
>+
>+      if (!(*hw->f_mac_read) || !(*hw->f_mac_write))
>+              return;
>+
>+      (*hw->f_mac_read)(hw,
>+                      &tx,
>+                      IPN3KE_10G_TX_FRAME_MAXLENGTH,
>+                      mac_num,
>+                      eth_group_sel);
>+
>+      (*hw->f_mac_read)(hw,
>+                      &rx,
>+                      IPN3KE_10G_RX_FRAME_MAXLENGTH,
>+                      mac_num,
>+                      eth_group_sel);
>+
>+      tmp = ipn3ke_mtu_cal(tx, rx);
>+
>+      (*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);
>+}
>+
>+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;
>+
>+      if (!(*hw->f_mac_read) || !(*hw->f_mac_write))
>+              return;
>+
>+      (*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 = ipn3ke_mtu_cal(tx, rx);
>+
>+      (*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);
>+}


Above 2 functions still share lots of duplicated code, can you extract the
duplicated code to a common function?

>+
>+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 +409,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..596df08 100644
>--- a/drivers/net/ipn3ke/ipn3ke_ethdev.h
>+++ b/drivers/net/ipn3ke/ipn3ke_ethdev.h
>@@ -654,6 +654,21 @@ 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_MAX_TX_SIZE_CONFIG                                0x407
>+#define IPN3KE_25G_MAX_RX_SIZE_CONFIG                                0x506
>+
>+#define IPN3KE_10G_TX_FRAME_MAXLENGTH                                0x002C
>+#define IPN3KE_10G_RX_FRAME_MAXLENGTH                                0x00AE
>+

It's better to remove unused MACROs here to make the patch clean.

Thanks,
Xiaolong

> #define IPN3KE_REGISTER_WIDTH                                        32
> 
> /*Bits[2:0]: Configuration of TX statistics counters:
>-- 
>1.8.3.1
>

Reply via email to