Re: chrome-platform/for-kernelci baseline: 98 runs, 5 regressions (v6.1-rc1-5-g27b86a65cd16)

[Brian Norris <briannorris@chromium.org>]

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