Attention is currently required from: Angel Pons, Erik van den Bogaert, Felix Singer, Frans Hendriks, Jeremy Soller, Michał Żygowski, Nico Huber, Piotr Król, Sean Rhodes, Tim Crawford.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75888?usp=email )
Change subject: mb/{skl,kbl}: Use true/false keywords for non-array dt options ......................................................................
Patch Set 5:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75888/comment/0c8060ea_76f0c9bb : PS5, Line 25: soc_gen="skylake" && \ : options=`grep -A 1337 -r "struct soc_intel_${soc_gen}_config" "src/soc/intel/${soc_gen}/chip.h" | \ : grep "bool" | \ : cut -d ';' -f 1 | \ : tr '\t' ' ' | \ : cut -d ' ' -f 3 | \ : sed -E 's/[a-zA-Z0-9_]+[.+]//g'` : devicetrees=`find "src/mainboard" -name '*.cb' \ : -exec grep -H "chip soc/intel/${soc_gen}" {} ; | \ : cut -d ':' -f 1` && \ : for option in ${options}; do \ : echo ${devicetrees} | \ : xargs sed -i'' -E \ : -e "s/"${option}"(\s+)=(\s+)"1"/${option}\1=\2true/g" \ : -e "s/"${option}"(\s+)=(\s+)"0"/${option}\1=\2false/g"; \ : done : I would modify the script like this:
- in some cases grep thinks, chip.h is a binary... add -a, since we know better - make sure to only match the struct in chip.h by using grep -z and multiline to match for the first closing brace + semicolon, which are not indented - drop ${soc_gen} in the chip.h regex, it's already limited to one chipset by the file path - replace `grep -H ... | cut ...` by `grep -l`, which does exactly the same - use a _proper_ regular expression for matching the options instead of the cut/tr/sed combination which is intransparent and might fail in corner cases - use a _proper_ regular expression for modifying the devicetrees by combining the options and using match + replacement regex (this also spares us the use of for loop + xargs) - don't try to make everything a one-liner, which only makes it more hard to read
All this leads to this script, which has the same result (tested and verified for skylake), but is just a bit cleaner IMHO:
``` soc_gen="skylake" options=`grep -Poz '(?s)struct soc_intel_[^ ]+_config {.*\n};\n' \ src/soc/intel/${soc_gen}/chip.h | \ grep -aoP '(?<=bool )[^[]]+(?=;)' | \ sed -zE 's/\n/|/g; s/|$//'` find src/mainboard -name *.cb | xargs grep -l skylake | \ xargs sed -i -E "/register\s+"?($options)/ {s/"//g; s/(=\s*)0/\1false/; s/(= \s*)1/\1true/}" ```
Explanation:
``` soc_gen="skylake" # grep the full struct - ((?s) enables DOTALL, -z makes multiline matching possible, -P is Perl regex, -o only output the matching part) options=`grep -Poz '(?s)struct soc_intel_[^ ]+_config {.*\n};\n' \ src/soc/intel/${soc_gen}/chip.h | \ # get all bool options that are not array type ([^[]] ignores any ...[...]) grep -aoP '(?<=bool )[^[]]+(?=;)' | \ # make it a regex OR expression like option1|option2|option3|; drop the trailing | sed -zE 's/\n/|/g; s/|$//'` # for any skylake devicetree ... find src/mainboard -name *.cb | xargs grep -l skylake | \ # match the boolean register options (with optional whitespace and optional quotes) and 1) drop the quotes 2) replace 0/1 with false/true xargs sed -i -E "/register\s+"?($options)/ {s/"//g; s/(=\s*)0/\1false/; s/(= \s*)1/\1true/}" ```