Again, apologies for commenting so late (v9). I reviewed only the shell script below and all these comments could be "future improvements" post merge. Or, before merge if there is a v10.
On 2025-05-23 09:46, Dave Jiang wrote: > --- /dev/null > +++ b/test/cxl-features.sh > @@ -0,0 +1,37 @@ > +#!/bin/bash -Ex > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (C) 2025 Intel Corporation. All rights reserved. > + > +main() > +{ > + local rc=77 > + # 237 is -ENODEV > + ERR_NODEV=237 Nit: local ERR_NODEV=237 > + > + . $(dirname "$0")/common Mmmm... interesting, moving this too goes beyond what I recommended but maybe it's better? In theory, sourced files have no side-effect or barely. In practice though... LGTM! > + FEATURES="$TEST_PATH"/fwctl > + > + trap 'err $LINENO' ERR > + > + modprobe cxl_test > + > + test -x "$FEATURES" || do_skip "no CXL Features Control" > + > + rc=0 > + "$FEATURES" || rc=$? > + > + echo "error: $rc" echo "status: $rc" > + if [ "$rc" -eq "$ERR_NODEV" ]; then > + do_skip "no CXL FWCTL char dev" > + elif [ "$rc" -ne 0 ]; then > + echo "fail: $LINENO" && exit 1 > + fi > + > + trap 'err $LINENO' ERR I had a chat with David and he confirmed this duplicate is an oversight. > + > + _cxl_cleanup > +} > + > +{ > + main "$@"; exit "$?" > +}