Re: [NDCTL PATCH v9 4/4] cxl/test: Add test for cxl features device

[Marc Herbert <marc.herbert@linux.intel.com>]

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 "$?"
> +}