2023-11-27 23:36:18

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [RFC PATCH v2 0/2] Add test to verify probe of devices from discoverable busses


Hi,

v1 [1] was discussed during Plumbers [2], where a lot of feedback was given. I
hope to justify the changes in v2 and address the feedback here.

One feedback from Shuah was that keeping per-platform files with the USB/PCI
devices to test as part of the kselftest tree wasn't maintainable. One proposed
alternative was to generate a list of probed devices on a known-good kernel and
use that as a reference. However you need someone to look at that generated
reference to be able to say it is a good one, and you need to save it to ensure
it will be reproducible later anyway, so that wouldn't actually solve the
problem. It is a matter of hand-crafting vs generating the test definitions, but
they will need to be vouched by someone and stored somewhere in both cases.

So for this v2, in patch 2 I just have a sample test definition, and the
per-platform test definitions would be added to a separate repository.

The other feedback received was that the BIOS might reconfigure the PCI
topology (at least on x86), meaning that relying on a sequence of device and
function numbers (eg 1d.0/02.0/0.0) as a stable description of a device on the
platform is not possible. I couldn't verify whether this is really the case (if
you have any more insight into this, please let me know), but with that in mind,
here in v2 I have taken a different approach. Here I'm using the device's
properties which are used for driver matching (the same that show on modalias)
to identify a device in a stable way.

This approach has some drawbacks compared to the one on v1. For one it doesn't
uniquely identify a device, so if there are multiple of the same device on a
platform they have to be checked as a group. Also the test definition isn't as
human-readable.

I'm adding in CC the people I recognized at the Plumbers session that were
interested in this work. Feel free to add anyone missing.

Thanks,
Nícolas

[1] https://lore.kernel.org/all/[email protected]
[2] https://www.youtube.com/watch?v=oE73eVSyFXQ&t=9377s

Original cover letter:

This is part of an effort to improve detection of regressions impacting
device probe on all platforms. The recently merged DT kselftest [3]
detects probe issues for all devices described statically in the DT.
That leaves out devices discovered at run-time from discoverable busses.

This is where this test comes in. All of the devices that are connected
through discoverable busses (ie USB and PCI), and which are internal and
therefore always present, can be described in a per-platform file so
they can be checked for. The test will check that the device has been
instantiated and bound to a driver.

Patch 1 introduces the test. Patch 2 adds the test definitions for the
google,spherion machine (Acer Chromebook 514) as an example.

This is the sample output from the test running on Spherion:

TAP version 13
Using board file: boards/google,spherion
1..3
ok 1 usb.camera
ok 2 usb.bluetooth
ok 3 pci.wifi
Totals: pass:3 fail:0 xfail:0 xpass:0 skip:0 error:0

[3] https://lore.kernel.org/all/[email protected]/

Changes in v2:
- Changed approach of encoding stable device reference in test file from
HW topology to device match fields (the ones from modalias)
- Better documented test format

Nícolas F. R. A. Prado (2):
kselftest: Add test to verify probe of devices from discoverable
busses
kselftest: devices: Add sample board file for google,spherion

tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/devices/.gitignore | 1 +
tools/testing/selftests/devices/Makefile | 8 +
.../selftests/devices/boards/google,spherion | 12 ++
.../devices/test_discoverable_devices.sh | 160 ++++++++++++++++++
5 files changed, 182 insertions(+)
create mode 100644 tools/testing/selftests/devices/.gitignore
create mode 100644 tools/testing/selftests/devices/Makefile
create mode 100644 tools/testing/selftests/devices/boards/google,spherion
create mode 100755 tools/testing/selftests/devices/test_discoverable_devices.sh

--
2.42.1


2023-11-27 23:36:40

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

Add a sample board file describing the file's format and with the list
of devices expected to be probed on the google,spherion machine as an
example.

Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
---

(no changes since v1)

.../testing/selftests/devices/boards/google,spherion | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 tools/testing/selftests/devices/boards/google,spherion

