| Message ID | 1612518255-23052-1-git-send-email-yangyicong@hisilicon.com |
|---|---|
| Headers | show |
| Series | Use subdir-ccflags-* to inherit debug flag | expand |
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
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
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 > > . >
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
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 > > . >
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. > > . >