[RFC PATCH 0/4] x86: Move ITSS from Apollo Lake to a more generic location

[RFC PATCH 0/4] x86: Move ITSS from Apollo Lake to a more generic location

[Wolfgang Wallner]

The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so
move it to a more generic location.
Additionally, the series adds the term "Interrupt Timer Subsystem" for
ITSS in the file descriptions.

Moving the ITSS implementation out of the Apllo Lake-specific folders not
only allows to use this driver on other platforms, it is also useful for
Apollo Lake when U-Boot is chain-loaded via coreboot (in which case the
files within arch/x86/cpu/apollolake would not be compiled and the
include/asm/arch-symlink points to arch-coreboot instead of
arch-apollolake).


Wolfgang Wallner (4):
  x86: apl: Add the term "Interrupt Timer Subsystem" to ITSS files
  x86: Move itss.h from Apollo Lake to the generic x86 include directory
  x86: Move itss.c from Apollo Lake to a more generic location
  x86: itss: Remove apl-prefix

 arch/x86/cpu/apollolake/Makefile              |  1 -
 arch/x86/cpu/intel_common/Makefile            |  1 +
 .../cpu/{apollolake => intel_common}/itss.c   | 60 +++++++++----------
 arch/x86/dts/chromebook_coral.dts             |  2 +-
 .../include/asm/{arch-apollolake => }/itss.h  |  2 +
 drivers/pinctrl/intel/pinctrl.c               |  2 +-
 6 files changed, 35 insertions(+), 33 deletions(-)
 rename arch/x86/cpu/{apollolake => intel_common}/itss.c (73%)
 rename arch/x86/include/asm/{arch-apollolake => }/itss.h (97%)

-- 
2.25.0

[RFC PATCH 1/4] x86: apl: Add the term "Interrupt Timer Subsystem" to ITSS files

[Wolfgang Wallner]

ITSS stands for "Interrupt Timer Subsystem", so add that term to the
description of the relevant files.

Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
---
ITSS stands for "Interrupt Timer Subsystem", at least according to
coreboot [1].

[1] https://coreboot.org/status/kconfig-options.html

 arch/x86/cpu/apollolake/itss.c              | 2 +-
 arch/x86/include/asm/arch-apollolake/itss.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/cpu/apollolake/itss.c b/arch/x86/cpu/apollolake/itss.c
index 8789f8e6bb..95c9ebddc1 100644
--- a/arch/x86/cpu/apollolake/itss.c
+++ b/arch/x86/cpu/apollolake/itss.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Something to do with Interrupts, but I don't know what ITSS stands for
+ * Interrupt Timer Subsystem
  *
  * Copyright (C) 2017 Intel Corporation.
  * Copyright (C) 2017 Siemens AG
