Re: [PATCH v8 3/3] drm/tests: bridge: add KUnit tests for devm_drm_bridge_alloc()

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hello Maxime,

thanks for reviewing this series.

On Tue, 27 May 2025 18:10:31 +0200
Maxime Ripard <mripard@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