[PATCH v2 0/4] Use subdir-ccflags-* to inherit debug flag

[PATCH v2 0/4] Use subdir-ccflags-* to inherit debug flag

[Yicong Yang]

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-yangyicong@hisilicon.com/

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@hisilicon.com/

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

[PATCH v2 1/4] driver core: Use subdir-ccflags-* to inherit debug flag

[Yicong Yang]

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

[PATCH v2 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

[Yicong Yang]

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

[PATCH v2 3/4] pps: Use subdir-ccflags-* to inherit debug flag

[Yicong Yang]

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

[PATCH v2 4/4] staging: comedi: Use subdir-ccflags-* to inherit debug flag

[Yicong Yang]

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

Re: [PATCH v2 3/4] pps: Use subdir-ccflags-* to inherit debug flag

[Rodolfo Giometti]

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

Re: [PATCH v2 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

[Guenter Roeck]

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
> 

Re: [PATCH v2 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag

[Yicong Yang]

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
>>
> 
> .
>