| Message ID | 20231127233558.868365-3-nfraprado@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add test to verify probe of devices from discoverable busses | expand |
> -----Original Message----- > From: Nícolas F. R. A. Prado <nfraprado@collabora.com> > 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 <nfraprado@collabora.com> > --- > > (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
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.
On Tue, Nov 28, 2023 at 12:10:46AM +0000, Bird, Tim wrote: > > -----Original Message----- > > From: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > 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 <nfraprado@collabora.com> > > --- > > > > (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
> -----Original Message----- > From: Nícolas F. R. A. Prado <nfraprado@collabora.com> > On Tue, Nov 28, 2023 at 12:10:46AM +0000, Bird, Tim wrote: > > > -----Original Message----- > > > From: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > 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 <nfraprado@collabora.com> > > > --- > > > > > > (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
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 <nfraprado@collabora.com> > --- > > (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 -- kernel@mailman.collabora.com > To unsubscribe send an email to kernel-leave@mailman.collabora.com
> -----Original Message----- > From: Christopher Obbard <chris.obbard@collabora.com> > 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
On Tue, Nov 28, 2023 at 05:54:57PM +0000, Bird, Tim wrote: > > -----Original Message----- > > From: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > On Tue, Nov 28, 2023 at 12:10:46AM +0000, Bird, Tim wrote: > > > > -----Original Message----- > > > > From: Nícolas F. R. A. Prado <nfraprado@collabora.com> > > > > 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 <nfraprado@collabora.com> > > > > --- > > > > > > > > (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
On Tue, Nov 28, 2023 at 06:33:52PM +0000, Bird, Tim wrote: > > > > -----Original Message----- > > From: Christopher Obbard <chris.obbard@collabora.com> > > 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
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
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 <nfraprado@collabora.com> --- (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