[PATCH] PCI: Use subdir-ccflags-* to inherit debug flag
From: Junhao He <hejunhao2@hisilicon.com> Use subdir-ccflags-* instead of ccflags-* to inherit the debug settings from Kconfig when traversing subdirectories. Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/pci/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc794..d62c4ac 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/ -ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG -- 2.8.1
Hi Yicong, Thank you for taking care of this! By the way, did you have some issues with things like pr_debug() and other things printing debug information not working correctly before? On 21-02-04 19:30:15, 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.
Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/pci/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc794..d62c4ac 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG -- 2.8.1
Reviewed-by: Krzysztof Wilczyński <kw@linux.com> Krzysztof
On 2021/2/4 20:28, Krzysztof Wilczyński wrote:
Hi Yicong,
Thank you for taking care of this!
By the way, did you have some issues with things like pr_debug() and other things printing debug information not working correctly before?
i cannot remember that i have met any, so maybe they work properly. :)
On 21-02-04 19:30:15, 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.
Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/pci/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc794..d62c4ac 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG -- 2.8.1
Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
Krzysztof
.
Hi Yicong, [...]
By the way, did you have some issues with things like pr_debug() and other things printing debug information not working correctly before?
i cannot remember that i have met any, so maybe they work properly. :) [...]
That's good to know! I suspect, some of the pr_debug() invocations might not be working correctly at the moment, so this is a nice improvement. Having said that, if you have some time, can you update the patch against the PCI tree with the commit message from this thread? https://lore.kernel.org/lkml/1612868899-9185-1-git-send-email-yangyicong@his... The commit messages there for the individual patches are much nicer, so if you don't mind, it would be nice have the same one in the PCI tree. My review still applies. Thank you again for sending the patch over! Krzysztof
On 2021/2/9 21:27, Krzysztof Wilczyński wrote:
Hi Yicong,
[...]
By the way, did you have some issues with things like pr_debug() and other things printing debug information not working correctly before?
i cannot remember that i have met any, so maybe they work properly. :) [...]
That's good to know! I suspect, some of the pr_debug() invocations might not be working correctly at the moment, so this is a nice improvement.
Having said that, if you have some time, can you update the patch against the PCI tree with the commit message from this thread?
https://lore.kernel.org/lkml/1612868899-9185-1-git-send-email-yangyicong@his...
The commit messages there for the individual patches are much nicer, so if you don't mind, it would be nice have the same one in the PCI tree.
My review still applies.
Thank you again for sending the patch over!
Bjorn has applied this with nicer commit. Thanks for reviewing this. :) Regards, Yicong
Krzysztof _______________________________________________ Linuxarm mailing list -- linuxarm@openeuler.org To unsubscribe send an email to linuxarm-leave@openeuler.org
[+cc Masahiro, Michal, linux-kbuild, linux-kernel] On Thu, Feb 04, 2021 at 07:30: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.
So I guess the current behavior is: If CONFIG_PCI_DEBUG=y, add -DDEBUG to CFLAGS in the current directory, but not in any subdirectories and the behavior after this patch is: If CONFIG_PCI_DEBUG=y, add -DDEBUG to CFLAGS in the current directory and any subdirectories Is that right? That makes sense to me. I wonder if any other places have this issue? 'git grep "^ccflags.*-DDEBUG"' finds a few cases where subdirectories use their own debug config 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 But some have subdirectories that look like they probably should be included by using subdir-ccflags, e.g., drivers/base/Makefile:ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG drivers/base/power/Makefile:ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG # drivers/base/{firmware_loader,regmap,test}/ not included drivers/hwmon/Makefile:ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG # drivers/hwmon/{occ,pmbus}/ not included drivers/pps/Makefile:ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG drivers/pps/clients/Makefile:ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG # drivers/pps/generators/ not included There are many more places that add -DDEBUG to ccflags-y that *don't* have subdirectories. I wonder the default should be that we use subdir-ccflags all the time, and use ccflags only when we actually want different CONFIG_*_DEBUG options for subdirectories.
Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/pci/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc794..d62c4ac 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG -- 2.8.1
On 2021/2/5 0:10, Bjorn Helgaas wrote:
[+cc Masahiro, Michal, linux-kbuild, linux-kernel]
On Thu, Feb 04, 2021 at 07:30: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.
So I guess the current behavior is:
If CONFIG_PCI_DEBUG=y, add -DDEBUG to CFLAGS in the current directory, but not in any subdirectories
and the behavior after this patch is:
If CONFIG_PCI_DEBUG=y, add -DDEBUG to CFLAGS in the current directory and any subdirectories
Is that right? That makes sense to me. I wonder if any other places have this issue?
that's right. we didn't check other places, but some have individual config in their sub-directory as you mentioned below.
'git grep "^ccflags.*-DDEBUG"' finds a few cases where subdirectories use their own debug config 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
But some have subdirectories that look like they probably should be included by using subdir-ccflags, e.g.,
drivers/base/Makefile:ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG drivers/base/power/Makefile:ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG # drivers/base/{firmware_loader,regmap,test}/ not included
drivers/hwmon/Makefile:ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG # drivers/hwmon/{occ,pmbus}/ not included
drivers/pps/Makefile:ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG drivers/pps/clients/Makefile:ccflags-$(CONFIG_PPS_DEBUG) := -DDEBUG # drivers/pps/generators/ not included
There are many more places that add -DDEBUG to ccflags-y that *don't* have subdirectories.
I wonder the default should be that we use subdir-ccflags all the time, and use ccflags only when we actually want different CONFIG_*_DEBUG options for subdirectories.
agree. if there is no debug config in the sub-directory, the config should be inherited from its parent directory using subdir-ccflags. we can post a separate serial to issue other places. Thanks, Yicong
Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/pci/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc794..d62c4ac 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG -- 2.8.1
.
[+cc Masahiro, Michal, linux-kbuild, linux-kernel] On Thu, Feb 04, 2021 at 07:30: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.
Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
I applied this with Krzysztof's reviewed-by and the commit log below to pci/misc for v5.12, thanks! Feel free to copy or improve the commit log for use elsewhere.
--- drivers/pci/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc794..d62c4ac 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
commit e8e9aababe60 ("PCI: Apply CONFIG_PCI_DEBUG to entire drivers/pci hierarchy") Author: Junhao He <hejunhao2@hisilicon.com> Date: Thu Feb 4 19:30:15 2021 +0800 PCI: Apply CONFIG_PCI_DEBUG to entire drivers/pci hierarchy CONFIG_PCI_DEBUG=y adds -DDEBUG to CFLAGS, which enables things like pr_debug() and dev_dbg() (and hence pci_dbg()). Previously we added -DDEBUG for files in drivers/pci/, but not files in subdirectories of drivers/pci/. Add -DDEBUG to CFLAGS for all files below drivers/pci/ so CONFIG_PCI_DEBUG applies to the entire hierarchy. [bhelgaas: commit log] Link: https://lore.kernel.org/r/1612438215-33105-1-git-send-email-yangyicong@hisil... Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: Krzysztof Wilczyński <kw@linux.com> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc79411e2d..d62c4ac4ae1b 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/ -ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
Hi Bjorn, Thank you! This looks great! [...]
commit e8e9aababe60 ("PCI: Apply CONFIG_PCI_DEBUG to entire drivers/pci hierarchy") Author: Junhao He <hejunhao2@hisilicon.com> Date: Thu Feb 4 19:30:15 2021 +0800
PCI: Apply CONFIG_PCI_DEBUG to entire drivers/pci hierarchy
CONFIG_PCI_DEBUG=y adds -DDEBUG to CFLAGS, which enables things like pr_debug() and dev_dbg() (and hence pci_dbg()). Previously we added -DDEBUG for files in drivers/pci/, but not files in subdirectories of drivers/pci/.
Add -DDEBUG to CFLAGS for all files below drivers/pci/ so CONFIG_PCI_DEBUG applies to the entire hierarchy.
[bhelgaas: commit log] Link: https://lore.kernel.org/r/1612438215-33105-1-git-send-email-yangyicong@hisil... Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc79411e2d..d62c4ac4ae1b 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
And thank you again, Yicong, for fixing this. Much appreciated. Krzysztof
On 2021/2/10 5:25, Bjorn Helgaas wrote:
[+cc Masahiro, Michal, linux-kbuild, linux-kernel]
On Thu, Feb 04, 2021 at 07:30: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.
Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
I applied this with Krzysztof's reviewed-by and the commit log below to pci/misc for v5.12, thanks!
Feel free to copy or improve the commit log for use elsewhere.
thanks for improving the commit. i admit that i didn't make the it clear enough. it's much better now. Thanks, Yicong
--- drivers/pci/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc794..d62c4ac 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
commit e8e9aababe60 ("PCI: Apply CONFIG_PCI_DEBUG to entire drivers/pci hierarchy") Author: Junhao He <hejunhao2@hisilicon.com> Date: Thu Feb 4 19:30:15 2021 +0800
PCI: Apply CONFIG_PCI_DEBUG to entire drivers/pci hierarchy
CONFIG_PCI_DEBUG=y adds -DDEBUG to CFLAGS, which enables things like pr_debug() and dev_dbg() (and hence pci_dbg()). Previously we added -DDEBUG for files in drivers/pci/, but not files in subdirectories of drivers/pci/.
Add -DDEBUG to CFLAGS for all files below drivers/pci/ so CONFIG_PCI_DEBUG applies to the entire hierarchy.
[bhelgaas: commit log] Link: https://lore.kernel.org/r/1612438215-33105-1-git-send-email-yangyicong@hisil... Signed-off-by: Junhao He <hejunhao2@hisilicon.com> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: Krzysztof Wilczyński <kw@linux.com>
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile index 11cc79411e2d..d62c4ac4ae1b 100644 --- a/drivers/pci/Makefile +++ b/drivers/pci/Makefile @@ -36,4 +36,4 @@ obj-$(CONFIG_PCI_ENDPOINT) += endpoint/ obj-y += controller/ obj-y += switch/
-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG +subdir-ccflags-$(CONFIG_PCI_DEBUG) := -DDEBUG
.
participants (3)
-
Bjorn Helgaas -
Krzysztof Wilczyński -
Yicong Yang