Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:8 as permitted sender) client-ip=2620:137:e000::3:8;
Message-ID: <15af6841-17f5-2bf0-cf57-597ce3cf9b6f@intel.com>
Date:   Thu, 19 Oct 2023 14:34:53 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] staging: vc04_services: remove empty functions
Content-Language: en-US
To:     Calvince Otieno <calvncce@gmail.com>
CC:     <outreachy@lists.linux.dev>, <gustavo@embeddedor.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Florian Fainelli <florian.fainelli@broadcom.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@broadcom.com>,
        <linux-staging@lists.linux.dev>, <linux-kernel@vger.kernel.org>
References: <ZTD677iqMkRPxT27@lab-ubuntu>
 <93a6d8b0-140c-983e-1b78-6f5a4e03e62e@intel.com>
 <CADFX3OTHX=5jNm8aaUyC_RzDPntiL4ZC=Jj+8bZPjHNmZwUs9A@mail.gmail.com>
From:   Karolina Stolarek <karolina.stolarek@intel.com>
Organization: Intel Technology Poland sp. z o.o. - ul. Slowackiego 173, 80-298
 Gdansk - KRS 101882 - NIP 957-07-52-316
In-Reply-To: <CADFX3OTHX=5jNm8aaUyC_RzDPntiL4ZC=Jj+8bZPjHNmZwUs9A@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UXZnSTVPVnoxZkgvcThlaVl5TENoRDREOE5QelZjdVVPd0VDK3lQUEE1ZlVI?=
 =?utf-8?B?YndudHd2L0s4L3JiV1djS1JteEVGY1MrQlJFRWFlT3YySzdQSEsvUnUyUDI2?=
 =?utf-8?B?MTkrRkt4cHE2VHFFYzZzWHVxeCtWYUFMaHJDNVR6Rkp2MkVYd08yTHoyRG04?=
 =?utf-8?B?NVAvT21IdmZodDVTQUMzRTBtM1RKK2JXc3ladTE0Vk00eUE1UmluTDZYWnNS?=
 =?utf-8?B?VWFhY0pRUmdTVERnQ3ZPbUN1S2FZRExVeTh4UXB1TmgxK3VINFh3aUhRKys1?=
 =?utf-8?B?S0c5ckVCeXN2eTZnYy9ESVZWWG5yaTJ5SHVKdUVDK1NYbGFIdjFWUFFGSjVq?=
 =?utf-8?B?SFFtK2NIZEptTjlZcG9XM0lYVjExL3k3RHAxWEhRQTU5K0RzdUFJTDFYRjBa?=
 =?utf-8?B?QXVxdXpnUkRQaXZVdERuTFhCSVNzMjd4TGFRZHd0VmZuakFhUVhSRFFyUExr?=
 =?utf-8?B?QlZ2R0NiNmpYcURIdi84L0tRU21KUFNIZW9aVjRzSWVHSFM2cXAvc0ZGZGxz?=
 =?utf-8?B?MG5aMkxwamJpR0pwa21oajVIdUFQWEVRd21FbVd5UHViejZaZXRsNzdHcUJK?=
 =?utf-8?B?UDVWSS9EV1NGektoN2VHSlBjMC9UTEVhV2lLVUhzRG5CVVgzWTkwMDI1b00z?=
 =?utf-8?B?dmJLUUNjdm1JUnFFdm5SUEE1eTc5K1lCVHdIYTN6anVaVWE1N2dnZUZxa1RX?=
 =?utf-8?B?SlZQUjVEOSt6eHVLZmw5aGI1OWtzZnNPd2g1TG50L01uTEp1bWlZNFRsVWk0?=
 =?utf-8?B?QVR4cGVDWDBBa1FkQTAvTCtiK0gxR3VFT2tnQUFEa2tJTWREU2lkZkRpMHRD?=
 =?utf-8?B?dk9DOGlERm1rcWx4aWhBdlpWMjhQQ1VrYUpJVFZRcTFPRUxnWWtoUlJoZmE0?=
 =?utf-8?B?dG10dTRCVmU1VVB0Ymc3QldXcWFHWG5TRzhJZUZ5WERZbHArcTY0bE5xRnha?=
 =?utf-8?B?VThVckdnVk1ob3BVdzZKcCtZOVZqMjFDRkVlTDZzM1kyajBTMm14cE40K014?=
 =?utf-8?B?bVFkT1lET05sTm0xSUIvV0ZxZ1RCanNsc1oxS0UxZVVpdElwd2s1RUU3OFZU?=
 =?utf-8?B?Ykpob0xQbTUvYnNoWFVXZlAzL3Raeld3NnU5cVdET1ZuUW5ZWWNpTnY2dTFI?=
 =?utf-8?B?elZEcldsWlUwWGg1U0JYdE1zYlpYWExpdmx3dTNCek5CRy81N3lTZzBKd0Z0?=
 =?utf-8?B?OTFmZ0dCM2svTGVoUHRCK1E4NDRRVCtLWUU3SlhobTRhN2ZwTGE5QnNlVTY4?=
 =?utf-8?B?MHUwdXpqK1Fzdm56MzVvRXVMcWpSWkF4d1pTMnJsVjV5eFFYM2NKYU8wakNy?=
 =?utf-8?B?Rzg0RFh3TEwwMjNqZGx0OHAwZ2ZVdzhBbG95N0hkVXJDRnJvUWxqU3FNeDc4?=
 =?utf-8?B?eUFwYVRFU1ljU01hQmhMays0cjkzS0owRDBkRDMyVERZNkRoWmVRVGNSeVZY?=
 =?utf-8?B?K3lDbSs2V2VpWjd2L1QvVG93bi9oUHlNZFVtWWZ2NFJKWDR3cFRCeCtYeUpL?=
 =?utf-8?B?OGVOa1hLQjM5OVoyT213L1poSHZWYXZGVU44VStyQm9VUlFCd3FaOGo3S1NR?=
 =?utf-8?B?RlQwVkZSY1prQUFLWWw4WGJKMWRvbUFrVE4xdzdteFZNWDdhbWh4cVI0bUlm?=
 =?utf-8?B?dERyZDR4MXBhUGtDYkprTHVRTW44RzRibDNiMEJ3RjJydFdqZ1dtdHFBcmxG?=
 =?utf-8?B?eS9LemFJSVVWL2s4TXJkUFg3dXYxV3ZPTGZ4RGx5N1BCYzNvRWtwQUd4anFU?=
 =?utf-8?B?RjFCejBEQU9iRlpUT2pyVUNRQW9ETXpBV3JFMGNaVXdlK0NqdXk1V3ViemUx?=
 =?utf-8?B?NHhBTXY5MTh6dEJQMWhIL3Fyc1duem90M2VJM09uY0RkcHI4UE9YT0w5ZVZw?=
 =?utf-8?B?SkxnVjVGVitBMEFCdnBiNTZEVGdseWRsaW40dlFVcnFaOVBOVnZhV25aS3Iv?=
 =?utf-8?B?aUNwcFVlOGVTbkJhQjlrbXdBRVRBNlBUYjZ0NGdTbGpNQWp4YVVmWkRjVTFi?=
 =?utf-8?B?cnQ3d281c0xyZVJ6Y0huNW0xbVA3d3pUMll6OGFscDM3d3FDTjA2eGI3L2JT?=
 =?utf-8?B?RzNkTGtuR25jOXpQY0pkOTV6aGpxeTE0YlFiR2FrMmFIaENOVEFLL0tUWnRk?=
 =?utf-8?B?ZEFUcVNEWVRTV2t6dlNLam9Cd3ovRFV2YitzREFkQXJiZktIZEpMTnhaYno0?=
 =?utf-8?B?cU85NG9nam1xMHoxcmowS1ArQmthcjlkSERGdFJkbmZrMFphQU1QeW53b0RW?=
 =?utf-8?B?Y01aMldxa3JiZnR4N1pzaFRPb1VBPT0=?=
X-MS-Exchange-CrossTenant-Network-Message-Id: dc151d4e-0e28-4d84-05ad-08dbd09fd111
X-MS-Exchange-CrossTenant-AuthSource: BY5PR11MB4243.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Oct 2023 12:35:03.6821
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: OEQgOH/26ooNJ3nBmS95kGXT+9E+rrbK5FgOXzSQHbGyiXpQFJfT7PvDuRhkMuVFO0XZ54NPdzsJ7pOLqDn6lKodogOOz5iE6LqFpAd/nvk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR11MB6275
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.2 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Thu, 19 Oct 2023 05:35:32 -0700 (PDT)

On 19.10.2023 13:21, Calvince Otieno wrote:
> On Thu, Oct 19, 2023 at 1:55 PM Karolina Stolarek
> <karolina.stolarek@intel.com> 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@gmail.com>
>>> ---
>>>    .../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