Few drivers use ccflags-* in their top directory to enable
-DDEBUG, but don't have config options to enable debug
in the sub-directories. They should want the subdirectories
inherit the debug flag from the top.
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 other
drivers may also have this issues. Previous discussion can
be find at
https://lore.kernel.org/linux-pci/[email protected]/
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 <[email protected]>
Use subdir-ccflags-* instead of ccflags-* to inherit the debug
settings from Kconfig when traversing subdirectories.
Suggested-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Junhao He <[email protected]>
Signed-off-by: Yicong Yang <[email protected]>
---
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 <[email protected]>
Use subdir-ccflags-* instead of ccflags-* to inherit the debug
settings from Kconfig when traversing subdirectories.
Suggested-by: Bjorn Helgaas <[email protected]>
Signed-off-by: Junhao He <[email protected]>
Signed-off-by: Yicong Yang <[email protected]>
---
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
On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote:
> From: Junhao He <[email protected]>
>
> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
> settings from Kconfig when traversing subdirectories.
That says what you do, but not _why_ you are doing it.
What does this offer in benefit of the existing way? What is it fixing?
Why do this "churn"?
thanks,
greg k-h
On Fri, Feb 05, 2021 at 05:44:15PM +0800, Yicong Yang wrote:
> From: Junhao He <[email protected]>
>
> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
> settings from Kconfig when traversing subdirectories.
Again, explain _why_.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/SubmittingPatches for what a proper changelog
should look like.
thanks,
greg k-h
On Fri, Feb 5, 2021 at 6:54 PM Greg KH <[email protected]> wrote:
>
> On Fri, Feb 05, 2021 at 05:44:15PM +0800, Yicong Yang wrote:
> > From: Junhao He <[email protected]>
> >
> > Use subdir-ccflags-* instead of ccflags-* to inherit the debug
> > settings from Kconfig when traversing subdirectories.
>
> Again, explain _why_.
>
> Please read the section entitled "The canonical patch format" in the
> kernel file, Documentation/SubmittingPatches for what a proper changelog
> should look like.
>
> thanks,
>
> greg k-h
I think this is a good clean-up,
assuming CONFIG_COMEDI_DEBUG intends to
give the DEBUG flag to all source files
under drivers/staging/comedi/.
--
Best Regards
Masahiro Yamada
Hi Greg,
On 2021/2/5 17:53, Greg KH wrote:
> On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote:
>> From: Junhao He <[email protected]>
>>
>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
>> settings from Kconfig when traversing subdirectories.
>
> That says what you do, but not _why_ you are doing it.
i'll illustrate the reason and reword the commit.
>
> What does this offer in benefit of the existing way? What is it fixing?
> Why do this "churn"?
currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the Makefile
of driver/base and driver/base/power, but not in the subdirectory
driver/base/firmware_loader. we cannot turn the debug on for subdirectory
firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
for the it.
as there is no other debug config in the subdirectory of driver/base,
CONFIG_DEBUG_DRIVER may also mean turn on the debug in the subdirectories, so use
subdir-ccflags-* instead for the -DDEBUG will allow us to enable debug for all
the subdirectories.
Thanks,
Yicong
>
> thanks,
>
> greg k-h
>
> .
>
On Mon, Feb 08, 2021 at 06:44:52PM +0800, Yicong Yang wrote:
> Hi Greg,
>
> On 2021/2/5 17:53, Greg KH wrote:
> > On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote:
> >> From: Junhao He <[email protected]>
> >>
> >> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
> >> settings from Kconfig when traversing subdirectories.
> >
> > That says what you do, but not _why_ you are doing it.
>
> i'll illustrate the reason and reword the commit.
>
> >
> > What does this offer in benefit of the existing way? What is it fixing?
> > Why do this "churn"?
>
> currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the Makefile
> of driver/base and driver/base/power, but not in the subdirectory
> driver/base/firmware_loader. we cannot turn the debug on for subdirectory
> firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
> for the it.
Is that necessary? Does that directory need it?
thanks,
greg k-h
On 2021/2/8 18:47, Greg KH wrote:
> On Mon, Feb 08, 2021 at 06:44:52PM +0800, Yicong Yang wrote:
>> Hi Greg,
>>
>> On 2021/2/5 17:53, Greg KH wrote:
>>> On Fri, Feb 05, 2021 at 05:44:12PM +0800, Yicong Yang wrote:
>>>> From: Junhao He <[email protected]>
>>>>
>>>> Use subdir-ccflags-* instead of ccflags-* to inherit the debug
>>>> settings from Kconfig when traversing subdirectories.
>>>
>>> That says what you do, but not _why_ you are doing it.
>>
>> i'll illustrate the reason and reword the commit.
>>
>>>
>>> What does this offer in benefit of the existing way? What is it fixing?
>>> Why do this "churn"?
>>
>> currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the Makefile
>> of driver/base and driver/base/power, but not in the subdirectory
>> driver/base/firmware_loader. we cannot turn the debug on for subdirectory
>> firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
>> for the it.
>
> Is that necessary? Does that directory need it?
there are several debug prints in firmware_loader/main.c:
./main.c:207: pr_debug("%s: fw-%s fw_priv=%p\n", __func__, fw_name, fw_priv);
./main.c:245: pr_debug("batched request - sharing the same struct fw_priv and lookup for multiple requests\n");
./main.c:269: pr_debug("%s: fw-%s fw_priv=%p data=%p size=%u\n",
./main.c:594: pr_debug("%s: fw-%s fw_priv=%p data=%p size=%u\n",
./main.c:605: pr_debug("%s: fw_name-%s devm-%p released\n",
./main.c:1175: pr_debug("%s: %s\n", __func__, fw_name);
./main.c:1181: pr_debug("%s: %s ret=%d\n", __func__, fw_name, ret);
./main.c:1214: pr_debug("%s: %s\n", __func__, fw_name);
./main.c:1272: pr_debug("%s: fw: %s\n", __func__, name);
./main.c:1389: pr_debug("%s\n", __func__);
./main.c:1415: pr_debug("%s\n", __func__);
>
> thanks,
>
> greg k-h
>
> .
>
On Mon, Feb 08, 2021 at 09:09:20PM +0800, Yicong Yang wrote:
> On 2021/2/8 18:47, Greg KH wrote:
> > On Mon, Feb 08, 2021 at 06:44:52PM +0800, Yicong Yang wrote:
> >> On 2021/2/5 17:53, Greg KH wrote:
> >>> What does this offer in benefit of the existing way? What is it fixing?
> >>> Why do this "churn"?
> >>
> >> currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the Makefile
> >> of driver/base and driver/base/power, but not in the subdirectory
> >> driver/base/firmware_loader. we cannot turn the debug on for subdirectory
> >> firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
> >> for the it.
> >
> > Is that necessary? Does that directory need it?
>
> there are several debug prints in firmware_loader/main.c:
>
> ./main.c:207: pr_debug("%s: fw-%s fw_priv=%p\n", __func__, fw_name, fw_priv);
> ./main.c:245: pr_debug("batched request - sharing the same struct fw_priv and lookup for multiple requests\n");
> <snip>
Even if these are not in scope for CONFIG_DEBUG_DRVIER there is a
config option that would allow you to observe them without changing
any code (CONFIG_DYNAMIC_DEBUG).
Daniel.
On 2021/2/10 19:42, Daniel Thompson wrote:
> On Mon, Feb 08, 2021 at 09:09:20PM +0800, Yicong Yang wrote:
>> On 2021/2/8 18:47, Greg KH wrote:
>>> On Mon, Feb 08, 2021 at 06:44:52PM +0800, Yicong Yang wrote:
>>>> On 2021/2/5 17:53, Greg KH wrote:
>>>>> What does this offer in benefit of the existing way? What is it fixing?
>>>>> Why do this "churn"?
>>>>
>>>> currently we have added ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG in the Makefile
>>>> of driver/base and driver/base/power, but not in the subdirectory
>>>> driver/base/firmware_loader. we cannot turn the debug on for subdirectory
>>>> firmware_loader if we config DEBUG_DRIVER and there is no kconfig option
>>>> for the it.
>>>
>>> Is that necessary? Does that directory need it?
>>
>> there are several debug prints in firmware_loader/main.c:
>>
>> ./main.c:207: pr_debug("%s: fw-%s fw_priv=%p\n", __func__, fw_name, fw_priv);
>> ./main.c:245: pr_debug("batched request - sharing the same struct fw_priv and lookup for multiple requests\n");
>> <snip>
>
> Even if these are not in scope for CONFIG_DEBUG_DRVIER there is a
> config option that would allow you to observe them without changing
> any code (CONFIG_DYNAMIC_DEBUG).
>
yes. they're two mechanisms of debug. i think it's the right thing to make
both work properly.
>
> Daniel.
>
> .
>