Hi Am 19.03.20 um 13:27 schrieb Wambui Karuga: > > > On Thu, 19 Mar 2020, Daniel Vetter wrote: > >> On Thu, Mar 19, 2020 at 08:55:24AM +0100, Greg KH wrote: >>> On Wed, Mar 18, 2020 at 08:10:43PM +0100, Daniel Vetter wrote: >>>> On Wed, Mar 18, 2020 at 5:58 PM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> >>>> wrote: >>>>> >>>>> On Wed, Mar 18, 2020 at 05:31:47PM +0100, Daniel Vetter wrote: >>>>>> On Wed, Mar 18, 2020 at 5:03 PM Wambui Karuga >>>>>> <wambui.karugax@xxxxxxxxx> wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Wed, 18 Mar 2020, Daniel Vetter wrote: >>>>>>> >>>>>>>> On Tue, Mar 10, 2020 at 04:31:14PM +0300, Wambui Karuga wrote: >>>>>>>>> Since 987d65d01356 (drm: debugfs: make >>>>>>>>> drm_debugfs_create_files() never fail), >>>>>>>>> drm_debugfs_create_files() never >>>>>>>>> fails and should return void. Therefore, remove its use as the >>>>>>>>> return value of drm_vram_mm_debugfs_init(), and have the function >>>>>>>>> return 0 directly. >>>>>>>>> >>>>>>>>> v2: have drm_vram_mm_debugfs_init() return 0 instead of void to >>>>>>>>> avoid >>>>>>>>> introducing build issues and build breakage. >>>>>>>>> >>>>>>>>> References: >>>>>>>>> https://lists.freedesktop.org/archives/dri-devel/2020-February/257183.html >>>>>>>>> >>>>>>>>> Signed-off-by: Wambui Karuga <wambui.karugax@xxxxxxxxx> >>>>>>>>> Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx> >>>>>>>>> --- >>>>>>>>> drivers/gpu/drm/drm_gem_vram_helper.c | 10 ++++------ >>>>>>>>> 1 file changed, 4 insertions(+), 6 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c >>>>>>>>> b/drivers/gpu/drm/drm_gem_vram_helper.c >>>>>>>>> index 92a11bb42365..c8bcc8609650 100644 >>>>>>>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c >>>>>>>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c >>>>>>>>> @@ -1048,14 +1048,12 @@ static const struct drm_info_list >>>>>>>>> drm_vram_mm_debugfs_list[] = { >>>>>>>>> */ >>>>>>>>> int drm_vram_mm_debugfs_init(struct drm_minor *minor) >>>>>>>>> { >>>>>>>>> - int ret = 0; >>>>>>>>> - >>>>>>>>> #if defined(CONFIG_DEBUG_FS) >>>>>>>> >>>>>>>> Just noticed that this #if here is not needed, we already have a >>>>>>>> dummy >>>>>>>> function for that case. Care to write a quick patch to remove >>>>>>>> it? On top >>>>>>>> of this patch series here ofc, I'm in the processing of merging >>>>>>>> the entire >>>>>>>> pile. >>>>>>>> >>>>>>>> Thanks, Daniel >>>>>>> Hi Daniel, >>>>>>> Without this check here, and compiling without CONFIG_DEBUG_FS, this >>>>>>> function is run and the drm_debugfs_create_files() does not have >>>>>>> access to >>>>>>> the parameters also protected by an #if above this function. So >>>>>>> the change >>>>>>> throws an error for me. Is that correct? >>>>>> >>>>>> Hm right. Other drivers don't #ifdef out their debugfs file functions >>>>>> ... kinda a bit disappointing that we can't do this in the neatest >>>>>> way >>>>>> possible. >>>>>> >>>>>> Greg, has anyone ever suggested to convert the debugfs_create_file >>>>>> function (and similar things) to macros that don't use any of the >>>>>> arguments, and then also annotating all the static >>>>>> functions/tables as >>>>>> __maybe_unused and let the compiler garbage collect everything? >>>>>> Instead of explicit #ifdef in all the drivers ... >>>>> >>>>> No, no one has suggested that, having the functions be static inline >>>>> should make it all "just work" properly if debugfs is not enabled. >>>>> The >>>>> variables will not be used, so the compiler should just optimize them >>>>> away properly. >>>>> >>>>> No checks for CONFIG_DEBUG_FS should be needed anywhere in .c code. >>>> >>>> So the trouble with this one is that the static inline functions for >>>> the debugfs file are wrapped in a #if too, and hence if we drop the >>>> #if around the function call stuff won't compile. Should we drop all >>>> the #if in the .c file and assume the compiler will remove all the >>>> dead code and dead functions? >>> >>> Yes you should :) >>> >>> there should not be any need for #if in a .c file for debugfs stuff. >> >> Wambui, can you pls try that out? I.e. removing all the #if for >> CONFIG_DEBUG_FS from that file. > > Removing them works with CONFIG_DEBUG_FS enabled or disabled. > I can send a patch for that. Please do. Removing explicit checks for CONFIG_ is usually a good thing. Best regards Thomas > > wambui karuga >> -Daniel >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> http://blog.ffwll.ch >> > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
