Message ID | 20200122150147.25709-4-wolfgang.wallner@br-automation.com |
---|---|
State | Accepted |
Commit | 43709fa0888cc80648939ae1588307334e6cc267 |
Headers | show |
Series | x86: Move ITSS from Apollo Lake to a more generic location | expand |
On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner <wolfgang.wallner at br-automation.com> wrote: > > The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so > move it to a common location within arch/x86. > > Signed-off-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com> > --- > At the moment, this commit enables building of itss.o unconditionally. > which is a bad idea I guess. > What is the preferred way to handle this? > Should I add a kconfig option e.g. in arch/x86/Kconfig? > > arch/x86/cpu/apollolake/Makefile | 1 - > arch/x86/cpu/intel_common/Makefile | 1 + > arch/x86/cpu/{apollolake => intel_common}/itss.c | 0 > 3 files changed, 1 insertion(+), 1 deletion(-) > rename arch/x86/cpu/{apollolake => intel_common}/itss.c (100%) Reviewed-by: Simon Glass <sjg at chromium.org>
Hello Simon, -----"Simon Glass" <sjg at chromium.org> schrieb: ----- > On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner > <wolfgang.wallner at br-automation.com> wrote: > > > > The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so > > move it to a common location within arch/x86. > > > > Signed-off-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com> > > --- > > At the moment, this commit enables building of itss.o unconditionally. > > which is a bad idea I guess. > > What is the preferred way to handle this? > > Should I add a kconfig option e.g. in arch/x86/Kconfig? Thank you for reviewing. But I'm still not sure how to handle the question mentioned above, and would like to ask for further feedback. My current idea would be to add a new kconfig option CONFIG_ITSS in arch/x86/Kconfig to control building of this driver (which would be automatically implied via 'select' when the build target is Apollo Lake). Does that make sense? If so, I would include the necessary modifications to kconfig in the next version of this patch. > > > > arch/x86/cpu/apollolake/Makefile | 1 - > > arch/x86/cpu/intel_common/Makefile | 1 + > > arch/x86/cpu/{apollolake => intel_common}/itss.c | 0 > > 3 files changed, 1 insertion(+), 1 deletion(-) > > rename arch/x86/cpu/{apollolake => intel_common}/itss.c (100%) > > Reviewed-by: Simon Glass <sjg at chromium.org> regards, Wolfgang
Hi Wolfgang, On Fri, 31 Jan 2020 at 00:47, Wolfgang Wallner <wolfgang.wallner at br-automation.com> wrote: > > Hello Simon, > > -----"Simon Glass" <sjg at chromium.org> schrieb: ----- > > On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner > > <wolfgang.wallner at br-automation.com> wrote: > > > > > > The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so > > > move it to a common location within arch/x86. > > > > > > Signed-off-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com> > > > --- > > > At the moment, this commit enables building of itss.o unconditionally. > > > which is a bad idea I guess. > > > What is the preferred way to handle this? > > > Should I add a kconfig option e.g. in arch/x86/Kconfig? > > Thank you for reviewing. > > But I'm still not sure how to handle the question mentioned above, and > would like to ask for further feedback. > > My current idea would be to add a new kconfig option CONFIG_ITSS in > arch/x86/Kconfig to control building of this driver (which would be > automatically implied via 'select' when the build target is Apollo Lake). > > Does that make sense? If so, I would include the necessary modifications > to kconfig in the next version of this patch. Yes that sounds fine to me. > > > > > > > arch/x86/cpu/apollolake/Makefile | 1 - > > > arch/x86/cpu/intel_common/Makefile | 1 + > > > arch/x86/cpu/{apollolake => intel_common}/itss.c | 0 > > > 3 files changed, 1 insertion(+), 1 deletion(-) > > > rename arch/x86/cpu/{apollolake => intel_common}/itss.c (100%) > > > > Reviewed-by: Simon Glass <sjg at chromium.org> > > > regards, Wolfgang Regards, Simon
diff --git a/arch/x86/cpu/apollolake/Makefile b/arch/x86/cpu/apollolake/Makefile index 1760df54d8..f99f2c6473 100644 --- a/arch/x86/cpu/apollolake/Makefile +++ b/arch/x86/cpu/apollolake/Makefile @@ -19,7 +19,6 @@ obj-y += fsp_s.o endif obj-y += hostbridge.o -obj-y += itss.o obj-y += lpc.o obj-y += p2sb.o obj-y += pch.o diff --git a/arch/x86/cpu/intel_common/Makefile b/arch/x86/cpu/intel_common/Makefile index cc4e1c962b..266e6e26fa 100644 --- a/arch/x86/cpu/intel_common/Makefile +++ b/arch/x86/cpu/intel_common/Makefile @@ -27,6 +27,7 @@ obj-y += microcode.o endif endif obj-y += pch.o +obj-y += itss.o ifdef CONFIG_SPL ifndef CONFIG_SPL_BUILD diff --git a/arch/x86/cpu/apollolake/itss.c b/arch/x86/cpu/intel_common/itss.c similarity index 100% rename from arch/x86/cpu/apollolake/itss.c rename to arch/x86/cpu/intel_common/itss.c
The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so move it to a common location within arch/x86. Signed-off-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com> --- At the moment, this commit enables building of itss.o unconditionally. which is a bad idea I guess. What is the preferred way to handle this? Should I add a kconfig option e.g. in arch/x86/Kconfig? arch/x86/cpu/apollolake/Makefile | 1 - arch/x86/cpu/intel_common/Makefile | 1 + arch/x86/cpu/{apollolake => intel_common}/itss.c | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename arch/x86/cpu/{apollolake => intel_common}/itss.c (100%)