linux-kernel - Re: [PATCH v10 2/2] drm: add kms driver for loongson display controller

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: <2833ed06-7f8a-b8c1-404e-b481b2fedb3f@189.cn>
Date:   Tue, 11 Apr 2023 11:33:39 +0800
From:   Sui Jingfeng <15330273260@....cn>
To:     Emil Velikov <emil.l.velikov@...il.com>
Cc:     Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Christian Koenig <christian.koenig@....com>,
        linaro-mm-sig@...ts.linaro.org, Li Yi <liyi@...ngson.cn>,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        nathan@...nel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH v10 2/2] drm: add kms driver for loongson display
 controller

Hi,

On 2023/4/4 22:10, Emil Velikov wrote:
>> --- /dev/null
>> +++ b/drivers/gpu/drm/loongson/lsdc_debugfs.c
>> +void lsdc_debugfs_init(struct drm_minor *minor)
>> +{
>> +#ifdef CONFIG_DEBUG_FS
>> +       drm_debugfs_create_files(lsdc_debugfs_list,
>> +                                ARRAY_SIZE(lsdc_debugfs_list),
>> +                                minor->debugfs_root,
>> +                                minor);
>> +#endif
>> +}
> Should probably build the file when debugfs is enabled and provide
> no-op stub in the header. See nouveau for an example.
>
But doing that way introduce duplication,  you actually write two 
implements for the same function prototype.

One for the real, another one for the dummy.

Typically skilled core framework programmer/writer like it, for multiple 
backend and multiple arch support

Because the functions set need to be implemented is large for those cases.

While we are just a driver implement based the drm core and only one 
single function here,

DEBUG_FS is enabled by default on our Mips and Loongarch. It is not 
suffer from high frequency changes.

In this case , CONFIG_DEBUG_FS just boils down to "true", a nearly 
always enabled decoration.


We do implement debugfs support that way in the before[1], but we pursue 
compact in the afterwards.

We could revise our driver if that is strongly recommended.


[1] https://patchwork.freedesktop.org/patch/480521/