* [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:44 [PATCH 0/4] Use subdir-ccflags-* to inherit debug flag Yicong Yang
@ 2021-02-05 9:44 ` Yicong Yang
2021-02-05 9:53 ` Greg KH
2021-02-05 9:44 ` [PATCH 2/4] hwmon: " Yicong Yang
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Yicong Yang @ 2021-02-05 9:44 UTC (permalink / raw)
To: gregkh, jdelvare, linux, giometti, abbotti, hsweeten, kw,
helgaas, linux-kernel, linux-pm, linux-hwmon, devel,
linux-kbuild, masahiroy, michal.lkml
Cc: prime.zeng, yangyicong, linuxarm
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>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:44 ` [PATCH 1/4] driver core: " Yicong Yang
@ 2021-02-05 9:53 ` Greg KH
2021-02-08 10:44 ` Yicong Yang
0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2021-02-05 9:53 UTC (permalink / raw)
To: Yicong Yang
Cc: jdelvare, linux, giometti, abbotti, hsweeten, kw, helgaas,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, linuxarm, prime.zeng
On Fri, Feb 05, 2021 at 05:44:12PM +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.
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:53 ` Greg KH
@ 2021-02-08 10:44 ` Yicong Yang
2021-02-08 10:47 ` Greg KH
0 siblings, 1 reply; 17+ messages in thread
From: Yicong Yang @ 2021-02-08 10:44 UTC (permalink / raw)
To: Greg KH
Cc: jdelvare, linux, giometti, abbotti, hsweeten, kw, helgaas,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, linuxarm, prime.zeng
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 <hejunhao2@hisilicon.com>
>>
>> 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
>
> .
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
2021-02-08 10:44 ` Yicong Yang
@ 2021-02-08 10:47 ` Greg KH
2021-02-08 13:09 ` Yicong Yang
0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2021-02-08 10:47 UTC (permalink / raw)
To: Yicong Yang
Cc: jdelvare, linux, giometti, abbotti, hsweeten, kw, helgaas,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, linuxarm, prime.zeng
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 <hejunhao2@hisilicon.com>
> >>
> >> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
2021-02-08 10:47 ` Greg KH
@ 2021-02-08 13:09 ` Yicong Yang
2021-02-10 11:42 ` Daniel Thompson
0 siblings, 1 reply; 17+ messages in thread
From: Yicong Yang @ 2021-02-08 13:09 UTC (permalink / raw)
To: Greg KH
Cc: jdelvare, linux, giometti, abbotti, hsweeten, kw, helgaas,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, linuxarm, prime.zeng
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 <hejunhao2@hisilicon.com>
>>>>
>>>> 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
>
> .
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
2021-02-08 13:09 ` Yicong Yang
@ 2021-02-10 11:42 ` Daniel Thompson
2021-02-24 9:56 ` Yicong Yang
0 siblings, 1 reply; 17+ messages in thread
From: Daniel Thompson @ 2021-02-10 11:42 UTC (permalink / raw)
To: Yicong Yang
Cc: Greg KH, jdelvare, linux, giometti, abbotti, hsweeten, kw,
helgaas, linux-kernel, linux-pm, linux-hwmon, devel,
linux-kbuild, masahiroy, michal.lkml, linuxarm, prime.zeng
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.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 1/4] driver core: Use subdir-ccflags-* to inherit debug flag
2021-02-10 11:42 ` Daniel Thompson
@ 2021-02-24 9:56 ` Yicong Yang
0 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2021-02-24 9:56 UTC (permalink / raw)
To: Daniel Thompson
Cc: Greg KH, jdelvare, linux, giometti, abbotti, hsweeten, kw,
helgaas, linux-kernel, linux-pm, linux-hwmon, devel,
linux-kbuild, masahiroy, michal.lkml, linuxarm, prime.zeng
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.
>
> .
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:44 [PATCH 0/4] Use subdir-ccflags-* to inherit debug flag Yicong Yang
2021-02-05 9:44 ` [PATCH 1/4] driver core: " Yicong Yang
@ 2021-02-05 9:44 ` Yicong Yang
2021-02-05 18:28 ` Guenter Roeck
2021-02-05 9:44 ` [PATCH 3/4] pps: " Yicong Yang
2021-02-05 9:44 ` [PATCH 4/4] staging: comedi: " Yicong Yang
3 siblings, 1 reply; 17+ messages in thread
From: Yicong Yang @ 2021-02-05 9:44 UTC (permalink / raw)
To: gregkh, jdelvare, linux, giometti, abbotti, hsweeten, kw,
helgaas, linux-kernel, linux-pm, linux-hwmon, devel,
linux-kbuild, masahiroy, michal.lkml
Cc: prime.zeng, yangyicong, linuxarm
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>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:44 ` [PATCH 2/4] hwmon: " Yicong Yang
@ 2021-02-05 18:28 ` Guenter Roeck
2021-02-05 20:08 ` Bjorn Helgaas
0 siblings, 1 reply; 17+ messages in thread
From: Guenter Roeck @ 2021-02-05 18:28 UTC (permalink / raw)
To: Yicong Yang
Cc: gregkh, jdelvare, giometti, abbotti, hsweeten, kw, helgaas,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, prime.zeng, linuxarm
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag
2021-02-05 18:28 ` Guenter Roeck
@ 2021-02-05 20:08 ` Bjorn Helgaas
2021-02-08 10:51 ` Yicong Yang
2021-02-08 21:00 ` Guenter Roeck
0 siblings, 2 replies; 17+ messages in thread
From: Bjorn Helgaas @ 2021-02-05 20:08 UTC (permalink / raw)
To: Guenter Roeck
Cc: Yicong Yang, gregkh, jdelvare, giometti, abbotti, hsweeten, kw,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, prime.zeng, linuxarm
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
> >
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag
2021-02-05 20:08 ` Bjorn Helgaas
@ 2021-02-08 10:51 ` Yicong Yang
2021-02-08 21:00 ` Guenter Roeck
1 sibling, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2021-02-08 10:51 UTC (permalink / raw)
To: Bjorn Helgaas, Guenter Roeck
Cc: gregkh, jdelvare, giometti, abbotti, hsweeten, kw, linux-kernel,
linux-pm, linux-hwmon, devel, linux-kbuild, masahiroy,
michal.lkml, prime.zeng, linuxarm
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
>>>
>
> .
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] hwmon: Use subdir-ccflags-* to inherit debug flag
2021-02-05 20:08 ` Bjorn Helgaas
2021-02-08 10:51 ` Yicong Yang
@ 2021-02-08 21:00 ` Guenter Roeck
1 sibling, 0 replies; 17+ messages in thread
From: Guenter Roeck @ 2021-02-08 21:00 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yicong Yang, gregkh, jdelvare, giometti, abbotti, hsweeten, kw,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, prime.zeng, linuxarm
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
>>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/4] pps: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:44 [PATCH 0/4] Use subdir-ccflags-* to inherit debug flag Yicong Yang
2021-02-05 9:44 ` [PATCH 1/4] driver core: " Yicong Yang
2021-02-05 9:44 ` [PATCH 2/4] hwmon: " Yicong Yang
@ 2021-02-05 9:44 ` Yicong Yang
2021-02-05 9:44 ` [PATCH 4/4] staging: comedi: " Yicong Yang
3 siblings, 0 replies; 17+ messages in thread
From: Yicong Yang @ 2021-02-05 9:44 UTC (permalink / raw)
To: gregkh, jdelvare, linux, giometti, abbotti, hsweeten, kw,
helgaas, linux-kernel, linux-pm, linux-hwmon, devel,
linux-kbuild, masahiroy, michal.lkml
Cc: prime.zeng, yangyicong, linuxarm
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>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/4] staging: comedi: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:44 [PATCH 0/4] Use subdir-ccflags-* to inherit debug flag Yicong Yang
` (2 preceding siblings ...)
2021-02-05 9:44 ` [PATCH 3/4] pps: " Yicong Yang
@ 2021-02-05 9:44 ` Yicong Yang
2021-02-05 9:54 ` Greg KH
3 siblings, 1 reply; 17+ messages in thread
From: Yicong Yang @ 2021-02-05 9:44 UTC (permalink / raw)
To: gregkh, jdelvare, linux, giometti, abbotti, hsweeten, kw,
helgaas, linux-kernel, linux-pm, linux-hwmon, devel,
linux-kbuild, masahiroy, michal.lkml
Cc: prime.zeng, yangyicong, linuxarm
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>
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 4/4] staging: comedi: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:44 ` [PATCH 4/4] staging: comedi: " Yicong Yang
@ 2021-02-05 9:54 ` Greg KH
2021-02-06 0:52 ` Masahiro Yamada
0 siblings, 1 reply; 17+ messages in thread
From: Greg KH @ 2021-02-05 9:54 UTC (permalink / raw)
To: Yicong Yang
Cc: jdelvare, linux, giometti, abbotti, hsweeten, kw, helgaas,
linux-kernel, linux-pm, linux-hwmon, devel, linux-kbuild,
masahiroy, michal.lkml, linuxarm, prime.zeng
On Fri, Feb 05, 2021 at 05:44:15PM +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.
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/4] staging: comedi: Use subdir-ccflags-* to inherit debug flag
2021-02-05 9:54 ` Greg KH
@ 2021-02-06 0:52 ` Masahiro Yamada
0 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2021-02-06 0:52 UTC (permalink / raw)
To: Greg KH
Cc: Yicong Yang, jdelvare, Guenter Roeck, giometti, Ian Abbott,
Hartley Sweeten, kw, Bjorn Helgaas, Linux Kernel Mailing List,
Linux PM mailing list, linux-hwmon, devel,
Linux Kbuild mailing list, Michal Marek, linuxarm, prime.zeng
On Fri, Feb 5, 2021 at 6:54 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Feb 05, 2021 at 05:44:15PM +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.
>
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread