Hi Thomas, It will take some time to handle all the comments for this patch. So I would request you to defer this patch for next release and take other patches in series. Also I will send one patch to add feature matrix for dlb2 platform and fix minor review comments given by You.
Regards Sunil Kumar Kori >-----Original Message----- >From: Thomas Monjalon <tho...@monjalon.net> >Sent: Wednesday, November 24, 2021 4:23 PM >To: Sunil Kumar Kori <sk...@marvell.com> >Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; nikhil....@intel.com; >Pavan Nikhilesh Bhagavatula <pbhagavat...@marvell.com>; >hemant.agra...@nxp.com; nipun.gu...@nxp.com; >harry.van.haa...@intel.com; mattias.ronnb...@ericsson.com; >liang.j...@intel.com; dev@dpdk.org >Subject: [EXT] Re: [PATCH v8 10/10] devtools: check event device doc tables > >External Email > >---------------------------------------------------------------------- >23/11/2021 12:07, sk...@marvell.com: >> --- a/devtools/check-doc-vs-code.sh >> +++ b/devtools/check-doc-vs-code.sh >> +all_event_drivers() >> +{ >> + find $rootdir/drivers/event -mindepth 1 -maxdepth 1 -type d | >> + sed 's,.*/,,' | >> + sort >> +} >> + >> +check_event_dev() # <driver> >> +{ >> + code=$rootdir/drivers/event/$1 >> + doc=$rootdir/doc/guides/eventdevs/features/$1.ini >> + [ -d $code ] || return 0 >> + [ -f $doc ] || return 0 >> + report=$($selfdir/parse-event-support.sh $code $doc) >> + if [ -n "$report" ]; then >> + error "doc out of sync for $1" >> + echo "$report" | sed 's,^,\t,' >> + fi >> +} > >These 2 functions are mostly copy/paste of rte_flow functions. >Given there will be more in future, I would prefer code being factorized. > >> if [ -z "$trusted_commit" ]; then >> # check all >> for driver in $(all_net_drivers); do >> check_rte_flow $driver >> done >> + > >I would remove this blank line. > >> + for driver in $(all_event_drivers); do >> + check_event_dev $driver >> + done >> exit $result >> fi >[...] >> +if has_code_change 'RTE_EVENT_DEV_CAP_*' || >> + has_code_change 'RTE_EVENT_ETH_RX_ADAPTER_CAP_*' || >> + has_code_change 'RTE_EVENT_ETH_TX_ADAPTER_CAP_*' || >> + has_code_change 'RTE_EVENT_CRYPTO_ADAPTER_CAP_*' || >> + has_code_change 'RTE_EVENT_TIMER_ADAPTER_CAP_*' || > >Can it be a single query? > >> + has_file_change 'doc/guides/eventdevs/features'; then >> + for driver in $(all_event_drivers); do > >No need to check all drivers. >For rte_flow, only changed drivers are checked. > >> + check_event_dev $driver >> + done >> +fi >[...] >> +# generate INI section >> +list() # <title> <pattern> <extra_patterns> { >> + echo "[$1]" >> + word0=$(git grep -who "$2[[:alnum:]_]*" $dir) >> + word1=$(echo "$3") > >Why echo? > >> + words="$word0""$word1" > >Why so many quotes? > >> + echo "$words" | sort -u | >> + awk 'sub(/'$2'/, "") {printf "%-20s = Y\n", tolower($0)}' >> +} >[...] >> +event_dev_rx_adptr_support() >> +{ >> + title="Eth Rx adapter Features" >> + pattern=$(echo "RTE_EVENT_ETH_RX_ADAPTER_CAP_" | >> + awk '{print toupper($0)}') >> + check_rx_adptr_sw_capa || >extra='RTE_EVENT_ETH_RX_ADAPTER_CAP_OVERRIDE_FLOW_ID >> + > RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ >> + > RTE_EVENT_ETH_RX_ADAPTER_CAP_EVENT_VECTOR' > >Why having extra parameter, instead of updating the pattern? >By the way, the pattern RTE_EVENT_ETH_RX_ADAPTER_CAP_ already include >all of this. > >> + list "$title" "$pattern" "$extra" >> +} >[...] >> +# compare with reference input >> +event_dev_sched_compare() >> +{ >> + section="Scheduling Features]" >> + { >> + event_dev_sched_support >> + sed -n "/$section/,/]/p" "$ref" | sed '/^$/d' >> + } | >> + sed '/]/d' | # ignore section title >> + sed 's, *=.*,,' | # ignore value (better in doc than generated one) >> + sort | uniq -u | # show differences >> + sed "s,^,Scheduling Features ," # prefix with category name } >> + >> +event_dev_rx_adptr_compare() >> +{ >> + section="Eth Rx adapter Features]" >> + { >> + event_dev_rx_adptr_support >> + sed -n "/$section/,/]/p" "$ref" | sed '/^$/d' >> + } | >> + sed '/]/d' | # ignore section title >> + sed 's, *=.*,,' | # ignore value (better in doc than generated one) >> + sort | uniq -u | # show differences >> + sed "s,^,Eth Rx adapter Features ," # prefix with category name } >> + >> +event_dev_tx_adptr_compare() >> +{ >> + section="Eth Tx adapter Features]" >> + { >> + event_dev_tx_adptr_support >> + sed -n "/$section/,/]/p" "$ref" | sed '/^$/d' >> + } | >> + sed '/]/d' | # ignore section title >> + sed 's, *=.*,,' | # ignore value (better in doc than generated one) >> + sort | uniq -u | # show differences >> + sed "s,^,Eth Tx adapter Features ," # prefix with category name } >> + >> +event_dev_crypto_adptr_compare() >> +{ >> + section="Crypto adapter Features]" >> + { >> + event_dev_crypto_adptr_support >> + sed -n "/$section/,/]/p" "$ref" | sed '/^$/d' >> + } | >> + sed '/]/d' | # ignore section title >> + sed 's, *=.*,,' | # ignore value (better in doc than generated one) >> + sort | uniq -u | # show differences >> + sed "s,^,Crypto adapter Features ," # prefix with category name } >> + >> +event_dev_timer_adptr_compare() >> +{ >> + section="Timer adapter Features]" >> + { >> + event_dev_timer_adptr_support >> + sed -n "/$section/,/]/p" "$ref" | sed '/^$/d' >> + } | >> + sed '/]/d' | # ignore section title >> + sed 's, *=.*,,' | # ignore value (better in doc than generated one) >> + sort | uniq -u | # show differences >> + sed "s,^,Timer adapter Features ," # prefix with category name } > >I think these functions can be factorized. >