diff --git a/arch/x86/include/asm/arch-apollolake/itss.h b/arch/x86/include/asm/arch-apollolake/itss.h
index 1e29503974..c75d8fe8c2 100644
--- a/arch/x86/include/asm/arch-apollolake/itss.h
+++ b/arch/x86/include/asm/arch-apollolake/itss.h
@@ -1,5 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /*
+ * Interrupt Timer Subsystem
+ *
  * Copyright (C) 2017 Intel Corporation.
  * Copyright 2019 Google LLC
  *
-- 
2.25.0

[RFC PATCH 2/4] x86: Move itss.h from Apollo Lake to the generic x86 include directory

[Wolfgang Wallner]

The code in this file is not specific to Apollo Lake. According to
coreboot sources (where this code comes from), it is common to at least:
  * Apollo Lake
  * Cannon Lake
  * Ice Lake
  * Skylake

Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
---

 arch/x86/cpu/apollolake/itss.c                    | 2 +-
 arch/x86/include/asm/{arch-apollolake => }/itss.h | 0
 drivers/pinctrl/intel/pinctrl.c                   | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/x86/include/asm/{arch-apollolake => }/itss.h (100%)

diff --git a/arch/x86/cpu/apollolake/itss.c b/arch/x86/cpu/apollolake/itss.c
index 95c9ebddc1..ff7a83d618 100644
--- a/arch/x86/cpu/apollolake/itss.c
+++ b/arch/x86/cpu/apollolake/itss.c
@@ -15,7 +15,7 @@
 #include <irq.h>
 #include <p2sb.h>
 #include <spl.h>
-#include <asm/arch/itss.h>
+#include <asm/itss.h>
 
 struct apl_itss_platdata {
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
diff --git a/arch/x86/include/asm/arch-apollolake/itss.h b/arch/x86/include/asm/itss.h
similarity index 100%
rename from arch/x86/include/asm/arch-apollolake/itss.h
rename to arch/x86/include/asm/itss.h
diff --git a/drivers/pinctrl/intel/pinctrl.c b/drivers/pinctrl/intel/pinctrl.c
index 4875a3b0b5..5bf5d8b0e2 100644
--- a/drivers/pinctrl/intel/pinctrl.c
+++ b/drivers/pinctrl/intel/pinctrl.c
@@ -25,7 +25,7 @@
 #include <asm/intel_pinctrl.h>
 #include <asm/intel_pinctrl_defs.h>
 #include <asm/arch/gpio.h>
-#include <asm/arch/itss.h>
+#include <asm/itss.h>
 #include <dm/device-internal.h>
 #include <dt-bindings/gpio/gpio.h>
 
-- 
2.25.0

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

[Wolfgang Wallner]

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@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%)

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
-- 
2.25.0

[RFC PATCH 4/4] x86: itss: Remove apl-prefix

[Wolfgang Wallner]

The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so
remove the apl-prefix of the implemented functions/structures/...

Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
---

 arch/x86/cpu/intel_common/itss.c  | 56 +++++++++++++++----------------
 arch/x86/dts/chromebook_coral.dts |  2 +-
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/arch/x86/cpu/intel_common/itss.c b/arch/x86/cpu/intel_common/itss.c
index ff7a83d618..9df51adecc 100644
--- a/arch/x86/cpu/intel_common/itss.c
+++ b/arch/x86/cpu/intel_common/itss.c
@@ -17,10 +17,10 @@
 #include <spl.h>
 #include <asm/itss.h>
 
-struct apl_itss_platdata {
+struct itss_platdata {
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
 	/* Put this first since driver model will copy the data here */
-	struct dtd_intel_apl_itss dtplat;
+	struct dtd_intel_itss dtplat;
 #endif
 };
 
@@ -30,13 +30,13 @@ struct pmc_route {
 	u32 gpio;
 };
 
-struct apl_itss_priv {
+struct itss_priv {
 	struct pmc_route *route;
 	uint route_count;
 	u32 irq_snapshot[NUM_IPC_REGS];
 };
 
-static int apl_set_polarity(struct udevice *dev, uint irq, bool active_low)
+static int set_polarity(struct udevice *dev, uint irq, bool active_low)
 {
 	u32 mask;
 	uint reg;
@@ -53,9 +53,9 @@ static int apl_set_polarity(struct udevice *dev, uint irq, bool active_low)
 }
 
 #ifndef CONFIG_TPL_BUILD
-static int apl_snapshot_polarities(struct udevice *dev)
+static int snapshot_polarities(struct udevice *dev)
 {
-	struct apl_itss_priv *priv = dev_get_priv(dev);
+	struct itss_priv *priv = dev_get_priv(dev);
 	const int start = GPIO_IRQ_START;
 	const int end = GPIO_IRQ_END;
 	int reg_start;
@@ -86,9 +86,9 @@ static void show_polarities(struct udevice *dev, const char *msg)
 	}
 }
 
