Hello Maxime,
thanks for reviewing this series.
On Tue, 27 May 2025 18:10:31 +0200
Maxime Ripard <mrip...@kernel.org> wrote:
[...]
> > @@ -422,11 +424,93 @@ static struct kunit_suite
> > drm_bridge_helper_reset_crtc_test_suite = {
> > .test_cases = drm_bridge_helper_reset_crtc_tests,
> > };
> >
> > +struct drm_bridge_alloc_test_ctx {
>
> drm_bridge_alloc_priv
>
> > + struct device *dev;
> > + struct dummy_drm_bridge *dummy_br;
> > + bool destroyed;
>
> This can be in drm_bridge_priv
Not really, because drm_bridge_priv will be freed just after calling
.destroy, and we need .destroyed after the free happened.
[...]
> > +static const struct drm_bridge_funcs drm_bridge_dummy_funcs = {
> > + .destroy = dummy_drm_bridge_destroy,
> > +};
>
> And same here, you don't need to create yet another function set, just
> add it to the existing ones.
OK, but it implies further changes.
In this version of this patch, the alloc tests being introduced use
drm_bridge_alloc_priv, while the other tests using the existing
function sets rely on drm_bridge_init_priv which has different fields.
So if all tests will call .destroy, we always need a valid struct
pointer for drm_bridge_priv.data.
Based on this, I think the only solution is to not introduce
drm_bridge_alloc_priv, and instead put its members (struct device *dev and bool
destroyed) to drm_bridge_init_priv, and then use drm_bridge_init_priv
for all tests.
The change is not very invasive, and perhaps even a cleanup, thus I'm
going to send as above in v9.
I'm OK with all the other changes you proposed. All queued for v9.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
