hi Xiao, Thanks for your comments! I will fix them ASAP.
Best regards, Xiaojun ________________________________ From: Wang, Xiao W <xiao.w.w...@intel.com> Sent: Tuesday, February 11, 2020 6:31 PM To: Xiaojun Liu <xiaojun....@silicom.co.il> Cc: dev@dpdk.org <dev@dpdk.org>; Zhang, Qi Z <qi.z.zh...@intel.com>; Kwan, Ngai-mint <ngai-mint.k...@intel.com>; Fornal, Jakub <jakub.for...@intel.com>; Keller, Jacob E <jacob.e.kel...@intel.com>; Jeff Zheng <jeff.zh...@silicom.co.il>; Eyal Cohen <ey...@silicom.co.il> Subject: RE: [PATCH v2 0/7] support switch management Since this is a big code change, so just some general comments/suggestions for your next version patch set: - please clean up comments like "XXX" in the code. - It's better to define MACRO for all register addrs and bit shift, please try to avoid magic number (e.g. in the serdes part). - There're threads created for handling events and timers, we'd better use pthread_join() to recycle them in fm10k_sm_detach(). - set "tapstop" and "shiftwidth" to 8 in vimrc, then recheck the macros and keep them aligned. - There're some "printf" for debugging, please use dpdk logging API instead. Best Regards, Xiao > -----Original Message----- > From: Xiaojun Liu <xiaojun....@silicom.co.il> > Sent: Tuesday, January 21, 2020 2:15 PM > To: Wang, Xiao W <xiao.w.w...@intel.com> > Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zh...@intel.com>; Kwan, Ngai-mint > <ngai-mint.k...@intel.com>; jakub.for...@intel.co; Keller, Jacob E > <jacob.e.kel...@intel.com>; Jeff Zheng <jeff.zh...@silicom.co.il>; Eyal > Cohen <ey...@silicom.co.il> > Subject: RE: [PATCH v2 0/7] support switch management > > Hi Xiao, > > Thank you! I will update the commit log and prepare a document to describe > the design and implementation. > > Best regards, > Xiaojun > > -----Original Message----- > From: Wang, Xiao W [mailto:xiao.w.w...@intel.com] > Sent: Tuesday, January 21, 2020 10:53 AM > To: Xiaojun Liu > Cc: dev@dpdk.org; Zhang, Qi Z; Kwan, Ngai-mint; jakub.for...@intel.co; > Keller, Jacob E > Subject: RE: [PATCH v2 0/7] support switch management > > Hi Xiaojun, > > Could you please help to improve the commit logs of all the 7 patches? They > look very similar, and info like below is not very helpful for reviewer, since > we already know which file you are adding. > "To support switch management, add the following files: > Add fm10k/switch/fm10k_debug.h(define log Macros). > Add fm10k/switch/fm10k_regs.h(define all the registers)." > > Please talk more about the design and implementation details in the commit > log. Refer to history patches if you need a sample. > > Also please help to address the compile error reported by automation in link > http://patches.dpdk.org/patch/63742/: > "ci/Intel-compilation fail Compilation issues" > > I would look deeper into your change, and you can address above comments > simultaneously. > > Best Regards, > Xiao > > > -----Original Message----- > > From: Xiaojun Liu <xiaojun....@silicom.co.il> > > Sent: Wednesday, December 11, 2019 5:52 PM > > To: Wang, Xiao W <xiao.w.w...@intel.com>; Zhang, Qi Z > > <qi.z.zh...@intel.com>; Kwan, Ngai-mint <ngai-mint.k...@intel.com>; > > jakub.for...@intel.co; Keller, Jacob E <jacob.e.kel...@intel.com> > > Cc: dev@dpdk.org; Xiaojun Liu <xiaojun....@silicom.co.il> > > Subject: [PATCH v2 0/7] support switch management > > > > To avoid configuration for both kernel driver > > and userspace SDK outside DPDK, we add switch > > management in FM10K DPDK PMD driver. > > To enable switch management, you need add > > CONFIG_RTE_FM10K_MANAGEMENT=y in > > config/common_linux when building. > > > > > > Xiaojun Liu (7): > > net/fm10k: add i2c sbus registers definition > > net/fm10k: add some modules of port > > net/fm10k: add config ffu statistics support > > net/fm10k: add flow and switch management > > net/fm10k: add switch initialization > > net/fm10k: add mirror and filter ctrl > > net/fm10k: add dpdk port mapping > > > > drivers/net/fm10k/Makefile | 22 + > > drivers/net/fm10k/fm10k_ethdev.c | 580 +++++- > > drivers/net/fm10k/switch/fm10k_config.c | 855 ++++++++ > > drivers/net/fm10k/switch/fm10k_config.h | 171 ++ > > drivers/net/fm10k/switch/fm10k_debug.h | 19 + > > drivers/net/fm10k/switch/fm10k_ext_port.c | 841 ++++++++ > > drivers/net/fm10k/switch/fm10k_ext_port.h | 136 ++ > > drivers/net/fm10k/switch/fm10k_ffu.c | 1209 +++++++++++ > > drivers/net/fm10k/switch/fm10k_ffu.h | 31 + > > drivers/net/fm10k/switch/fm10k_flow.c | 872 ++++++++ > > drivers/net/fm10k/switch/fm10k_flow.h | 26 + > > drivers/net/fm10k/switch/fm10k_i2c.c | 310 +++ > > drivers/net/fm10k/switch/fm10k_i2c.h | 54 + > > drivers/net/fm10k/switch/fm10k_regs.h | 2202 > ++++++++++++++++++++ > > drivers/net/fm10k/switch/fm10k_sbus.c | 292 +++ > > drivers/net/fm10k/switch/fm10k_sbus.h | 40 + > > drivers/net/fm10k/switch/fm10k_serdes.c | 1886 +++++++++++++++++ > > drivers/net/fm10k/switch/fm10k_serdes.h | 32 + > > drivers/net/fm10k/switch/fm10k_sm.c | 182 ++ > > drivers/net/fm10k/switch/fm10k_sm.h | 78 + > > drivers/net/fm10k/switch/fm10k_spico_code.c | 2966 > > +++++++++++++++++++++++++++ > > drivers/net/fm10k/switch/fm10k_spico_code.h | 21 + > > drivers/net/fm10k/switch/fm10k_stats.c | 1242 +++++++++++ > > drivers/net/fm10k/switch/fm10k_stats.h | 257 +++ > > drivers/net/fm10k/switch/fm10k_switch.c | 2562 > > +++++++++++++++++++++++ > > drivers/net/fm10k/switch/fm10k_switch.h | 336 +++ > > 26 files changed, 17188 insertions(+), 34 deletions(-) > > create mode 100644 drivers/net/fm10k/switch/fm10k_config.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_config.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_debug.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_ext_port.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_ext_port.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_ffu.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_ffu.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_flow.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_flow.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_i2c.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_i2c.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_regs.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_sbus.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_sbus.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_serdes.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_serdes.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_sm.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_sm.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_spico_code.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_spico_code.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_stats.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_stats.h > > create mode 100644 drivers/net/fm10k/switch/fm10k_switch.c > > create mode 100644 drivers/net/fm10k/switch/fm10k_switch.h > > > > -- > > 1.8.3.1