-static int apl_restore_polarities(struct udevice *dev)
+static int restore_polarities(struct udevice *dev)
 {
-	struct apl_itss_priv *priv = dev_get_priv(dev);
+	struct itss_priv *priv = dev_get_priv(dev);
 	const int start = GPIO_IRQ_START;
 	const int end = GPIO_IRQ_END;
 	int reg_start;
@@ -132,9 +132,9 @@ static int apl_restore_polarities(struct udevice *dev)
 }
 #endif
 
-static int apl_route_pmc_gpio_gpe(struct udevice *dev, uint pmc_gpe_num)
+static int route_pmc_gpio_gpe(struct udevice *dev, uint pmc_gpe_num)
 {
-	struct apl_itss_priv *priv = dev_get_priv(dev);
+	struct itss_priv *priv = dev_get_priv(dev);
 	struct pmc_route *route;
 	int i;
 
@@ -146,14 +146,14 @@ static int apl_route_pmc_gpio_gpe(struct udevice *dev, uint pmc_gpe_num)
 	return -ENOENT;
 }
 
-static int apl_itss_ofdata_to_platdata(struct udevice *dev)
+static int itss_ofdata_to_platdata(struct udevice *dev)
 {
-	struct apl_itss_priv *priv = dev_get_priv(dev);
+	struct itss_priv *priv = dev_get_priv(dev);
 	int ret;
 
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
-	struct apl_itss_platdata *plat = dev_get_platdata(dev);
-	struct dtd_intel_apl_itss *dtplat = &plat->dtplat;
+	struct itss_platdata *plat = dev_get_platdata(dev);
+	struct dtd_intel_itss *dtplat = &plat->dtplat;
 
 	/*
 	 * It would be nice to do this in the bind() method, but with
@@ -189,26 +189,26 @@ static int apl_itss_ofdata_to_platdata(struct udevice *dev)
 	return 0;
 }
 
-static const struct irq_ops apl_itss_ops = {
-	.route_pmc_gpio_gpe	= apl_route_pmc_gpio_gpe,
-	.set_polarity	= apl_set_polarity,
+static const struct irq_ops itss_ops = {
+	.route_pmc_gpio_gpe	= route_pmc_gpio_gpe,
+	.set_polarity	= set_polarity,
 #ifndef CONFIG_TPL_BUILD
-	.snapshot_polarities = apl_snapshot_polarities,
-	.restore_polarities = apl_restore_polarities,
+	.snapshot_polarities = snapshot_polarities,
+	.restore_polarities = restore_polarities,
 #endif
 };
 
-static const struct udevice_id apl_itss_ids[] = {
-	{ .compatible = "intel,apl-itss"},
+static const struct udevice_id itss_ids[] = {
+	{ .compatible = "intel,itss"},
 	{ }
 };
 
-U_BOOT_DRIVER(apl_itss_drv) = {
-	.name		= "intel_apl_itss",
+U_BOOT_DRIVER(itss_drv) = {
+	.name		= "intel_itss",
 	.id		= UCLASS_IRQ,
-	.of_match	= apl_itss_ids,
-	.ops		= &apl_itss_ops,
-	.ofdata_to_platdata = apl_itss_ofdata_to_platdata,
-	.platdata_auto_alloc_size = sizeof(struct apl_itss_platdata),
-	.priv_auto_alloc_size = sizeof(struct apl_itss_priv),
+	.of_match	= itss_ids,
+	.ops		= &itss_ops,
+	.ofdata_to_platdata = itss_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct itss_platdata),
+	.priv_auto_alloc_size = sizeof(struct itss_priv),
 };
diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts
index 24fcbb5063..a1820fa187 100644
--- a/arch/x86/dts/chromebook_coral.dts
+++ b/arch/x86/dts/chromebook_coral.dts
@@ -171,7 +171,7 @@
 
 			itss {
 				u-boot,dm-pre-reloc;
-				compatible = "intel,apl-itss";
+				compatible = "intel,itss";
 				intel,p2sb-port-id = <PID_ITSS>;
 				intel,pmc-routes = <
 					PMC_GPE_SW_31_0 GPIO_GPE_SW_31_0
-- 
2.25.0

[RFC PATCH 1/4] x86: apl: Add the term "Interrupt Timer Subsystem" to ITSS files

[Simon Glass]

On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> ITSS stands for "Interrupt Timer Subsystem", so add that term to the
> description of the relevant files.
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
> ITSS stands for "Interrupt Timer Subsystem", at least according to
> coreboot [1].
>
> [1] https://coreboot.org/status/kconfig-options.html
>
>  arch/x86/cpu/apollolake/itss.c              | 2 +-
>  arch/x86/include/asm/arch-apollolake/itss.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[RFC PATCH 2/4] x86: Move itss.h from Apollo Lake to the generic x86 include directory

[Simon Glass]

On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> The code in this file is not specific to Apollo Lake. According to
> coreboot sources (where this code comes from), it is common to at least:
>   * Apollo Lake
>   * Cannon Lake
>   * Ice Lake
>   * Skylake
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
>
>  arch/x86/cpu/apollolake/itss.c                    | 2 +-
>  arch/x86/include/asm/{arch-apollolake => }/itss.h | 0
>  drivers/pinctrl/intel/pinctrl.c                   | 2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename arch/x86/include/asm/{arch-apollolake => }/itss.h (100%)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Simon Glass]

On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner
<wolfgang.wallner@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@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@chromium.org>

[RFC PATCH 4/4] x86: itss: Remove apl-prefix

[Simon Glass]

On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so
> remove the apl-prefix of the implemented functions/structures/...
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
>
>  arch/x86/cpu/intel_common/itss.c  | 56 +++++++++++++++----------------
>  arch/x86/dts/chromebook_coral.dts |  2 +-
>  2 files changed, 29 insertions(+), 29 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

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

[Wolfgang Wallner]

Hello Simon,

-----"Simon Glass" <sjg@chromium.org> schrieb: -----
> On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner
> <wolfgang.wallner@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@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@chromium.org>


regards, Wolfgang

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

[Simon Glass]

Hi Wolfgang,

On Fri, 31 Jan 2020 at 00:47, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> Hello Simon,
>
> -----"Simon Glass" <sjg@chromium.org> schrieb: -----
> > On Wed, 22 Jan 2020 at 08:01, Wolfgang Wallner
> > <wolfgang.wallner@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@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@chromium.org>
>
>
> regards, Wolfgang

Regards,
Simon

[RFC PATCH 1/4] x86: apl: Add the term "Interrupt Timer Subsystem" to ITSS files

[Bin Meng]

On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> ITSS stands for "Interrupt Timer Subsystem", so add that term to the
> description of the relevant files.
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
> ITSS stands for "Interrupt Timer Subsystem", at least according to
> coreboot [1].
>
> [1] https://coreboot.org/status/kconfig-options.html
>
>  arch/x86/cpu/apollolake/itss.c              | 2 +-
>  arch/x86/include/asm/arch-apollolake/itss.h | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[RFC PATCH 2/4] x86: Move itss.h from Apollo Lake to the generic x86 include directory

[Bin Meng]

On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> The code in this file is not specific to Apollo Lake. According to
> coreboot sources (where this code comes from), it is common to at least:
>   * Apollo Lake
>   * Cannon Lake
>   * Ice Lake
>   * Skylake
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
>
>  arch/x86/cpu/apollolake/itss.c                    | 2 +-
>  arch/x86/include/asm/{arch-apollolake => }/itss.h | 0
>  drivers/pinctrl/intel/pinctrl.c                   | 2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename arch/x86/include/asm/{arch-apollolake => }/itss.h (100%)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

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

[Bin Meng]

On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
<wolfgang.wallner@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@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: Bin Meng <bmeng.cn@gmail.com>

[RFC PATCH 4/4] x86: itss: Remove apl-prefix

[Bin Meng]

On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so
> remove the apl-prefix of the implemented functions/structures/...
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
>
>  arch/x86/cpu/intel_common/itss.c  | 56 +++++++++++++++----------------
>  arch/x86/dts/chromebook_coral.dts |  2 +-
>  2 files changed, 29 insertions(+), 29 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[RFC PATCH 1/4] x86: apl: Add the term "Interrupt Timer Subsystem" to ITSS files

[Bin Meng]

On Mon, Feb 3, 2020 at 5:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
> <wolfgang.wallner@br-automation.com> wrote:
> >
> > ITSS stands for "Interrupt Timer Subsystem", so add that term to the
> > description of the relevant files.
> >
> > Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> > ---
> > ITSS stands for "Interrupt Timer Subsystem", at least according to
> > coreboot [1].
> >
> > [1] https://coreboot.org/status/kconfig-options.html
> >
> >  arch/x86/cpu/apollolake/itss.c              | 2 +-
> >  arch/x86/include/asm/arch-apollolake/itss.h | 2 ++
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

[RFC PATCH 2/4] x86: Move itss.h from Apollo Lake to the generic x86 include directory

[Bin Meng]

On Mon, Feb 3, 2020 at 5:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
> <wolfgang.wallner@br-automation.com> wrote:
> >
> > The code in this file is not specific to Apollo Lake. According to
> > coreboot sources (where this code comes from), it is common to at least:
> >   * Apollo Lake
> >   * Cannon Lake
> >   * Ice Lake
> >   * Skylake
> >
> > Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> > ---
> >
> >  arch/x86/cpu/apollolake/itss.c                    | 2 +-
> >  arch/x86/include/asm/{arch-apollolake => }/itss.h | 0
> >  drivers/pinctrl/intel/pinctrl.c                   | 2 +-
> >  3 files changed, 2 insertions(+), 2 deletions(-)
> >  rename arch/x86/include/asm/{arch-apollolake => }/itss.h (100%)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

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

[Bin Meng]

On Mon, Feb 3, 2020 at 5:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
> <wolfgang.wallner@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@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: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

[RFC PATCH 4/4] x86: itss: Remove apl-prefix

[Bin Meng]

On Mon, Feb 3, 2020 at 5:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jan 22, 2020 at 11:01 PM Wolfgang Wallner
> <wolfgang.wallner@br-automation.com> wrote:
> >
> > The Interrupt Timer Subsystem (ITSS) is not specific to Apollo Lake, so
> > remove the apl-prefix of the implemented functions/structures/...
> >
> > Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> > ---
> >
> >  arch/x86/cpu/intel_common/itss.c  | 56 +++++++++++++++----------------
> >  arch/x86/dts/chromebook_coral.dts |  2 +-
> >  2 files changed, 29 insertions(+), 29 deletions(-)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

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

[Wolfgang Wallner]

Hi Bin,

> -----"Bin Meng" <bmeng.cn@gmail.com> schrieb: -----
> applied to u-boot-x86, thanks!

I intended to send a V2 of this patch, see discussion at [1].
As you have pulled that patch now, should I send the proposed modifications
to kconfig as a separate patch instead of including it in a V2 of this one?

regards, Wolfgang

[1] https://lists.denx.de/pipermail/u-boot/2020-January/398711.html

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

[Bin Meng]

Hi Wolfgang,

On Mon, Feb 3, 2020 at 6:05 PM Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> Hi Bin,
>
> > -----"Bin Meng" <bmeng.cn@gmail.com> schrieb: -----
> > applied to u-boot-x86, thanks!
>
> I intended to send a V2 of this patch, see discussion at [1].
> As you have pulled that patch now, should I send the proposed modifications
> to kconfig as a separate patch instead of including it in a V2 of this one?
>

It's OK to send a separate patch for the Kconfig option. Thanks!

Regards,
Bin