Re: [PATCH 1/3] soc/tegra: pmc: Use debugfs_initialized()

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3348 bytes --]

On Tue, Jun 06, 2023 at 04:36:06PM +0100, Jon Hunter wrote:
> From: Bharat Nihalani <bnihalani@nvidia.com>
> 
> The kernel command line parameter debugfs=off can be used to dynamically
> disable debugfs support at boot time. However, the Tegra PMC driver will
> always attempt to register debugfs entries if CONFIG_DEBUG_FS is
> enabled. Therefore, if CONFIG_DEBUG_FS is enabled but the user sets
> debugfs=off, then probing the PMC driver will fail.
> 
> Fix this by using the function debugfs_initialized() to check if debugfs
> support is enabled before calling any debugfs functions in the Tegra PMC
> driver. Note that if CONFIG_DEBUG_FS is not defined
> debugfs_initialized() will return false.
> 
> Signed-off-by: Bharat Nihalani <bnihalani@nvidia.com>
> Signed-off-by: Kartik <kkartik@nvidia.com>
> Co-developed-by: Jon Hunter <jonathanh@nvidia.com>
> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
> ---
>  drivers/soc/tegra/pmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 5d17799524c9..12e852a8a609 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -3026,7 +3026,7 @@ static int tegra_pmc_probe(struct platform_device *pdev)
>  
>  	tegra_pmc_reset_sysfs_init(pmc);
>  
> -	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
> +	if (debugfs_initialized()) {
>  		err = tegra_powergate_debugfs_init();
>  		if (err < 0)
>  			goto cleanup_sysfs;

Judging by other patches I've seen sent over the last few years, I think
the more idiomatic fix would be to just ignore the error returns from
debugfs calls. I think in this particular case we can probably just make
tegra_powergate_debugfs_init() return void. Any subsequent calls using
the pmc->debugfs pointer will ignore errors or NULL.

Can you test whether something like the below fixes the problem you were
seeing as well?

--- >8 ---
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 5d17799524c9..16992ddd6e04 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1190,14 +1190,10 @@ static int powergate_show(struct seq_file *s, void *data)
 
 DEFINE_SHOW_ATTRIBUTE(powergate);
 
-static int tegra_powergate_debugfs_init(void)
+static void tegra_powergate_debugfs_init(void)
 {
 	pmc->debugfs = debugfs_create_file("powergate", S_IRUGO, NULL, NULL,
 					   &powergate_fops);
-	if (!pmc->debugfs)
-		return -ENOMEM;
-
-	return 0;
 }
 
 static int tegra_powergate_of_get_clks(struct tegra_powergate *pg,
@@ -3026,11 +3022,8 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	tegra_pmc_reset_sysfs_init(pmc);
 
-	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
-		err = tegra_powergate_debugfs_init();
-		if (err < 0)
-			goto cleanup_sysfs;
-	}
+	if (IS_ENABLED(CONFIG_DEBUG_FS))
+		tegra_powergate_debugfs_init();
 
 	err = tegra_pmc_pinctrl_init(pmc);
 	if (err)
@@ -3067,7 +3060,6 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 	tegra_powergate_remove_all(pdev->dev.of_node);
 cleanup_debugfs:
 	debugfs_remove(pmc->debugfs);
-cleanup_sysfs:
 	device_remove_file(&pdev->dev, &dev_attr_reset_reason);
 	device_remove_file(&pdev->dev, &dev_attr_reset_level);
 	clk_notifier_unregister(pmc->clk, &pmc->clk_nb);
--- >8 ---

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]