| Message ID | 1439326077-25507-2-git-send-email-lho@apm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 11, 2015 at 02:47:54PM -0600, Loc Ho wrote: > This patch adds an function to retrieve the EDAC debugfs node. This allows > EDAC driver to create debugfs node under the EDAC debugfs node. > > Signed-off-by: Loc Ho <lho@apm.com> > --- > drivers/edac/edac_core.h | 1 + > drivers/edac/edac_mc_sysfs.c | 10 ++++++++++ > 2 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h > index ad42587..6771e64 100644 > --- a/drivers/edac/edac_core.h > +++ b/drivers/edac/edac_core.h > @@ -511,5 +511,6 @@ extern void edac_pci_remove_sysfs(struct edac_pci_ctl_info *pci); > * edac misc APIs > */ > extern char *edac_op_state_to_string(int op_state); > +extern struct dentry *edac_get_debugfs(void); > > #endif /* _EDAC_CORE_H_ */ > diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c > index 33df7d9..b76afe0 100644 > --- a/drivers/edac/edac_mc_sysfs.c > +++ b/drivers/edac/edac_mc_sysfs.c > @@ -959,6 +959,16 @@ nomem: > debugfs_remove(mci->debugfs); > return -ENOMEM; > } > + > +struct dentry *edac_get_debugfs(void) > +{ > + return edac_debugfs; > +} EXPORT_SYMBOL_GPL() if it is going to be used by modules. > +#else > +struct dentry *edac_get_debugfs(void) static inline struct ... > +{ > + return NULL; > +} > #endif > > /* > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-edac" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Wed, Aug 12, 2015 at 12:12:19PM +0200, Borislav Petkov wrote:
> EXPORT_SYMBOL_GPL() if it is going to be used by modules.
Better yet, move it to the header.
Hi Borislav, >> EXPORT_SYMBOL_GPL() if it is going to be used by modules. > > Better yet, move it to the header. I can't move it completely to the header and make it static. It will generate a lot of compiler warning as it isn't used by other modules. I will add EXPORT_SYMBOL_GPL() in the next version. -Loc
On Wed, Aug 12, 2015 at 10:23:11AM -0700, Loc Ho wrote: > I can't move it completely to the header and make it static. It will > generate a lot of compiler warning as it isn't used by other modules. Come again? I'm talking about static inline functions in headers. Grep the kernel sources for examples.
Hi, > >>> EXPORT_SYMBOL_GPL() if it is going to be used by modules. >> >> Better yet, move it to the header. > > I can't move it completely to the header and make it static. It will > generate a lot of compiler warning as it isn't used by other modules. > I will add EXPORT_SYMBOL_GPL() in the next version. Okay... The 'inline' helps with the compiler warning. If I move all to header file with 'static inline' and wrapper around CONFIG_EDAC_DEBUG, don't I need to export and declare the variable 'edac_debugfs'? Are you okay with export this variable? Or, it is static inline if not defined CONFIG_EDAC_DEBUG. Otherwise, just an extern header function and export it as well? -Loc
On Wed, Aug 12, 2015 at 09:30:58PM -0700, Loc Ho wrote: > Okay... The 'inline' helps with the compiler warning. If I move all to > header file with 'static inline' and wrapper around CONFIG_EDAC_DEBUG, > don't I need to export and declare the variable 'edac_debugfs'? Are > you okay with export this variable? You need to make edac_debugfs extern in the header and define it in edac_mc_sysfs.c. But that's ok because edac_core.h will be included only by EDAC drivers and they're supposed to see edac_debugfs anyway... Thanks.
Hi, > On Wed, Aug 12, 2015 at 09:30:58PM -0700, Loc Ho wrote: > > Okay... The 'inline' helps with the compiler warning. If I move all to > > header file with 'static inline' and wrapper around CONFIG_EDAC_DEBUG, > > don't I need to export and declare the variable 'edac_debugfs'? Are > > you okay with export this variable? > > You need to make edac_debugfs extern in the header and define it in > edac_mc_sysfs.c. > > But that's ok because edac_core.h will be included only by EDAC drivers > and they're supposed to see edac_debugfs anyway... > What's the point in having an accessor function that returns a driver internal variable when this variable is exported anyway? This will only further inconsistencies because half of the users will use the variable directly and the other half will use the accessor function. Lothar Waßmann
On Thu, Aug 13, 2015 at 10:41:50AM +0200, Lothar Waßmann wrote: > What's the point in having an accessor function that returns a driver > internal variable when this variable is exported anyway? Not driver-internal but EDAC-core internal. > This will only further inconsistencies because half of the users will > use the variable directly and the other half will use the accessor > function. The idea is that it is not visible outside of EDAC. The EDAC drivers themselves should see it. And yes, I admit that the accessors are maybe redundant then since EDAC drivers can use edac_debugfs ptr directly. Hmm, and that's ok too because my other concern about future refactoring is taken care of too because as long as all users of edac_debugfs are confined to drivers/edac/, changing stuff there is easy. Ok, I'm convinced, Loc, feel free to drop the accessors in your next version. Thanks.
diff --git a/drivers/edac/edac_core.h b/drivers/edac/edac_core.h index ad42587..6771e64 100644 --- a/drivers/edac/edac_core.h +++ b/drivers/edac/edac_core.h @@ -511,5 +511,6 @@ extern void edac_pci_remove_sysfs(struct edac_pci_ctl_info *pci); * edac misc APIs */ extern char *edac_op_state_to_string(int op_state); +extern struct dentry *edac_get_debugfs(void); #endif /* _EDAC_CORE_H_ */ diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c index 33df7d9..b76afe0 100644 --- a/drivers/edac/edac_mc_sysfs.c +++ b/drivers/edac/edac_mc_sysfs.c @@ -959,6 +959,16 @@ nomem: debugfs_remove(mci->debugfs); return -ENOMEM; } + +struct dentry *edac_get_debugfs(void) +{ + return edac_debugfs; +} +#else +struct dentry *edac_get_debugfs(void) +{ + return NULL; +} #endif /*
This patch adds an function to retrieve the EDAC debugfs node. This allows EDAC driver to create debugfs node under the EDAC debugfs node. Signed-off-by: Loc Ho <lho@apm.com> --- drivers/edac/edac_core.h | 1 + drivers/edac/edac_mc_sysfs.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 0 deletions(-)