| Message ID | 20250407124331.69459-1-18255117159@163.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] PCI: tegra194: Fix debugfs directory creation when CONFIG_PCIEASPM is disabled | expand |
Hi, Nitpick: PATCH is missing within the [] in the subject. Le 07/04/2025 à 14:43, Hans Zhang a écrit : > Previously, the debugfs directory was unconditionally created in > tegra_pcie_config_rp() regardless of the CONFIG_PCIEASPM setting. > This led to unnecessary directory creation when ASPM support was disabled. > > Move the debugfs directory creation into init_debugfs() which is > conditionally compiled based on CONFIG_PCIEASPM. This ensures: > - The directory is only created when ASPM-related debugfs entries are > needed. > - Proper error handling for directory creation failures. > - Avoids cluttering debugfs with empty directories when ASPM is disabled. > > Signed-off-by: Hans Zhang <18255117159@163.com> > --- > Changes since v2: > https://lore.kernel.org/linux-pci/20250406134355.49036-1-18255117159@163.com/ > > - Maintainer recommends ignoring the devm_kasprintf return value. The > module should still work correctly. So just a return; Note, that I'm not a maintainer ;-) For what it worth: Reviewed-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> CJ > > Changes since v1: > https://lore.kernel.org/linux-pci/20250405145459.26800-1-18255117159@163.com/ > > - The first version was committed incorrectly because the judgment > parameter in "debugfs_remove_recursive" was not noticed. > --- > drivers/pci/controller/dwc/pcie-tegra194.c | 19 ++++++++++--------- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 5103995cd6c7..bc419688527a 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -713,7 +713,16 @@ static void init_host_aspm(struct tegra_pcie_dw *pcie) > > static void init_debugfs(struct tegra_pcie_dw *pcie) > { > - debugfs_create_devm_seqfile(pcie->dev, "aspm_state_cnt", pcie->debugfs, > + struct device *dev = pcie->dev; > + char *name; > + > + name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node); > + if (!name) > + return; > + > + pcie->debugfs = debugfs_create_dir(name, NULL); > + > + debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs, > aspm_state_cnt); > } > #else > @@ -1634,7 +1643,6 @@ static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie) > static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie) > { > struct device *dev = pcie->dev; > - char *name; > int ret; > > pm_runtime_enable(dev); > @@ -1664,13 +1672,6 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie) > goto fail_host_init; > } > > - name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node); > - if (!name) { > - ret = -ENOMEM; > - goto fail_host_init; > - } > - > - pcie->debugfs = debugfs_create_dir(name, NULL); > init_debugfs(pcie); > > return ret; > > base-commit: a8662bcd2ff152bfbc751cab20f33053d74d0963
On 2025/4/8 03:43, Christophe JAILLET wrote: > Hi, > > Nitpick: PATCH is missing within the [] in the subject. > Hi Christophe, I will pay attention in the future. > > Le 07/04/2025 à 14:43, Hans Zhang a écrit : >> Previously, the debugfs directory was unconditionally created in >> tegra_pcie_config_rp() regardless of the CONFIG_PCIEASPM setting. >> This led to unnecessary directory creation when ASPM support was >> disabled. >> >> Move the debugfs directory creation into init_debugfs() which is >> conditionally compiled based on CONFIG_PCIEASPM. This ensures: >> - The directory is only created when ASPM-related debugfs entries are >> needed. >> - Proper error handling for directory creation failures. >> - Avoids cluttering debugfs with empty directories when ASPM is disabled. >> >> Signed-off-by: Hans Zhang <18255117159@163.com> >> --- >> Changes since v2: >> https://lore.kernel.org/linux-pci/20250406134355.49036-1-18255117159@163.com/ >> >> - Maintainer recommends ignoring the devm_kasprintf return value. The >> module should still work correctly. So just a return; > > Note, that I'm not a maintainer ;-) > Maybe I made a mistake. ;-) No.72 Christophe JAILLET <christophe ! jaillet () wanadoo ! fr> 2104(0.17%) @Hobbyists @French Best regards, Hans > For what it worth: > > Reviewed-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > CJ >> >> Changes since v1: >> https://lore.kernel.org/linux-pci/20250405145459.26800-1-18255117159@163.com/ >> >> - The first version was committed incorrectly because the judgment >> parameter in "debugfs_remove_recursive" was not noticed. >> --- >> drivers/pci/controller/dwc/pcie-tegra194.c | 19 ++++++++++--------- >> 1 file changed, 10 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c >> b/drivers/pci/controller/dwc/pcie-tegra194.c >> index 5103995cd6c7..bc419688527a 100644 >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >> @@ -713,7 +713,16 @@ static void init_host_aspm(struct tegra_pcie_dw >> *pcie) >> static void init_debugfs(struct tegra_pcie_dw *pcie) >> { >> - debugfs_create_devm_seqfile(pcie->dev, "aspm_state_cnt", >> pcie->debugfs, >> + struct device *dev = pcie->dev; >> + char *name; >> + >> + name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node); >> + if (!name) >> + return; >> + >> + pcie->debugfs = debugfs_create_dir(name, NULL); >> + >> + debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs, >> aspm_state_cnt); >> } >> #else >> @@ -1634,7 +1643,6 @@ static void tegra_pcie_deinit_controller(struct >> tegra_pcie_dw *pcie) >> static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie) >> { >> struct device *dev = pcie->dev; >> - char *name; >> int ret; >> pm_runtime_enable(dev); >> @@ -1664,13 +1672,6 @@ static int tegra_pcie_config_rp(struct >> tegra_pcie_dw *pcie) >> goto fail_host_init; >> } >> - name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node); >> - if (!name) { >> - ret = -ENOMEM; >> - goto fail_host_init; >> - } >> - >> - pcie->debugfs = debugfs_create_dir(name, NULL); >> init_debugfs(pcie); >> return ret; >> >> base-commit: a8662bcd2ff152bfbc751cab20f33053d74d0963
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c index 5103995cd6c7..bc419688527a 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -713,7 +713,16 @@ static void init_host_aspm(struct tegra_pcie_dw *pcie) static void init_debugfs(struct tegra_pcie_dw *pcie) { - debugfs_create_devm_seqfile(pcie->dev, "aspm_state_cnt", pcie->debugfs, + struct device *dev = pcie->dev; + char *name; + + name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node); + if (!name) + return; + + pcie->debugfs = debugfs_create_dir(name, NULL); + + debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs, aspm_state_cnt); } #else @@ -1634,7 +1643,6 @@ static void tegra_pcie_deinit_controller(struct tegra_pcie_dw *pcie) static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie) { struct device *dev = pcie->dev; - char *name; int ret; pm_runtime_enable(dev); @@ -1664,13 +1672,6 @@ static int tegra_pcie_config_rp(struct tegra_pcie_dw *pcie) goto fail_host_init; } - name = devm_kasprintf(dev, GFP_KERNEL, "%pOFP", dev->of_node); - if (!name) { - ret = -ENOMEM; - goto fail_host_init; - } - - pcie->debugfs = debugfs_create_dir(name, NULL); init_debugfs(pcie); return ret;
Previously, the debugfs directory was unconditionally created in tegra_pcie_config_rp() regardless of the CONFIG_PCIEASPM setting. This led to unnecessary directory creation when ASPM support was disabled. Move the debugfs directory creation into init_debugfs() which is conditionally compiled based on CONFIG_PCIEASPM. This ensures: - The directory is only created when ASPM-related debugfs entries are needed. - Proper error handling for directory creation failures. - Avoids cluttering debugfs with empty directories when ASPM is disabled. Signed-off-by: Hans Zhang <18255117159@163.com> --- Changes since v2: https://lore.kernel.org/linux-pci/20250406134355.49036-1-18255117159@163.com/ - Maintainer recommends ignoring the devm_kasprintf return value. The module should still work correctly. So just a return; Changes since v1: https://lore.kernel.org/linux-pci/20250405145459.26800-1-18255117159@163.com/ - The first version was committed incorrectly because the judgment parameter in "debugfs_remove_recursive" was not noticed. --- drivers/pci/controller/dwc/pcie-tegra194.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) base-commit: a8662bcd2ff152bfbc751cab20f33053d74d0963