Re: [PATCH v3 02/27] staging: iio: resolver: ad2s1210: fix use before initialization

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 6 Oct 2023 14:48:29 +0000
Vincent Whitchurch <Vincent.Whitchurch@axis.com> wrote:

Hi Vincent

Thanks for the update,

> On Mon, 2023-10-02 at 10:17 +0100, Jonathan Cameron wrote:
> > Hmm. What happened to roadtest?  I was hoping that would solve this sort
> > of issue by allowing simple testing of basic functionality...  
> 
> Roadtest is alive and well.  Several of my coworkers have been using it
> for development and testing of new drivers[0][1][2][3][4] and
> patches[5][6], and this has resulted in easier testing and refactoring
> during development, more robust code, and of course the ability to
> easily detect regressions after the patches are merged.
> 
> [0] https://lore.kernel.org/lkml/20230323-add-opt4001-driver-v2-2-0bae0398669d@axis.com/
> [1] https://lore.kernel.org/lkml/d218a1bc75402b5ebd6e12a563f7315f83fe966c.1689753076.git.waqar.hameed@axis.com/
> [2] https://lore.kernel.org/lkml/7b856b74c4c0f8c6c539d7c692051c9203b103c0.1692699931.git.waqar.hameed@axis.com/
> [3] https://lore.kernel.org/lkml/20231002-rx8111-add-timestamp0-v1-1-353727cf7f14@axis.com/
> [4] https://lore.kernel.org/lkml/20230502-tps6287x-driver-v3-2-e25140a023f5@axis.com/
> [5] https://lore.kernel.org/lkml/20221012102347.153201-1-chenhuiz@axis.com/
> [6] https://lore.kernel.org/lkml/20220413114014.2204623-3-camel.guo@axis.com/
> 
> In fact, by running our roadtests on newer kernels we have found
> numerous bugs[10][12][14] and regressions[7][8][9][11][15] in mainline,
> including subsystem-level issues affecting other drivers too.
> 
> [7] https://lore.kernel.org/lkml/20230911-regulator-voltage-sel-v1-1-886eb1ade8d8@axis.com/
> [8] https://lore.kernel.org/lkml/20230918-power-uaf-v1-1-73c397178c42@axis.com/
> [9] https://lore.kernel.org/lkml/20230829-tps-voltages-v1-1-7ba4f958a194@axis.com/
> [10] https://lore.kernel.org/lkml/20230613-genirq-nested-v3-1-ae58221143eb@axis.com/
> [11] https://lore.kernel.org/lkml/20220503114333.456476-1-camel.guo@axis.com/
> [12] https://lore.kernel.org/linux-iio/20220816080828.1218667-1-vincent.whitchurch@axis.com/
> [13] https://lore.kernel.org/linux-iio/20220519091925.1053897-1-vincent.whitchurch@axis.com/
> [14] https://lore.kernel.org/linux-iio/20220620144231.GA23345@axis.com/
> [15] https://lore.kernel.org/linux-spi/YxBX4bXG02E4lSUW@axis.com/
> 
> (The above lists are not exhaustive.)
> 

Great stuff!

> > Hope it is still headed for a new version / upstream!  
> 
> I pushed out an update with a squash of (most parts of) our internal
> version out to the following repo, it's based on v6.6-rc4.
> 
>   https://github.com/vwax/linux/tree/roadtest/devel

Thanks.

> 
> (There are currently 6 lines of --diff-filter=M against v6.6-rc4 on the
>  linked repo.  Two of those are from a patch which is posted and waiting
>  for review on the lists, and the rest are for enabling regmap debugfs
>  writes which are used from some of the newer tests.)
> 
> Since roadtest itself does not require any patches to the kernel or any
> out-of-tree modules, the maintenance of the framework would not really
> be simplified by putting it in the upstream tree.  However, there is of
> course a potentially large benefit to the quality of many kinds of
> kernel drivers if roadtest gets used by others, and having it in-tree
> could facilitate that.  And it would potentially allow regressions like
> the ones we're finding to be caught _before_ they go in, since anyone
> can run the tests without special hardware.

Exactly  - my main interest is the dream of getting to the point where
new drivers typically also come with roadtest tests, with the aim that
they will be used for regression testing. For IIO I might lean on
/ ask nicely  few of the bigger contributors to add fairly comprehensive
tests for say one in 3 of their drivers, providing a canary for any
subsystem level problems that might sneak in. The stability gained for
those drivers might also prove it's own benefit to push people to add tests.
At somepoint in the longer term I might even make it a requirement for
upstreaming a new driver + slowly tackle the backlog of existing ones.
From my experiments with it last year, this is a trivial burden fo

> 
> The idea of having to maintain it in-tree and doing all the work that
> goes along with that (dealing with the expectations of maintainers,
> wrangling patches from mailing lists, etc), is something I personally
> have had a hard time warming up to, but I have some coworkers who may
> potentially be interested in that kind of work, so I wouldn't rule out
> another posting of the patch set targeting upstream sometime in the
> future.

I fully appreciate your concern.  I just really like roadtest and want
a smooth way to integrate using it with my upstream maintenance (and occasional
development) process...  I of course can't expect you to commit to anything
though - I'd be delighted if someone else wants to take this forwards but
that would be very much their decision to make!

Having not yet waded into the latest code, how 'stable' is it from the point
of view of modifications to tests?  I can rebase the ones I have out of tree
and see, but I'm after an assessment that incorporates what you are
planning to change in future.

I guess the nasty stuff is if you have a few hundred additional drivers
in the test set, any modification to the way they interact with the core
of roadtest becomes very painful to push into those tests.

One starting point would be to separate the tests directory from the
directories containing roadtest frameworks etc as that would help to
limit scope of responsibility.

If a potential upstream roadtest maintainer is primarily concerned about
review + handling of the actual tests, other than potentially letting in
some ugly code, I'd imagine any subsystem maintainer who opts into this
will take that burden on - perhaps with the occasional question heading
your way. I'd certainly not expect you to have to deal with high patch flows
and would ensure that didn't happen for any IIO tests (any review people
have time for is of course welcome!)

+CC a few maintainers of other subsystems who may be interested (I know
one of them is ;)

Jonathan