| Message ID | 1612518255-23052-3-git-send-email-yangyicong@hisilicon.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Use subdir-ccflags-* to inherit debug flag | expand |
On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote: > From: Junhao He <hejunhao2@hisilicon.com> > > Use subdir-ccflags-* instead of ccflags-* to inherit the debug > settings from Kconfig when traversing subdirectories. > > Suggested-by: Bjorn Helgaas <bhelgaas@google.com> > Signed-off-by: Junhao He <hejunhao2@hisilicon.com> > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> What problem does this fix ? Maybe I am missing it, but I don't see DEBUG being used in a subdirectory of drivers/hwmon. Guenter > --- > drivers/hwmon/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 09a86c5..1c0c089 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o > obj-$(CONFIG_SENSORS_OCC) += occ/ > obj-$(CONFIG_PMBUS) += pmbus/ > > -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG > +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG > > -- > 2.8.1 >
On Fri, Feb 05, 2021 at 10:28:32AM -0800, Guenter Roeck wrote: > On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote: > > From: Junhao He <hejunhao2@hisilicon.com> > > > > Use subdir-ccflags-* instead of ccflags-* to inherit the debug > > settings from Kconfig when traversing subdirectories. > > > > Suggested-by: Bjorn Helgaas <bhelgaas@google.com> > > Signed-off-by: Junhao He <hejunhao2@hisilicon.com> > > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> > > What problem does this fix ? Maybe I am missing it, but I don't see > DEBUG being used in a subdirectory of drivers/hwmon. It's my fault for raising this question [1]. Yicong fixed a real problem in drivers/pci, where we are currently using ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG so CONFIG_PCI_DEBUG=y turns on debug in drivers/pci, but not in the subdirectories. That's surprising to users. So my question was whether we should default to using subdir-ccflags for -DDEBUG in general, and only use ccflags when we have subdirectories that have their own debug options, e.g., drivers/i2c/Makefile:ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG drivers/i2c/algos/Makefile:ccflags-$(CONFIG_I2C_DEBUG_ALGO) := -DDEBUG drivers/i2c/busses/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG drivers/i2c/muxes/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG I mentioned drivers/hwmon along with a few others that have subdirectories, do not have per-subdirectory debug options, and use ccflags. I didn't try to determine whether those subdirectories currently use -DDEBUG. In the case of drivers/hwmon, several drivers do use pr_debug(), and CONFIG_HWMON_DEBUG_CHIP=y turns those on. But if somebody were to add pr_debug() to drivers/hwmon/occ/common.c, for example, CONFIG_HWMON_DEBUG_CHIP=y would *not* turn it on. That sounds surprising to me, but if that's what you intend, that's totally fine. [1] https://lore.kernel.org/r/20210204161048.GA68790@bjorn-Precision-5520 > > --- > > drivers/hwmon/Makefile | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > > index 09a86c5..1c0c089 100644 > > --- a/drivers/hwmon/Makefile > > +++ b/drivers/hwmon/Makefile > > @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o > > obj-$(CONFIG_SENSORS_OCC) += occ/ > > obj-$(CONFIG_PMBUS) += pmbus/ > > > > -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG > > +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG > > > > -- > > 2.8.1 > >
On 2021/2/6 4:08, Bjorn Helgaas wrote: > On Fri, Feb 05, 2021 at 10:28:32AM -0800, Guenter Roeck wrote: >> On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote: >>> From: Junhao He <hejunhao2@hisilicon.com> >>> >>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug >>> settings from Kconfig when traversing subdirectories. >>> >>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com> >>> Signed-off-by: Junhao He <hejunhao2@hisilicon.com> >>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> >> >> What problem does this fix ? Maybe I am missing it, but I don't see >> DEBUG being used in a subdirectory of drivers/hwmon. > > It's my fault for raising this question [1]. Yicong fixed a real > problem in drivers/pci, where we are currently using > > ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG > > so CONFIG_PCI_DEBUG=y turns on debug in drivers/pci, but not in the > subdirectories. That's surprising to users. > > So my question was whether we should default to using subdir-ccflags > for -DDEBUG in general, and only use ccflags when we have > subdirectories that have their own debug options, e.g., > > drivers/i2c/Makefile:ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG > drivers/i2c/algos/Makefile:ccflags-$(CONFIG_I2C_DEBUG_ALGO) := -DDEBUG > drivers/i2c/busses/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > drivers/i2c/muxes/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > > I mentioned drivers/hwmon along with a few others that have > subdirectories, do not have per-subdirectory debug options, and use > ccflags. I didn't try to determine whether those subdirectories > currently use -DDEBUG. > > In the case of drivers/hwmon, several drivers do use pr_debug(), > and CONFIG_HWMON_DEBUG_CHIP=y turns those on. But if somebody > were to add pr_debug() to drivers/hwmon/occ/common.c, for example, > CONFIG_HWMON_DEBUG_CHIP=y would *not* turn it on. That sounds > surprising to me, but if that's what you intend, that's totally fine. i thought CONFIG_HWMON_DEBUG_CHIP=y means to enable debug including the subdirectories, so use subdir-ccflags-* will make sure the debug message on in the subdirectories, if there will be. please let me know if i understand wrong. Thanks, Yicong > > [1] https://lore.kernel.org/r/20210204161048.GA68790@bjorn-Precision-5520 > >>> --- >>> drivers/hwmon/Makefile | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >>> index 09a86c5..1c0c089 100644 >>> --- a/drivers/hwmon/Makefile >>> +++ b/drivers/hwmon/Makefile >>> @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o >>> obj-$(CONFIG_SENSORS_OCC) += occ/ >>> obj-$(CONFIG_PMBUS) += pmbus/ >>> >>> -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG >>> +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG >>> >>> -- >>> 2.8.1 >>> > > . >
On 2/5/21 12:08 PM, Bjorn Helgaas wrote: > On Fri, Feb 05, 2021 at 10:28:32AM -0800, Guenter Roeck wrote: >> On Fri, Feb 05, 2021 at 05:44:13PM +0800, Yicong Yang wrote: >>> From: Junhao He <hejunhao2@hisilicon.com> >>> >>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug >>> settings from Kconfig when traversing subdirectories. >>> >>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com> >>> Signed-off-by: Junhao He <hejunhao2@hisilicon.com> >>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> >> >> What problem does this fix ? Maybe I am missing it, but I don't see >> DEBUG being used in a subdirectory of drivers/hwmon. > > It's my fault for raising this question [1]. Yicong fixed a real > problem in drivers/pci, where we are currently using > > ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG > > so CONFIG_PCI_DEBUG=y turns on debug in drivers/pci, but not in the > subdirectories. That's surprising to users. > > So my question was whether we should default to using subdir-ccflags > for -DDEBUG in general, and only use ccflags when we have > subdirectories that have their own debug options, e.g., > > drivers/i2c/Makefile:ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG > drivers/i2c/algos/Makefile:ccflags-$(CONFIG_I2C_DEBUG_ALGO) := -DDEBUG > drivers/i2c/busses/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > drivers/i2c/muxes/Makefile:ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG > > I mentioned drivers/hwmon along with a few others that have > subdirectories, do not have per-subdirectory debug options, and use > ccflags. I didn't try to determine whether those subdirectories > currently use -DDEBUG. > > In the case of drivers/hwmon, several drivers do use pr_debug(), > and CONFIG_HWMON_DEBUG_CHIP=y turns those on. But if somebody > were to add pr_debug() to drivers/hwmon/occ/common.c, for example, > CONFIG_HWMON_DEBUG_CHIP=y would *not* turn it on. That sounds > surprising to me, but if that's what you intend, that's totally fine. > That does make sense, but that explanation is missing from the description. Guenter > [1] https://lore.kernel.org/r/20210204161048.GA68790@bjorn-Precision-5520 > >>> --- >>> drivers/hwmon/Makefile | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile >>> index 09a86c5..1c0c089 100644 >>> --- a/drivers/hwmon/Makefile >>> +++ b/drivers/hwmon/Makefile >>> @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o >>> obj-$(CONFIG_SENSORS_OCC) += occ/ >>> obj-$(CONFIG_PMBUS) += pmbus/ >>> >>> -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG >>> +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG >>> >>> -- >>> 2.8.1 >>>
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 09a86c5..1c0c089 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -201,5 +201,5 @@ obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o obj-$(CONFIG_SENSORS_OCC) += occ/ obj-$(CONFIG_PMBUS) += pmbus/ -ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG +subdir-ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG