| Message ID | 20250406134355.49036-1-18255117159@163.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] PCI: tegra194: Fix debugfs directory creation when CONFIG_PCIEASPM is disabled | expand |
Le 06/04/2025 à 15: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-9Onoh4P/yGk@public.gmane.org> > --- > Changes since v1: > https://lore.kernel.org/linux-pci/20250405145459.26800-1-18255117159-9Onoh4P/yGk@public.gmane.org > > - The first version was committed incorrectly because the judgment > parameter in "debugfs_remove_recursive" was not noticed. > --- > drivers/pci/controller/dwc/pcie-tegra194.c | 27 +++++++++++++--------- > 1 file changed, 16 insertions(+), 11 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c > index 5103995cd6c7..f048b2342af4 100644 > --- a/drivers/pci/controller/dwc/pcie-tegra194.c > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c > @@ -711,16 +711,27 @@ static void init_host_aspm(struct tegra_pcie_dw *pcie) > dw_pcie_writel_dbi(pci, PCIE_PORT_AFR, val); > } > > -static void init_debugfs(struct tegra_pcie_dw *pcie) > +static int init_debugfs(struct tegra_pcie_dw *pcie) I would keep it a void function. If devm_kasprintf() fails, which is unlikely, then the module should still work correctly. So just a return; should be fine IMHO. Usually, errors related to debugfs are silently ignored as is does not prevent this to work. CJ > { > - 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 -ENOMEM; > + > + pcie->debugfs = debugfs_create_dir(name, NULL); > + > + debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs, > aspm_state_cnt); > + > + return 0; > } > #else > static inline void disable_aspm_l12(struct tegra_pcie_dw *pcie) { return; } > static inline void disable_aspm_l11(struct tegra_pcie_dw *pcie) { return; } > static inline void init_host_aspm(struct tegra_pcie_dw *pcie) { return; } > -static inline void init_debugfs(struct tegra_pcie_dw *pcie) { return; } > +static inline int init_debugfs(struct tegra_pcie_dw *pcie) { return 0; } > #endif > > static void tegra_pcie_enable_system_interrupts(struct dw_pcie_rp *pp) > @@ -1634,7 +1645,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,14 +1674,9 @@ 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; > + ret = init_debugfs(pcie); > + if (ret < 0) > goto fail_host_init; > - } > - > - pcie->debugfs = debugfs_create_dir(name, NULL); > - init_debugfs(pcie); > > return ret; > > > base-commit: a8662bcd2ff152bfbc751cab20f33053d74d0963
On 2025/4/7 02:10, Christophe JAILLET wrote: > Le 06/04/2025 à 15: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-9Onoh4P/yGk@public.gmane.org> >> --- >> Changes since v1: >> https://lore.kernel.org/linux-pci/20250405145459.26800-1-18255117159-9Onoh4P/yGk@public.gmane.org >> >> - The first version was committed incorrectly because the judgment >> parameter in "debugfs_remove_recursive" was not noticed. >> --- >> drivers/pci/controller/dwc/pcie-tegra194.c | 27 +++++++++++++--------- >> 1 file changed, 16 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c >> b/drivers/pci/controller/dwc/pcie-tegra194.c >> index 5103995cd6c7..f048b2342af4 100644 >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >> @@ -711,16 +711,27 @@ static void init_host_aspm(struct tegra_pcie_dw >> *pcie) >> dw_pcie_writel_dbi(pci, PCIE_PORT_AFR, val); >> } >> -static void init_debugfs(struct tegra_pcie_dw *pcie) >> +static int init_debugfs(struct tegra_pcie_dw *pcie) > > I would keep it a void function. > If devm_kasprintf() fails, which is unlikely, then the module should > still work correctly. So just a return; should be fine IMHO. > > Usually, errors related to debugfs are silently ignored as is does not > prevent this to work. > Hi Christophe, Thanks your for reply. Will change. Best regards, Hans > CJ > >> { >> - 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 -ENOMEM; >> + >> + pcie->debugfs = debugfs_create_dir(name, NULL); >> + >> + debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs, >> aspm_state_cnt); >> + >> + return 0; >> } >> #else >> static inline void disable_aspm_l12(struct tegra_pcie_dw *pcie) { >> return; } >> static inline void disable_aspm_l11(struct tegra_pcie_dw *pcie) { >> return; } >> static inline void init_host_aspm(struct tegra_pcie_dw *pcie) { >> return; } >> -static inline void init_debugfs(struct tegra_pcie_dw *pcie) { return; } >> +static inline int init_debugfs(struct tegra_pcie_dw *pcie) { return 0; } >> #endif >> static void tegra_pcie_enable_system_interrupts(struct dw_pcie_rp *pp) >> @@ -1634,7 +1645,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,14 +1674,9 @@ 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; >> + ret = init_debugfs(pcie); >> + if (ret < 0) >> 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..f048b2342af4 100644 --- a/drivers/pci/controller/dwc/pcie-tegra194.c +++ b/drivers/pci/controller/dwc/pcie-tegra194.c @@ -711,16 +711,27 @@ static void init_host_aspm(struct tegra_pcie_dw *pcie) dw_pcie_writel_dbi(pci, PCIE_PORT_AFR, val); } -static void init_debugfs(struct tegra_pcie_dw *pcie) +static int 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 -ENOMEM; + + pcie->debugfs = debugfs_create_dir(name, NULL); + + debugfs_create_devm_seqfile(dev, "aspm_state_cnt", pcie->debugfs, aspm_state_cnt); + + return 0; } #else static inline void disable_aspm_l12(struct tegra_pcie_dw *pcie) { return; } static inline void disable_aspm_l11(struct tegra_pcie_dw *pcie) { return; } static inline void init_host_aspm(struct tegra_pcie_dw *pcie) { return; } -static inline void init_debugfs(struct tegra_pcie_dw *pcie) { return; } +static inline int init_debugfs(struct tegra_pcie_dw *pcie) { return 0; } #endif static void tegra_pcie_enable_system_interrupts(struct dw_pcie_rp *pp) @@ -1634,7 +1645,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,14 +1674,9 @@ 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; + ret = init_debugfs(pcie); + if (ret < 0) 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 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 | 27 +++++++++++++--------- 1 file changed, 16 insertions(+), 11 deletions(-) base-commit: a8662bcd2ff152bfbc751cab20f33053d74d0963