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 <redacted>
Sent: Wednesday, November 24, 2021 4:23 PM
To: Sunil Kumar Kori <redacted>
Cc: Jerin Jacob Kollanukkaran <redacted>; nikhil.rao@intel.com;
Pavan Nikhilesh Bhagavatula [off-list ref];
hemant.agrawal@nxp.com; nipun.gupta@nxp.com;
harry.van.haaren@intel.com; mattias.ronnblom@ericsson.com;
liang.j.ma@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, skori@marvell.com:

quoted

--- 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.

quoted

 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.

quoted

+	for driver in $(all_event_drivers); do
+		check_event_dev $driver
+	done
 	exit $result
 fi

[...]

quoted

+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?

quoted

+		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.

quoted

+		check_event_dev $driver
+	done
+fi

[...]

quoted

+# generate INI section
+list() # <title> <pattern> <extra_patterns> {
+	echo "[$1]"
+	word0=$(git grep -who "$2[[:alnum:]_]*" $dir)
+	word1=$(echo "$3")

Why echo?

quoted

+	words="$word0""$word1"

Why so many quotes?

quoted

+	echo "$words" | sort -u |
+		awk 'sub(/'$2'/, "") {printf "%-20s = Y\n", tolower($0)}'
+}

[...]

quoted

+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

quoted

+

RTE_EVENT_ETH_RX_ADAPTER_CAP_MULTI_EVENTQ

quoted

+

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.

quoted

+	list "$title" "$pattern" "$extra"
+}

[...]

quoted

+# 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.