On Mon, Oct 31, 2022 at 10:52:52PM +0000, Mark Brown wrote:
On Mon, Oct 31, 2022 at 03:21:55PM -0700, Brian Norris wrote:
I can't find a single mention of "i2s1" or "probed" in the kernelci repo, so I must be missing something. Is there some external config file in another repo? Or else the test configs are autogenerating cases on the fly based on parsing...the device tree?
The KernelCI repo just says what testsuites to invoke and how, it's not got the actual testsuites. Those X didn't probe failures come from bootrr:
https://github.com/andersson/bootrr
forked to:
https://github.com/kernelci/bootrr
(which could use some upstreaming...) with the specific errors for
Neither of those looks particularly active. If I patch stuff, is it better to send PRs to the 'andersson' one or the 'kernelci' one?
gru-kevin coming from:
https://github.com/kernelci/bootrr/blob/main/boards/google%2Ckevin
which ends up in our rootfss.
Ah, thanks. That helps. Although it hurts in other ways, see below.
Those failures in particular come from some reorganisation of the DT for the Rockchip devices a while back which regularly gets bisected by our bisect bot, I did report it or something very similar as looking like a false positive but nobody followed up. I see there's some version dependent checks for the acclerators which may not be working properly any more I guess but nothing for the I2S.
Anyway, I don't know how or why that ever passed, because AFAICT, RK3399 Chromebooks should only have a single I2S block enabled, and they're passing the 'rockchip-i2s0-probed' case. So it feels like I need to be disabling some test case.
Yes, that was what I'd determined too - the reorganisation of the DT looked legit, I can't remember what it was exactly. I suspect it may have boiled down to adding some missing default disables, or removing an erroious enable for the board.
Ah, based off your pointers, I see the test was looking for what used to be the i2s2 alias. But then I recall we stopped using that i2s instance:
https://git.kernel.org/linus/b5fbaf7d779f5f02b7f75b080e7707222573be2a arm64: dts: rockchip: Switch RK3399-Gru DP to SPDIF output
I forgot that folks did that downstream long ago but never bothered finishing upstreaming that until I got to it this year... ...but still, it's kinda sad that we've bothered to set up all this "CI" and then nobody paid any attention :( I only noticed because I recently subscribed to chrome-platform@lists.linux.dev.
Anyway, I guess I gotta go patch the test expectations.
Somewhat similar story for cros-ec-sensors-accel{0,1}-probed, although I believe the sensor driver is still working for me; I also see no cros-ec-sensors errors in the KernelCI logs. So I wonder what exactly the test is looking for (e.g., maybe the device name changed?).
IIRC there were some of these that were a device name change.
Oh, this one makes me gag.
"assert_device_present cros-ec-sensors-accel0-probed cros-ec-sensors cros-ec-accel.11.*"
? Really, ".11"? That sounds like we're trying to test kernel implementation details, asynchronous probe race conditions, Makefile / linker ordering, and similar -- not anything that we actually expect to remain stable across kernel versions :(
I'm not sure there's a great stable way to refer to such devices, so maybe it'd be better to write this as "count the number of devices" instead? Or I think this particular driver supports an "id" sysfs attribute, which refers to a stable underlying firmware ID. But that'd involve even more device-specific logic.
I don't think I even care *why* the ID changed; that ID is far from a stable thing, if I'm reading it correctly. At least most of the others refer to hardware addresses, which are a little more reasonable to rely on (even if the device naming still isn't a stable guarantee).
Brian