linux-kernel - Re: [PATCH v7 2/2] drm/tests: bridge: add a KUnit test for devm_drm_bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250414-misty-hungry-woodlouse-dbbd64@houat>
Date: Mon, 14 Apr 2025 17:49:16 +0200
From: Maxime Ripard <mripard@...nel.org>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Simona Vetter <simona@...ll.ch>, Jonathan Corbet <corbet@....net>, 
	Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>, 
	Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>, 
	Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Anusha Srivatsa <asrivats@...hat.com>, Paul Kocialkowski <paulk@...-base.io>, 
	Dmitry Baryshkov <lumag@...nel.org>, Hervé Codina <herve.codina@...tlin.com>, 
	Hui Pu <Hui.Pu@...ealthcare.com>, Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
	dri-devel@...ts.freedesktop.org, linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 2/2] drm/tests: bridge: add a KUnit test for
 devm_drm_bridge_alloc()

Hi,

On Wed, Apr 09, 2025 at 04:50:35PM +0200, Luca Ceresoli wrote:
> Add a basic KUnit test for the newly introduced drm_bridge_alloc().
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
> 
> ---
> 
> Changed in v7:
>  - rebase on current drm-misc-next, which now has a drm_bridge_test.c file
>  - cleanup commit message
> 
> Changed in v6:
>  - update to new devm_drm_bridge_alloc() API
>  - remove drm_test_drm_bridge_put test, not straightforward to write with
>    the new API and the current notification mechanism
>  - do not allocate a drm_device: a bridge is allocated without one
>  - rename some identifiers for easier code reading
> 
> This patch was added in v5.
> ---
>  drivers/gpu/drm/tests/drm_bridge_test.c | 60 +++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
> index ff88ec2e911c9cc9a718483f09d4c764f45f991a..87fb64744b67f0780457a546aba77ba945a0ce67 100644
> --- a/drivers/gpu/drm/tests/drm_bridge_test.c
> +++ b/drivers/gpu/drm/tests/drm_bridge_test.c
> @@ -8,6 +8,7 @@
>  #include <drm/drm_bridge_helper.h>
>  #include <drm/drm_kunit_helpers.h>
>  
> +#include <kunit/device.h>
>  #include <kunit/test.h>
>  
>  struct drm_bridge_init_priv {
> @@ -407,11 +408,70 @@ 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 {
> +	struct device *dev;
> +};

You don't need a struct there then, you can just pass the device pointer.

> +/*
> + * Mimick the typical struct defined by a bridge driver, which embeds a
> + * bridge plus other fields.
> + */
> +struct dummy_drm_bridge {
> +	int dummy; // ensure we test non-zero @bridge offset
> +	struct drm_bridge bridge;
> +};

drm_bridge_init_priv gives you that already.

> +static const struct drm_bridge_funcs drm_bridge_dummy_funcs = {
> +};
> +
> +static int drm_test_bridge_alloc_init(struct kunit *test)
> +{
> +	struct drm_bridge_alloc_test_ctx *ctx;
> +
> +	ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> +	ctx->dev = kunit_device_register(test, "drm-bridge-dev");
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dev);
> +
> +	test->priv = ctx;
> +
> +	return 0;
> +}
> +
> +/*
> + * Test that the allocation and initialization of a bridge works as
> + * expected and doesn't report any error.
> + */
> +static void drm_test_drm_bridge_alloc(struct kunit *test)
> +{
> +	struct drm_bridge_alloc_test_ctx *ctx = test->priv;
> +	struct dummy_drm_bridge *dummy;
> +
> +	dummy = devm_drm_bridge_alloc(ctx->dev, struct dummy_drm_bridge, bridge,
> +				      &drm_bridge_dummy_funcs);
> +	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dummy);

Why did you need the dummy value in dummy_drm_bridge if you're not using
it?

We'd need a couple more tests, in particular some to make sure the
bridge pointer is properly cleaned up when the device goes away, but not
when we have called drm_bridge_get pointer on it, etc.

Maxime

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)