diff --git a/tools/testing/selftests/devices/boards/google,spherion b/tools/testing/selftests/devices/boards/google,spherion
new file mode 100644
index 000000000000..db9a17cccd03
--- /dev/null
+++ b/tools/testing/selftests/devices/boards/google,spherion
@@ -0,0 +1,12 @@
+# Example test definition for Google Spherion Chromebook
+#
+# Format:
+# usb|pci test_name number_of_matches field=value [ field=value ... ]
+#
+# The available match fields vary by bus. The field-value match pairs for a
+# device can be retrieved from the device's modalias attribute in sysfs. A
+# subset of the fields may be used to make the match more generic so it can work
+# with the different hardware variants of a device on the machine.
+usb camera 1 ic=0e isc=01 ip=00
+usb bluetooth 1 ic=e0 isc=01 ip=01 in=00
+pci wifi 1 v=14c3 d=7961
--
2.42.1

2023-11-27 23:36:42

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: [RFC PATCH v2 1/2] kselftest: Add test to verify probe of devices from discoverable busses

Add a new test to verify that a list of expected devices on a given
platform have been successfully probed by a driver.

Add a new test to verify that all expected devices from discoverable
busses (ie USB, PCI) have been successfully instantiated and probed by a
driver.

The per-platform list of expected devices is selected from the ones
under the boards/ directory based on compatible.

Signed-off-by: Nícolas F. R. A. Prado <[email protected]>

---

(no changes since v1)

tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/devices/.gitignore | 1 +
tools/testing/selftests/devices/Makefile | 8 +
.../devices/test_discoverable_devices.sh | 160 ++++++++++++++++++
4 files changed, 170 insertions(+)
create mode 100644 tools/testing/selftests/devices/.gitignore
create mode 100644 tools/testing/selftests/devices/Makefile
create mode 100755 tools/testing/selftests/devices/test_discoverable_devices.sh

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 3b2061d1c1a5..7f5088006c3c 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -13,6 +13,7 @@ TARGETS += core
TARGETS += cpufreq
TARGETS += cpu-hotplug
TARGETS += damon
+TARGETS += devices
TARGETS += dmabuf-heaps
TARGETS += drivers/dma-buf
TARGETS += drivers/s390x/uvdevice
diff --git a/tools/testing/selftests/devices/.gitignore b/tools/testing/selftests/devices/.gitignore
new file mode 100644
index 000000000000..e3c5c04d1b19
--- /dev/null
+++ b/tools/testing/selftests/devices/.gitignore
@@ -0,0 +1 @@
+ktap_helpers.sh
diff --git a/tools/testing/selftests/devices/Makefile b/tools/testing/selftests/devices/Makefile
new file mode 100644
index 000000000000..ff2fdc8fc5e2
--- /dev/null
+++ b/tools/testing/selftests/devices/Makefile
@@ -0,0 +1,8 @@
+TEST_PROGS := test_discoverable_devices.sh
+TEST_GEN_FILES := ktap_helpers.sh
+TEST_FILES := boards
+
+include ../lib.mk
+
+$(OUTPUT)/ktap_helpers.sh:
+ cp ../dt/ktap_helpers.sh $@
diff --git a/tools/testing/selftests/devices/test_discoverable_devices.sh b/tools/testing/selftests/devices/test_discoverable_devices.sh
new file mode 100755
index 000000000000..82bad5ba7081
--- /dev/null
+++ b/tools/testing/selftests/devices/test_discoverable_devices.sh
@@ -0,0 +1,160 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2023 Collabora Ltd
+#
+# This script tests for presence and driver binding of devices from discoverable
+# busses (ie USB, PCI).
+#
+# The per-platform list of devices to be tested is stored inside the boards/
+# directory and chosen based on compatible. Each line of the file follows the
+# following format:
+#
+# usb|pci test_name number_of_matches field=value [ field=value ... ]
+#
+# The available match fields vary by bus. The field-value match pairs for a
+# device can be retrieved from the device's modalias attribute in sysfs.
+#
+
+DIR="$(dirname $(readlink -f "$0"))"
+
+source "${DIR}"/ktap_helpers.sh
+
+KSFT_FAIL=1
+KSFT_SKIP=4
+
+retval=0
+
+match()
+{
+ FILE="$1"
+ ID="$2"
+
+ [ ! -f "$FILE" ] && return 1
+ [ "$ID" = "" ] && return 0
+ grep -q "$ID" "$FILE" || return 1
+ return 0
+}
+
+usb()
+{
+ name="$1"
+ count="$2"
+ shift 2
+
+ for arg in $@; do
+ [[ "$arg" =~ ^v= ]] && v="${arg#v=}"
+ [[ "$arg" =~ ^p= ]] && p="${arg#p=}"
+ [[ "$arg" =~ ^d= ]] && d="${arg#d=}"
+ [[ "$arg" =~ ^dc= ]] && dc="${arg#dc=}"
+ [[ "$arg" =~ ^dsc= ]] && dsc="${arg#dsc=}"
+ [[ "$arg" =~ ^dp= ]] && dp="${arg#dp=}"
+ [[ "$arg" =~ ^ic= ]] && ic="${arg#ic=}"
+ [[ "$arg" =~ ^isc= ]] && isc="${arg#isc=}"
+ [[ "$arg" =~ ^ip= ]] && ip="${arg#ip=}"
+ [[ "$arg" =~ ^in= ]] && in="${arg#in=}"
+ done
+
+ cur_count=0
+
+ for dev in $(find /sys/bus/usb/devices -maxdepth 1); do
+ match "$dev"/idVendor "$v" || continue
+ match "$dev"/idProduct "$p" || continue
+ match "$dev"/bcdDevice "$d" || continue
+ match "$dev"/bDeviceClass "$dc" || continue
+ match "$dev"/bDeviceSubClass "$dsc" || continue
+ match "$dev"/bDeviceProtocol "$dp" || continue
+
+ # Matched device. Now search through interfaces
+ for intf in $(find "$dev"/ -maxdepth 1 -type d); do
+ match "$intf"/bInterfaceClass "$ic" || continue
+ match "$intf"/bInterfaceSubClass "$isc" || continue
+ match "$intf"/bInterfaceProtocol "$ip" || continue
+ match "$intf"/bInterfaceNumber "$in" || continue
+
+ # Matched interface. Add to count if it was probed by driver.
+ [ -d "$intf"/driver ] && cur_count=$((cur_count+1))
+ done
+ done
+
+ if [ "$cur_count" -eq "$count" ]; then
+ ktap_test_pass usb."$name"
+ else
+ ktap_test_fail usb."$name"
+ retval="$KSFT_FAIL"
+ fi
+}
+
+pci()
+{
+ name="$1"
+ count="$2"
+ shift 2
+
+ for arg in $@; do
+ [[ "$arg" =~ ^v= ]] && v="${arg#v=}"
+ [[ "$arg" =~ ^d= ]] && d="${arg#d=}"
+ [[ "$arg" =~ ^sv= ]] && sv="${arg#sv=}"
+ [[ "$arg" =~ ^sd= ]] && sd="${arg#sd=}"
+ [[ "$arg" =~ ^bc= ]] && bc="${arg#bc=}"
+ [[ "$arg" =~ ^sc= ]] && sc="${arg#sc=}"
+ [[ "$arg" =~ ^i= ]] && i="${arg#i=}"
+ done
+
+ cur_count=0
+
+ for dev in $(find /sys/bus/pci/devices -maxdepth 1); do
+ match "$dev"/vendor "$v" || continue
+ match "$dev"/device "$d" || continue
+ match "$dev"/subsystem_vendor "$sv" || continue
+ match "$dev"/subsystem_device "$sd" || continue
+
+ [ -z "$bc" ] && bc='..'
+ [ -z "$sc" ] && sc='..'
+ [ -z "$i" ] && i='..'
+ match "$dev/"class "$bc$sc$i" || continue
+
+ # Matched device. Add to count if it was probed by driver.
+ [ -d "$dev"/driver ] && cur_count=$((cur_count+1))
+ done
+
+ if [ "$cur_count" -eq "$count" ]; then
+ ktap_test_pass pci."$name"
+ else
+ ktap_test_fail pci."$name"
+ retval="$KSFT_FAIL"
+ fi
+}
+
+ktap_print_header
+
+plat_compatible=/proc/device-tree/compatible
+
+if [ ! -f "$plat_compatible" ]; then
+ ktap_skip_all "No board compatible available"
+ exit "$KSFT_SKIP"
+fi
+
+compatibles=$(tr '\000' '\n' < "$plat_compatible")
+
+for compatible in $compatibles; do
+ if [ -f boards/"$compatible" ]; then
+ board_file=boards/"$compatible"
+ break
+ fi
+done
+
+if [ -z "$board_file" ]; then
+ ktap_skip_all "No matching board file found"
+ exit "$KSFT_SKIP"
+fi
+
+echo "# Using board file: " "$board_file"
+
+num_tests=$(grep -E "^(usb|pci)" "$board_file" | wc -l)
+ktap_set_plan "$num_tests"
+
+source "$board_file"
+
+ktap_print_totals
+exit "${retval}"
--
2.42.1

2023-11-28 00:36:46

by Bird, Tim

[permalink] [raw]
Subject: RE: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

> -----Original Message-----
> From: Nícolas F. R. A. Prado <[email protected]>
> Add a sample board file describing the file's format and with the list
> of devices expected to be probed on the google,spherion machine as an
> example.
>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
> ---
>
> (no changes since v1)
>
> .../testing/selftests/devices/boards/google,spherion | 12 ++++++++++++

Overall, while trying to maintain a comprehensive set of board definitions
seems hard, I think having a few as examples is useful.

I'm not a big fan of naming these with a comma in the name. Is there a reason
you are not using dash or underscore?

Do you anticipate a convention of <producer> <board-or-product-name> tuples for
the filename?
-- Tim

> 1 file changed, 12 insertions(+)
> create mode 100644 tools/testing/selftests/devices/boards/google,spherion
>
> diff --git a/tools/testing/selftests/devices/boards/google,spherion b/tools/testing/selftests/devices/boards/google,spherion
> new file mode 100644
> index 000000000000..db9a17cccd03
> --- /dev/null
> +++ b/tools/testing/selftests/devices/boards/google,spherion
> @@ -0,0 +1,12 @@
> +# Example test definition for Google Spherion Chromebook
> +#
> +# Format:
> +# usb|pci test_name number_of_matches field=value [ field=value ... ]
> +#
> +# The available match fields vary by bus. The field-value match pairs for a
> +# device can be retrieved from the device's modalias attribute in sysfs. A
> +# subset of the fields may be used to make the match more generic so it can work
> +# with the different hardware variants of a device on the machine.
> +usb camera 1 ic=0e isc=01 ip=00
> +usb bluetooth 1 ic=e0 isc=01 ip=01 in=00
> +pci wifi 1 v=14c3 d=7961
> --
> 2.42.1

2023-11-28 14:11:55

by Mark Brown

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

On Tue, Nov 28, 2023 at 12:10:46AM +0000, Bird, Tim wrote:

> I'm not a big fan of naming these with a comma in the name. Is there a reason
> you are not using dash or underscore?

That's a common patter with this sort of software (eg, bootrr does the
same). It's convenient to just be able to use the compatible straight
from DT without having to mangle it.


Attachments:
(No filename) (369.00 B)
signature.asc (499.00 B)
Download all attachments

2023-11-28 14:45:02

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

On Tue, Nov 28, 2023 at 12:10:46AM +0000, Bird, Tim wrote:
> > -----Original Message-----
> > From: N?colas F. R. A. Prado <[email protected]>
> > Add a sample board file describing the file's format and with the list
> > of devices expected to be probed on the google,spherion machine as an
> > example.
> >
> > Signed-off-by: N?colas F. R. A. Prado <[email protected]>
> > ---
> >
> > (no changes since v1)
> >
> > .../testing/selftests/devices/boards/google,spherion | 12 ++++++++++++
>
> Overall, while trying to maintain a comprehensive set of board definitions
> seems hard, I think having a few as examples is useful.
>
> I'm not a big fan of naming these with a comma in the name. Is there a reason
> you are not using dash or underscore?

I'm using the name that we get from the DT compatible, so the right file can be
automatically selected by the test.

>
> Do you anticipate a convention of <producer> <board-or-product-name> tuples for
> the filename?

I'd just stick to the DT compatible as it's the simplest option and should work
just the same, assuming I understood correctly what you mean.

Thanks,
N?colas

2023-11-28 17:57:23

by Bird, Tim

[permalink] [raw]
Subject: RE: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

> -----Original Message-----
> From: Nícolas F. R. A. Prado <[email protected]>
> On Tue, Nov 28, 2023 at 12:10:46AM +0000, Bird, Tim wrote:
> > > -----Original Message-----
> > > From: Nícolas F. R. A. Prado <[email protected]>
> > > Add a sample board file describing the file's format and with the list
> > > of devices expected to be probed on the google,spherion machine as an
> > > example.
> > >
> > > Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
> > > ---
> > >
> > > (no changes since v1)
> > >
> > > .../testing/selftests/devices/boards/google,spherion | 12 ++++++++++++
> >
> > Overall, while trying to maintain a comprehensive set of board definitions
> > seems hard, I think having a few as examples is useful.
> >
> > I'm not a big fan of naming these with a comma in the name. Is there a reason
> > you are not using dash or underscore?
>
> I'm using the name that we get from the DT compatible, so the right file can be
> automatically selected by the test.
>
> >
> > Do you anticipate a convention of <producer> <board-or-product-name> tuples for
> > the filename?
>
> I'd just stick to the DT compatible as it's the simplest option and should work
> just the same, assuming I understood correctly what you mean.

