Re: [PATCH] staging: vc04_services: remove empty functions

[Karolina Stolarek]

On 19.10.2023 13:21, Calvince Otieno wrote:
> On Thu, Oct 19, 2023 at 1:55 PM Karolina Stolarek
> <karolina.stolarek@xxxxxxxxx> wrote:
> > On 19.10.2023 11:46, Calvince Otieno wrote:
> > > The functions vchiq_debugfs_init(), vchiq_debugfs_deinit(),
> > > vchiq_debugfs_add_instance(), and vchiq_debugfs_remove_instance()
> > > are declared and defined but contains no code or statements.
> > > They do nothing.
> > >
> > > Signed-off-by: Calvince Otieno <calvncce@xxxxxxxxx>
> > > ---
> > >    .../interface/vchiq_arm/vchiq_arm.c              |  3 ---
> > >    .../interface/vchiq_arm/vchiq_debugfs.c          | 16 ----------------
> > >    .../interface/vchiq_arm/vchiq_debugfs.h          |  8 --------
> > >    .../interface/vchiq_arm/vchiq_dev.c              |  4 ----
> > >    4 files changed, 31 deletions(-)
> > >
> > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > > index eef9c8c06e66..b802d1ecc8f6 100644
> > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > > @@ -1806,8 +1806,6 @@ static int vchiq_probe(struct platform_device *pdev)
> > >        if (err)
> > >                goto failed_platform_init;
> > >
> > > -     vchiq_debugfs_init();
> > > -
> > >        vchiq_log_info(vchiq_arm_log_level,
> > >                       "vchiq: platform initialised - version %d (min %d)",
> > >                       VCHIQ_VERSION, VCHIQ_VERSION_MIN);
> > > @@ -1838,7 +1836,6 @@ static void vchiq_remove(struct platform_device *pdev)
> > >    {
> > >        vchiq_device_unregister(bcm2835_audio);
> > >        vchiq_device_unregister(bcm2835_camera);
> > > -     vchiq_debugfs_deinit();
> > >        vchiq_deregister_chrdev();
> > >    }
> > >
> > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
> > > index dc667afd1f8c..7e45076e5ebd 100644
> > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
> > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c
> > > @@ -228,20 +228,4 @@ void vchiq_debugfs_deinit(void)
> > >
> > >    #else /* CONFIG_DEBUG_FS */
> > We need to have these definitions, so a kernel with no CONFIG_DEBUG_FS
> > selected builds. Have you experimented with this config with your
> > changes applied?
> >
> > All the best,
> > Karolina
> >
> > > -void vchiq_debugfs_init(void)
> > > -{
> > > -}
> > > -
> > > -void vchiq_debugfs_deinit(void)
> > > -{
> > > -}
> > > -
> > > -void vchiq_debugfs_add_instance(struct vchiq_instance *instance)
> > > -{
> > > -}
> > > -
> > > -void vchiq_debugfs_remove_instance(struct vchiq_instance *instance)
> > > -{
> > > -}
> > > -
> > >    #endif /* CONFIG_DEBUG_FS */
> > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h
> > > index e9bf055a4ca9..76424473c32b 100644
> > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h
> > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.h
> > > @@ -10,12 +10,4 @@ struct vchiq_debugfs_node {
> > >        struct dentry *dentry;
> > >    };
> > >
> > > -void vchiq_debugfs_init(void);
> > > -
> > > -void vchiq_debugfs_deinit(void);
> > > -
> > > -void vchiq_debugfs_add_instance(struct vchiq_instance *instance);
> > > -
> > > -void vchiq_debugfs_remove_instance(struct vchiq_instance *instance);
> > > -
> > >    #endif /* VCHIQ_DEBUGFS_H */
> > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> > > index 841e1a535642..953f39f537a8 100644
> > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c
> > > @@ -1185,8 +1185,6 @@ static int vchiq_open(struct inode *inode, struct file *file)
> > >        instance->state = state;
> > >        instance->pid = current->tgid;
> > >
> > > -     vchiq_debugfs_add_instance(instance);
> > > -
> > >        init_completion(&instance->insert_event);
> > >        init_completion(&instance->remove_event);
> > >        mutex_init(&instance->completion_mutex);
> > > @@ -1297,8 +1295,6 @@ static int vchiq_release(struct inode *inode, struct file *file)
> > >
> > >        free_bulk_waiter(instance);
> > >
> > > -     vchiq_debugfs_remove_instance(instance);
> > > -
> > >        kfree(instance);
> > >        file->private_data = NULL;
>
> Could you kindly clarify what these definitions are doing.
> At the moment they do absolutely nothing, I might be wrong

These functions have different definitions, depending on the value of 
DEBUG_FS[1]. If this symbol is defined, these functions create entries 
in /sys/kernel/debug, which are useful for kernel developers. Please 
mind that these functions are defined twice in that file. For example:

https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c#L206

https://elixir.bootlin.com/linux/latest/source/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_debugfs.c#L231

And like I said before, we have to also define empty functions, so no 
matter if that config option is selected or not, the kernel can be built.

All the best,
Karolina

---------------------------------
[1] - https://elixir.bootlin.com/linux/latest/source/lib/Kconfig.debug#L626