[PATCH v2 0/4] Use subdir-ccflags-* to inherit debug flag
Few drivers use ccflags-* in their top directory to enable -DDEBUG, but don't have config options to enable debug in the sub-directories, or they use per subdirectory ccflags-* to have DEBUG with the same kconfig option. Considering they intends to enable debug for all the files under the directory with the same kconfig option, it will be clearer to use subdir-ccflags-* instead of ccflags-* to inherit the debug settings from Kconfig when traversing subdirectories. We primarily find this issue when debugging PCIe and thought other drivers may also have this issues. Previous discussion can be find at https://lore.kernel.org/linux-pci/1612438215-33105-1-git-send-email-yangyico... Change since v1: - reword the commits to illustrate the reasons of the change and the benefits. v1: https://lore.kernel.org/lkml/1612518255-23052-1-git-send-email-yangyicong@hi... Junhao He (4): driver core: Use subdir-ccflags-* to inherit debug flag hwmon: Use subdir-ccflags-* to inherit debug flag pps: Use subdir-ccflags-* to inherit debug flag staging: comedi: Use subdir-ccflags-* to inherit debug flag drivers/base/Makefile | 2 +- drivers/base/power/Makefile | 2 -- drivers/hwmon/Makefile | 2 +- drivers/pps/Makefile | 2 +- drivers/staging/comedi/Makefile | 2 +- drivers/staging/comedi/drivers/Makefile | 1 - drivers/staging/comedi/drivers/tests/Makefile | 2 -- drivers/staging/comedi/kcomedilib/Makefile | 2 -- 8 files changed, 4 insertions(+), 11 deletions(-) -- 2.8.1
From: Junhao He <hejunhao2@hisilicon.com> Currently we can turn on the debug message in the top directory driver/base and subdirectory driver/base/power with kconfig option CONFIG_DEBUG_DRIVER. But the DEBUG flags will not pass to subdirectory drvier/base/firmware_loader which the ccflags-$(CONFIG_DEBUG_DRIVER) is missing and there is no kconfig option to turn on the debug message for it. Use subdir-ccflags-* for the DEBUG flag in the top directory will fix this. Considering CONFIG_DEBUG_DRIVER intends to turn on the debug recursively, use subdir-cclags-* will be clearer and avoid omittance of DEBUG define in the subdirectory. Suggested-by: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/base/Makefile | 2 +- drivers/base/power/Makefile | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 5e7bf96..c6bdf19 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -27,5 +27,5 @@ obj-$(CONFIG_GENERIC_ARCH_TOPOLOGY) += arch_topology.o obj-y += test/ -ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG +subdir-ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile index 8fdd007..2990167 100644 --- a/drivers/base/power/Makefile +++ b/drivers/base/power/Makefile @@ -5,5 +5,3 @@ obj-$(CONFIG_PM_TRACE_RTC) += trace.o obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o domain_governor.o obj-$(CONFIG_HAVE_CLK) += clock_ops.o obj-$(CONFIG_PM_QOS_KUNIT_TEST) += qos-test.o - -ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG -- 2.8.1
From: Junhao He <hejunhao2@hisilicon.com> We use ccflags-$(CONFIG_HWMON_DEBUG_CHIP) for the debug message in drivers/hwmon, but the DEBUG flag will not pass to the subdirectory. Considering CONFIG_HWMON_DEBUG_CHIP intends to have DEBUG recursively in driver/hwmon. It will be clearer to use subdir-ccflags-* instead of ccflags-* to inherit the debug settings from Kconfig when traversing subdirectories, and it will avoid omittance of DEBUG define when debug messages added in the subdirectories. Suggested-by: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- 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 Tue, Feb 09, 2021 at 07:08:17PM +0800, Yicong Yang wrote:
From: Junhao He <hejunhao2@hisilicon.com>
We use ccflags-$(CONFIG_HWMON_DEBUG_CHIP) for the debug message in drivers/hwmon, but the DEBUG flag will not pass to the subdirectory.
Considering CONFIG_HWMON_DEBUG_CHIP intends to have DEBUG recursively in driver/hwmon. It will be clearer to use subdir-ccflags-* instead of ccflags-* to inherit the debug settings from Kconfig when traversing subdirectories, and it will avoid omittance of DEBUG define when debug messages added in the subdirectories.
The above paragraph doesn't add clarity and may as well be dropped. On the other side, the commit message still doesn't mention that pr_debug depends on DEBUG, which I am sure many people don't know or remember. This is the prime reason why this patch is acceptable, so it most definitely needs to be mentioned here. Guenter
Suggested-by: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- 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/9 23:06, Guenter Roeck wrote:
On Tue, Feb 09, 2021 at 07:08:17PM +0800, Yicong Yang wrote:
From: Junhao He <hejunhao2@hisilicon.com>
We use ccflags-$(CONFIG_HWMON_DEBUG_CHIP) for the debug message in drivers/hwmon, but the DEBUG flag will not pass to the subdirectory.
Considering CONFIG_HWMON_DEBUG_CHIP intends to have DEBUG recursively in driver/hwmon. It will be clearer to use subdir-ccflags-* instead of ccflags-* to inherit the debug settings from Kconfig when traversing subdirectories, and it will avoid omittance of DEBUG define when debug messages added in the subdirectories.
The above paragraph doesn't add clarity and may as well be dropped. On the other side, the commit message still doesn't mention that pr_debug depends on DEBUG, which I am sure many people don't know or remember. This is the prime reason why this patch is acceptable, so it most definitely needs to be mentioned here.
sorry, i didn't realize that you mean this. will impove this in the next version after the lunar new year holiday over. Thanks, Yicong
Guenter
Suggested-by: Bjorn Helgaas <bhelgaas@google.com> Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- 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
.
From: Junhao He <hejunhao2@hisilicon.com> We use ccflags-$(CONFIG_PPS_DEBUG) for the debug message in drivers/pps, but the DEBUG flag will not pass to the subdirectory. Considering CONFIG_HWMON_DEBUG_CHIP intends to turn on debug recursively under driver/pps, so it will be clearer to 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> --- drivers/pps/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile index ceaf65c..7a2d3f7 100644 --- a/drivers/pps/Makefile +++ b/drivers/pps/Makefile @@ -8,4 +8,4 @@ pps_core-$(CONFIG_NTP_PPS) += kc.o obj-$(CONFIG_PPS) := pps_core.o obj-y += clients/ generators/ -ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG -- 2.8.1
On 09/02/21 12:08, Yicong Yang wrote:
From: Junhao He <hejunhao2@hisilicon.com>
We use ccflags-$(CONFIG_PPS_DEBUG) for the debug message in drivers/pps, but the DEBUG flag will not pass to the subdirectory.
Considering CONFIG_HWMON_DEBUG_CHIP intends to turn on debug recursively under driver/pps, so it will be clearer to 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> --- drivers/pps/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pps/Makefile b/drivers/pps/Makefile index ceaf65c..7a2d3f7 100644 --- a/drivers/pps/Makefile +++ b/drivers/pps/Makefile @@ -8,4 +8,4 @@ pps_core-$(CONFIG_NTP_PPS) += kc.o obj-$(CONFIG_PPS) := pps_core.o obj-y += clients/ generators/
-ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG
Acked-by: Rodolfo Giometti <giometti@enneenne.com> -- GNU/Linux Solutions e-mail: giometti@enneenne.com Linux Device Driver giometti@linux.it Embedded Systems phone: +39 349 2432127 UNIX programming skype: rodolfo.giometti
From: Junhao He <hejunhao2@hisilicon.com> As CONFIG_COMEDI_DEBUG intends to have the DEBUG flag recursively under drivers/staging/comedi, use subdir-ccflags-* instead of ccflags-* will make it clearer as the DEBUG flag will be inherited 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> --- drivers/staging/comedi/Makefile | 2 +- drivers/staging/comedi/drivers/Makefile | 1 - drivers/staging/comedi/drivers/tests/Makefile | 2 -- drivers/staging/comedi/kcomedilib/Makefile | 2 -- 4 files changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/staging/comedi/Makefile b/drivers/staging/comedi/Makefile index 072ed83..f51cc14 100644 --- a/drivers/staging/comedi/Makefile +++ b/drivers/staging/comedi/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG comedi-y := comedi_fops.o range.o drivers.o \ comedi_buf.o diff --git a/drivers/staging/comedi/drivers/Makefile b/drivers/staging/comedi/drivers/Makefile index b24ac00..7cafc36 100644 --- a/drivers/staging/comedi/drivers/Makefile +++ b/drivers/staging/comedi/drivers/Makefile @@ -1,7 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 # Makefile for individual comedi drivers # -ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG # Comedi "helper" modules obj-$(CONFIG_COMEDI_8254) += comedi_8254.o diff --git a/drivers/staging/comedi/drivers/tests/Makefile b/drivers/staging/comedi/drivers/tests/Makefile index b5d8e13..44ac13d 100644 --- a/drivers/staging/comedi/drivers/tests/Makefile +++ b/drivers/staging/comedi/drivers/tests/Makefile @@ -1,7 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 # Makefile for comedi drivers unit tests # -ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG - obj-$(CONFIG_COMEDI_TESTS) += example_test.o ni_routes_test.o CFLAGS_ni_routes_test.o := -DDEBUG diff --git a/drivers/staging/comedi/kcomedilib/Makefile b/drivers/staging/comedi/kcomedilib/Makefile index 8031142..9f20318 100644 --- a/drivers/staging/comedi/kcomedilib/Makefile +++ b/drivers/staging/comedi/kcomedilib/Makefile @@ -1,6 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 -ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG - obj-$(CONFIG_COMEDI_KCOMEDILIB) += kcomedilib.o kcomedilib-objs := kcomedilib_main.o -- 2.8.1
participants (3)
-
Guenter Roeck -
Rodolfo Giometti -
Yicong Yang