OK - I see that was mentioned in the original submission. I should
have read more closely.

It makes sense. Maybe it's worth mentioning in the commit message that the
filename is the compatible string from the DT for this board?

This convention, IMHO, should be documented somewhere.

Thanks.
-- Tim

2023-11-28 18:00:05

by Christopher Obbard

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

Hi Nícolas,

On Mon, 2023-11-27 at 18:34 -0500, Nícolas F. R. A. Prado wrote:
> Add a sample board file describing the file's format and with the list
> of devices expected to be probed on the google,spherion machine as an
> example.

Did you consider using some machine-readable & extensible format like yaml?
Surely we don't need to invent yet-another file-format? :-)


Chris

>
> Signed-off-by: Nícolas F. R. A. Prado <[email protected]>
> ---
>
> (no changes since v1)
>
>  .../testing/selftests/devices/boards/google,spherion | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>  create mode 100644 tools/testing/selftests/devices/boards/google,spherion
>
> diff --git a/tools/testing/selftests/devices/boards/google,spherion
> b/tools/testing/selftests/devices/boards/google,spherion
> new file mode 100644
> index 000000000000..db9a17cccd03
> --- /dev/null
> +++ b/tools/testing/selftests/devices/boards/google,spherion
> @@ -0,0 +1,12 @@
> +# Example test definition for Google Spherion Chromebook
> +#
> +# Format:
> +#   usb|pci test_name number_of_matches field=value [ field=value ... ]
> +#
> +# The available match fields vary by bus. The field-value match pairs for a
> +# device can be retrieved from the device's modalias attribute in sysfs. A
> +# subset of the fields may be used to make the match more generic so it can
> work
> +# with the different hardware variants of a device on the machine.
> +usb camera 1 ic=0e isc=01 ip=00
> +usb bluetooth 1 ic=e0 isc=01 ip=01 in=00
> +pci wifi 1 v=14c3 d=7961
> --
> 2.42.1
>
> _______________________________________________
> Kernel mailing list -- [email protected]
> To unsubscribe send an email to [email protected]

2023-11-28 18:34:33

by Bird, Tim

[permalink] [raw]
Subject: RE: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion



> -----Original Message-----
> From: Christopher Obbard <[email protected]>
> Hi Nícolas,
>
> On Mon, 2023-11-27 at 18:34 -0500, Nícolas F. R. A. Prado wrote:
> > Add a sample board file describing the file's format and with the list
> > of devices expected to be probed on the google,spherion machine as an
> > example.
>
> Did you consider using some machine-readable & extensible format like yaml?
> Surely we don't need to invent yet-another file-format? :-)

I went back to examine the test more closely. These board files are loaded via
the shell's 'source' command.

If I'm reading the test correctly, the format is machine-readable and extensible, since
it's a fragment of a shell script. The 'usb' and 'pci' first entries on the lines are
actually function calls, and the other items in a test line are arguments.

So, as an RFC - how about calling the board files: "<compatible-string>.sh" to make this
clear, and maybe adding a comment at the top about the nature of the file?

There's probably a use case for reading this file not in this original shell script context,
so I think Christopher's point about a machine-readable AND easily human-readable
format is valid. Personally, I find this format not too bad to read (but then I'm a
shell junky.)

I believe, Nicolas, that you were already planning on putting some comments in the
file to describe the line format (function arguments?), based on feedback from Greg KH.
IMHO, knowing that the format allows comments is useful, so adding a sample
comment would be welcome.
-- Tim

2023-11-28 19:52:42

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

On Tue, Nov 28, 2023 at 05:54:57PM +0000, Bird, Tim wrote:
> > -----Original Message-----
> > From: N?colas F. R. A. Prado <[email protected]>
> > On Tue, Nov 28, 2023 at 12:10:46AM +0000, Bird, Tim wrote:
> > > > -----Original Message-----
> > > > From: N?colas F. R. A. Prado <[email protected]>
> > > > Add a sample board file describing the file's format and with the list
> > > > of devices expected to be probed on the google,spherion machine as an
> > > > example.
> > > >
> > > > Signed-off-by: N?colas F. R. A. Prado <[email protected]>
> > > > ---
> > > >
> > > > (no changes since v1)
> > > >
> > > > .../testing/selftests/devices/boards/google,spherion | 12 ++++++++++++
> > >
> > > Overall, while trying to maintain a comprehensive set of board definitions
> > > seems hard, I think having a few as examples is useful.
> > >
> > > I'm not a big fan of naming these with a comma in the name. Is there a reason
> > > you are not using dash or underscore?
> >
> > I'm using the name that we get from the DT compatible, so the right file can be
> > automatically selected by the test.
> >
> > >
> > > Do you anticipate a convention of <producer> <board-or-product-name> tuples for
> > > the filename?
> >
> > I'd just stick to the DT compatible as it's the simplest option and should work
> > just the same, assuming I understood correctly what you mean.
>
> OK - I see that was mentioned in the original submission. I should
> have read more closely.
>
> It makes sense. Maybe it's worth mentioning in the commit message that the
> filename is the compatible string from the DT for this board?
>
> This convention, IMHO, should be documented somewhere.

I have that as part of the comment at the top of the test script in patch 1:

# The per-platform list of devices to be tested is stored inside the boards/
# directory and chosen based on compatible.

And also in the commit message of patch 1.

But I guess this sample file is the most likely one to be read when someone
writes a new board file, so I'll document it here too for next version.

Thanks,
N?colas

2023-11-28 21:06:05

by Nícolas F. R. A. Prado

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/2] kselftest: devices: Add sample board file for google,spherion

On Tue, Nov 28, 2023 at 06:33:52PM +0000, Bird, Tim wrote:
>
>
> > -----Original Message-----
> > From: Christopher Obbard <[email protected]>
> > Hi N?colas,
> >
> > On Mon, 2023-11-27 at 18:34 -0500, N?colas F. R. A. Prado wrote:
> > > Add a sample board file describing the file's format and with the list
> > > of devices expected to be probed on the google,spherion machine as an
> > > example.
> >
> > Did you consider using some machine-readable & extensible format like yaml?
> > Surely we don't need to invent yet-another file-format? :-)

For this RFC my focus was to gather feedback on even more basic aspects of the
test, mainly:
- Is using a device's match properties (the ones that constitute modalias) a
good way to encode a device in a stable way or can we do better? (See cover
letter for comparison to HW topology approach)

So I just went for the simplest format I could think of. Moving forward, I agree
YAML might be a better fit and I can try it out for the next version.

>
> I went back to examine the test more closely. These board files are loaded via
> the shell's 'source' command.
>
> If I'm reading the test correctly, the format is machine-readable and extensible, since
> it's a fragment of a shell script. The 'usb' and 'pci' first entries on the lines are
> actually function calls, and the other items in a test line are arguments.
>
> So, as an RFC - how about calling the board files: "<compatible-string>.sh" to make this
> clear, and maybe adding a comment at the top about the nature of the file?
>
> There's probably a use case for reading this file not in this original shell script context,
> so I think Christopher's point about a machine-readable AND easily human-readable
> format is valid. Personally, I find this format not too bad to read (but then I'm a
> shell junky.)

That's right, the board files are shell scripts that are sourced. I went this
route for simplicity rather than necessity. In fact, I'd prefer to have the
board files be dumb files with just the data necessary to describe the devices
on the platform moving forward. For this purpose I'll try using YAML for the
next version and seeing how it goes.

>
> I believe, Nicolas, that you were already planning on putting some comments in the
> file to describe the line format (function arguments?), based on feedback from Greg KH.
> IMHO, knowing that the format allows comments is useful, so adding a sample
> comment would be welcome.

Well, the text added at the top of this file describing the format of each line
was already done in response to Greg's comment. Although I didn't mention
anything about comments indeed, I'll make sure to document that for next
version (even if it is in YAML it doesn't hurt to have comments as part of the
example).

Also, I've noticed that my patches show "(no changes since v1)". Please
disregard these. There have clearly been changes since v1 (the whole approach is
different), which I've documented on the cover letter, but those trailers were
added by mistake when generating the patches.

Thanks,
N?colas