diff mbox series

[RFC,3/4] x86: Move itss.c from Apollo Lake to a more generic location

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

Commit Message

Wolfgang Wallner Jan. 22, 2020, 3:01 p.m. UTC
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%)

Comments

Simon Glass Jan. 30, 2020, 2:16 a.m. UTC | #1
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>
Wolfgang Wallner Jan. 31, 2020, 7:47 a.m. UTC | #2
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
Simon Glass Jan. 31, 2020, 6:14 p.m. UTC | #3
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 mbox series

Patch

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