Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <31192e08-3d83-6fb0-06b0-51d50d073abc@redhat.com>
Date:   Thu, 1 Dec 2022 11:38:29 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH v2 01/17] drm/tests: helpers: Move the helper header to
 include/drm
Content-Language: en-US
To:     Maxime Ripard <maxime@cerno.tech>
Cc:     Daniel Vetter <daniel@ffwll.ch>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        David Airlie <airlied@gmail.com>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        dri-devel@lists.freedesktop.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        kunit-dev@googlegroups.com, linux-media@vger.kernel.org,
        linux-kselftest@vger.kernel.org, linaro-mm-sig@lists.linaro.org,
        Brendan Higgins <brendan.higgins@linux.dev>,
        =?UTF-8?Q?Ma=c3=adra_Canal?= <mairacanal@riseup.net>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        linux-kernel@vger.kernel.org, David Gow <davidgow@google.com>
References: <20221123-rpi-kunit-tests-v2-0-efe5ed518b63@cerno.tech>
 <20221123-rpi-kunit-tests-v2-1-efe5ed518b63@cerno.tech>
 <ebe02c21-7770-0415-096e-76a839bad154@redhat.com>
 <20221201102756.cdvxu4iryhu3rjpf@houat>
From:   Javier Martinez Canillas <javierm@redhat.com>
In-Reply-To: <20221201102756.cdvxu4iryhu3rjpf@houat>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hello Maxime,

On 12/1/22 11:27, Maxime Ripard wrote:

[...]

>>
>> I wonder if now that this header was moved outside of the tests directory,
>> if we should add stub functions in the header file that are just defined
>> but do nothing if CONFIG_DRM_KUNIT_TEST isn't enabled. So that including
>> it in drivers will be a no-op.
>>
>> Or do you plan to conditionally include this header file in drivers? So
>> that is only included when CONFIG_DRM_KUNIT_TEST is enabled?
> 
> I'm not entirely sure. I'd expect only the tests to include it, and thus
> would depend on DRM_KUNIT_TEST already. But we can always add the stubs
> if it's ever included in a different context.
> 
>> Another thing that wondered is if we want a different namespace for this
>> header, i.e: <drm/testing/drm_kunit_helpers.h>, to make it clear that is
>> not part of the DRM API but just for testing helpers.
> 
> If there's a single header, I don't think we need to create the
> directory. This is also something we can consolidate later on if needed.
>

Agree on both. It's better to land as is and then figure out if needs
to be changed once other drivers add more tests.
 
>> But these are open questions really, and they can be done as follow-up:
>>
>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> Thanks :)

You are welcome!

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat