Re: [RFC PATCH 2/2] kselftest: devices: Add board file for google,spherion

From: "Nícolas F. R. A. Prado" <nfraprado@collabora.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Shuah Khan <shuah@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	devicetree@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-usb@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	linux-kselftest@vger.kernel.org, kernel@collabora.com,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] kselftest: devices: Add board file for google,spherion
Date: Wed, 25 Oct 2023 08:32:42 -0400	[thread overview]
Message-ID: <e49c63c4-2b24-4428-801c-1f854a98c593@notapiano> (raw)
In-Reply-To: <2023102546-filled-onboard-3dfb@gregkh>

On Wed, Oct 25, 2023 at 12:32:15PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 24, 2023 at 05:18:00PM -0400, Nícolas F. R. A. Prado wrote:
> > Add the list of devices expected to be probed from the USB and PCI
> > busses on the google,spherion machine. The USB host controller at
> > 11200000 is shared between two busses, for USB2 and USB3, so an
> > additional match is used to select the USB2 bus.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---
> > 
> >  tools/testing/selftests/devices/boards/google,spherion | 3 +++
> >  1 file changed, 3 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..ba86ffcfe43c
> > --- /dev/null
> > +++ b/tools/testing/selftests/devices/boards/google,spherion
> > @@ -0,0 +1,3 @@
> > +usb camera 11200000,PRODUCT=.*/2/.* 1.4.1 1 0,1
> > +usb bluetooth 11200000,PRODUCT=.*/2/.* 1.4.2 1 0,1
> > +pci wifi 11230000 0.0/0.0
> 
> USB busses (and PCI ids) are not determinisitic and can, and will,
> change values randomly.  So while it is nice to test "did the devices
> show up properly", you can not do that based on bus ids at all, sorry.
> 
> Unless I'm reading these values wrong?  What are the fields
> representing?  Perhaps a comment at the top to describe them so that we
> know how to parse them?

Hi Greg,

I have described the fields in the commit message of patch 1. Here they are:

usb <test_name> <controller_address>[,<additional_match>] <ports_path> <configuration> <interfaces>

pci <test_name> <controller_address> <device-function_pairs_path>

I'm aware that bus IDs are assigned at runtime, and that's exactly why I've
avoided those in the test definitions, instead describing the hardware topology,
which won't ever change.

And just to be extra clear, by hardware topology I mean:

For USB, we find the USB bus based on the address of its controller (and
optionally its productID if two busses share the same controller for USB2 and
USB3), and then find the device by following the ports at each hub. The
configuration number and interfaces then describe what interfaces to check for
presence and driver binding.

For PCI, we find the controller again based on its address, and follow the
device-function pairs at each level in the topology until we arrive at the
desired device.

We don't rely on the USB bus number, nor on the PCI domain and bus number, since
these are all assigned at runtime.

Thanks,
Nícolas