[V2 0/3] Introduce AMD Seattle platform support

[V2 0/3] Introduce AMD Seattle platform support

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch series introduce minimal support for AMD Seattle system.
This has been tested to boot into dom0 and domU.

The is platform currently using spintable as multi-core booting protocol.
However, it does supports system_off and system_reset from PSCI-0.2.
The firmware will support full PSCI-0.2 supports (except MIGRATION)
in the future.

It also introduces GIC-400 device-tree matching string.

Changes from V1:
    * Remove dependency of system_off/reset of the Seattle platform
      from PSCI subsystem.
    * PSCI stuff is not re-submitted as a separate patch series.

Suravee Suthikulpanit (3):
  gicv2: Add compatibility id for gic-400
  amd/seattle: Add early printk message for the platform
  amd/seattle: Initial revision of AMD Seattle support

 xen/arch/arm/Rules.mk            |  5 +++
 xen/arch/arm/gic-v2.c            |  1 +
 xen/arch/arm/platforms/Makefile  |  1 +
 xen/arch/arm/platforms/seattle.c | 69 ++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/gic.h        |  4 ++-
 5 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 xen/arch/arm/platforms/seattle.c

-- 
1.9.3

[V2 1/3] gicv2: Add compatibility id for gic-400

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/gic-v2.c     | 1 +
 xen/include/asm-arm/gic.h | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/gic-v2.c b/xen/arch/arm/gic-v2.c
index f053b5d..faad1ff 100644
--- a/xen/arch/arm/gic-v2.c
+++ b/xen/arch/arm/gic-v2.c
@@ -768,6 +768,7 @@ static const char * const gicv2_dt_compat[] __initconst =
 {
     DT_COMPAT_GIC_CORTEX_A15,
     DT_COMPAT_GIC_CORTEX_A7,
+    DT_COMPAT_GIC_400,
     NULL
 };
 
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 51a0a26..187dc46 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -152,11 +152,13 @@
 #include <xen/irq.h>
 #include <asm-arm/vgic.h>
 
+#define DT_COMPAT_GIC_400            "arm,gic-400"
 #define DT_COMPAT_GIC_CORTEX_A15     "arm,cortex-a15-gic"
 #define DT_COMPAT_GIC_CORTEX_A7      "arm,cortex-a7-gic"
 
 #define DT_MATCH_GIC_V2 DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A15), \
-                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7)
+                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_CORTEX_A7), \
+                        DT_MATCH_COMPATIBLE(DT_COMPAT_GIC_400)
 
 #define DT_COMPAT_GIC_V3             "arm,gic-v3"
 
-- 
1.9.3

[V2 2/3] amd/seattle: Add early printk message for the platform

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/Rules.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 26fafa2..35b6253 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -100,6 +100,11 @@ EARLY_PRINTK_INC := pl011
 EARLY_PRINTK_BAUD := 115200
 EARLY_UART_BASE_ADDRESS := 0x7ff80000
 endif
+ifeq ($(CONFIG_EARLY_PRINTK), seattle)
+EARLY_PRINTK_INC := pl011
+EARLY_PRINTK_BAUD := 115200
+EARLY_UART_BASE_ADDRESS := 0xe1010000
+endif
 
 ifneq ($(EARLY_PRINTK_INC),)
 EARLY_PRINTK := y
-- 
1.9.3

[V2 3/3] amd/seattle: Initial revision of AMD Seattle support

[suravee.suthikulpanit]

From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

This patch add inital (minimal) platform support for AMD Seattle,
which mainly just define the matching ID, and specify system_off,
and sytem_reset mechanism.

Initially, the firmware only support a subset of PSCI-0.2 functions,
system-off and sytem-reset. The boot protocol is still using spin-table.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 xen/arch/arm/platforms/Makefile  |  1 +
 xen/arch/arm/platforms/seattle.c | 69 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)
 create mode 100644 xen/arch/arm/platforms/seattle.c

diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index 680364f..03e7a14 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_ARM_32) += midway.o
 obj-$(CONFIG_ARM_32) += omap5.o
 obj-$(CONFIG_ARM_32) += sunxi.o
 obj-$(CONFIG_ARM_64) += xgene-storm.o
+obj-$(CONFIG_ARM_64) += seattle.o
diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
new file mode 100644
index 0000000..06d4e99
--- /dev/null
+++ b/xen/arch/arm/platforms/seattle.c
@@ -0,0 +1,69 @@
+/*
+ * xen/arch/arm/seattle.c
+ *
+ * AMD Seattle specific settings
+ *
+ * Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
+ * Copyright (c) 2014 Advance Micro Devices Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/platform.h>
+#include <xen/mm.h>
+#include <xen/vmap.h>
+#include <asm/io.h>
+#include <asm/gic.h>
+#include <asm/psci.h>
+
+static const char * const seattle_dt_compat[] __initconst =
+{
+    "amd,seattle",
+    NULL
+};
+
+/* Seattle firmware only implements PSCI handler for
+ * system off and system reset at this point.
+ * This is temporary until full PSCI-0.2 is supported.
+ * Then, these function will be removed.
+ */
+static noinline void seattle_smc_psci(register_t func_id)
+{
+    asm volatile(
+        "smc #0"
+        : "+r" (func_id)
+        :);
+}
+
+static noinline void seattle_system_reset(void)
+{
+    seattle_smc_psci(PSCI_0_2_FN_SYSTEM_RESET);
+}
+
+static noinline void seattle_system_off(void)
+{
+    seattle_smc_psci(PSCI_0_2_FN_SYSTEM_OFF);
+}
+
+PLATFORM_START(seattle, "SEATTLE")
+    .compatible = seattle_dt_compat,
+    .reset      = seattle_system_reset,
+    .poweroff   = seattle_system_off,
+PLATFORM_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
1.9.3

Re: [V2 3/3] amd/seattle: Initial revision of AMD Seattle support

[Julien Grall]

Hi Suravee,

On 10/01/2014 08:51 PM, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch add inital (minimal) platform support for AMD Seattle,
> which mainly just define the matching ID, and specify system_off,
> and sytem_reset mechanism.
> 
> Initially, the firmware only support a subset of PSCI-0.2 functions,
> system-off and sytem-reset. The boot protocol is still using spin-table.

s/sytem-reset/system-reset/

I find "the boot protocol" not clear, I guess you are talking about CPU
bring up. Maybe something like "CPU management ..." will be better?

> 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
>  xen/arch/arm/platforms/Makefile  |  1 +
>  xen/arch/arm/platforms/seattle.c | 69 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
>  create mode 100644 xen/arch/arm/platforms/seattle.c
> 
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index 680364f..03e7a14 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_ARM_32) += midway.o
>  obj-$(CONFIG_ARM_32) += omap5.o
>  obj-$(CONFIG_ARM_32) += sunxi.o
>  obj-$(CONFIG_ARM_64) += xgene-storm.o
> +obj-$(CONFIG_ARM_64) += seattle.o

NIT: Could we order the platform name alphabetically? i.e move
"seattle.o" just above "xgene-storm.o".

> diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
> new file mode 100644
> index 0000000..06d4e99
> --- /dev/null
> +++ b/xen/arch/arm/platforms/seattle.c
> @@ -0,0 +1,69 @@
> +/*
> + * xen/arch/arm/seattle.c
> + *
> + * AMD Seattle specific settings
> + *
> + * Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> + * Copyright (c) 2014 Advance Micro Devices Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <asm/platform.h>

> +#include <xen/mm.h>
> +#include <xen/vmap.h>
> +#include <asm/io.h>
> +#include <asm/gic.h>

I don't think those 4 includes are required here.

> +#include <asm/psci.h>
> +
> +static const char * const seattle_dt_compat[] __initconst =
> +{
> +    "amd,seattle",
> +    NULL
> +};
> +
> +/* Seattle firmware only implements PSCI handler for
> + * system off and system reset at this point.
> + * This is temporary until full PSCI-0.2 is supported.
> + * Then, these function will be removed.
> + */
> +static noinline void seattle_smc_psci(register_t func_id)
> +{
> +    asm volatile(
> +        "smc #0"
> +        : "+r" (func_id)
> +        :);
> +}

We already have multiple implementation of smc in different place. Can
we provide a common function rather than adding another one?


> +static noinline void seattle_system_reset(void)

noinline is not necessary here.

> +{
> +    seattle_smc_psci(PSCI_0_2_FN_SYSTEM_RESET);
> +}
> +
> +static noinline void seattle_system_off(void)

ditto

> +{
> +    seattle_smc_psci(PSCI_0_2_FN_SYSTEM_OFF);
> +}
> +
> +PLATFORM_START(seattle, "SEATTLE")
> +    .compatible = seattle_dt_compat,
> +    .reset      = seattle_system_reset,
> +    .poweroff   = seattle_system_off,
> +PLATFORM_END
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> 

Regards,

-- 
Julien Grall

Re: [V2 0/3] Introduce AMD Seattle platform support

[Ian Campbell]

On Wed, 2014-10-01 at 14:51 -0500, suravee.suthikulpanit@amd.com wrote:

Since we are post feature freeze this needs an ack from the release
manager, Konrad CCd.

Konrad, this is similar to the bcm series in that the only damage it can
do is to the new platform being added. I think we should take it for
4.5.

> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> This patch series introduce minimal support for AMD Seattle system.
> This has been tested to boot into dom0 and domU.
> 
> The is platform currently using spintable as multi-core booting protocol.
> However, it does supports system_off and system_reset from PSCI-0.2.
> The firmware will support full PSCI-0.2 supports (except MIGRATION)
> in the future.
> 
> It also introduces GIC-400 device-tree matching string.
> 
> Changes from V1:
>     * Remove dependency of system_off/reset of the Seattle platform
>       from PSCI subsystem.
>     * PSCI stuff is not re-submitted as a separate patch series.
> 
> Suravee Suthikulpanit (3):
>   gicv2: Add compatibility id for gic-400
>   amd/seattle: Add early printk message for the platform
>   amd/seattle: Initial revision of AMD Seattle support
> 
>  xen/arch/arm/Rules.mk            |  5 +++
>  xen/arch/arm/gic-v2.c            |  1 +
>  xen/arch/arm/platforms/Makefile  |  1 +
>  xen/arch/arm/platforms/seattle.c | 69 ++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/gic.h        |  4 ++-
>  5 files changed, 79 insertions(+), 1 deletion(-)
>  create mode 100644 xen/arch/arm/platforms/seattle.c
> 

Re: [V2 0/3] Introduce AMD Seattle platform support

[Konrad Rzeszutek Wilk]

On Thu, Oct 02, 2014 at 01:59:41PM +0100, Ian Campbell wrote:
> On Wed, 2014-10-01 at 14:51 -0500, suravee.suthikulpanit@amd.com wrote:
> 
> Since we are post feature freeze this needs an ack from the release
> manager, Konrad CCd.
> 
> Konrad, this is similar to the bcm series in that the only damage it can
> do is to the new platform being added. I think we should take it for
> 4.5.

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> > From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> > 
> > This patch series introduce minimal support for AMD Seattle system.
> > This has been tested to boot into dom0 and domU.
> > 
> > The is platform currently using spintable as multi-core booting protocol.
> > However, it does supports system_off and system_reset from PSCI-0.2.
> > The firmware will support full PSCI-0.2 supports (except MIGRATION)
> > in the future.
> > 
> > It also introduces GIC-400 device-tree matching string.
> > 
> > Changes from V1:
> >     * Remove dependency of system_off/reset of the Seattle platform
> >       from PSCI subsystem.
> >     * PSCI stuff is not re-submitted as a separate patch series.
> > 
> > Suravee Suthikulpanit (3):
> >   gicv2: Add compatibility id for gic-400
> >   amd/seattle: Add early printk message for the platform
> >   amd/seattle: Initial revision of AMD Seattle support
> > 
> >  xen/arch/arm/Rules.mk            |  5 +++
> >  xen/arch/arm/gic-v2.c            |  1 +
> >  xen/arch/arm/platforms/Makefile  |  1 +
> >  xen/arch/arm/platforms/seattle.c | 69 ++++++++++++++++++++++++++++++++++++++++
> >  xen/include/asm-arm/gic.h        |  4 ++-
> >  5 files changed, 79 insertions(+), 1 deletion(-)
> >  create mode 100644 xen/arch/arm/platforms/seattle.c
> > 
> 
> 

Re: [V2 2/3] amd/seattle: Add early printk message for the platform

[Julien Grall]

Hi Suravee,

On 10/01/2014 08:51 PM, suravee.suthikulpanit@amd.com wrote:
> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> 
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Reviewed-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/Rules.mk | 5 +++++
>  1 file changed, 5 insertions(+)


Just noticed, could you update docs/misc/arm/early-printk.txt to add an
entry for your board?

Thanks,

> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 26fafa2..35b6253 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -100,6 +100,11 @@ EARLY_PRINTK_INC := pl011
>  EARLY_PRINTK_BAUD := 115200
>  EARLY_UART_BASE_ADDRESS := 0x7ff80000
>  endif
> +ifeq ($(CONFIG_EARLY_PRINTK), seattle)
> +EARLY_PRINTK_INC := pl011
> +EARLY_PRINTK_BAUD := 115200
> +EARLY_UART_BASE_ADDRESS := 0xe1010000
> +endif
>  
>  ifneq ($(EARLY_PRINTK_INC),)
>  EARLY_PRINTK := y
> 


-- 
Julien Grall

Re: [V2 2/3] amd/seattle: Add early printk message for the platform

[Suravee Suthikulpanit]

On 10/02/2014 10:52 AM, Julien Grall wrote:
> Hi Suravee,
>
> On 10/01/2014 08:51 PM, suravee.suthikulpanit@amd.com wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> Reviewed-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>   xen/arch/arm/Rules.mk | 5 +++++
>>   1 file changed, 5 insertions(+)
>
>
> Just noticed, could you update docs/misc/arm/early-printk.txt to add an
> entry for your board?
>
> Thanks,
>

Done.

Suravee

Re: [V2 3/3] amd/seattle: Initial revision of AMD Seattle support

[Suravee Suthikulpanit]

Please see comments inline below.

On 10/02/2014 05:41 AM, Julien Grall wrote:
> Hi Suravee,
>
> On 10/01/2014 08:51 PM, suravee.suthikulpanit@amd.com wrote:
>> From: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>>
>> This patch add inital (minimal) platform support for AMD Seattle,
>> which mainly just define the matching ID, and specify system_off,
>> and sytem_reset mechanism.
>>
>> Initially, the firmware only support a subset of PSCI-0.2 functions,
>> system-off and sytem-reset. The boot protocol is still using spin-table.
>
> s/sytem-reset/system-reset/

Ah... Thanks

>
> I find "the boot protocol" not clear, I guess you are talking about CPU
> bring up. Maybe something like "CPU management ..." will be better?
>

Reword to: "The mechanism for bring up auxiliary processors is still 
using spin-table."

>>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
>> ---
>>   xen/arch/arm/platforms/Makefile  |  1 +
>>   xen/arch/arm/platforms/seattle.c | 69 ++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 70 insertions(+)
>>   create mode 100644 xen/arch/arm/platforms/seattle.c
>>
>> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
>> index 680364f..03e7a14 100644
>> --- a/xen/arch/arm/platforms/Makefile
>> +++ b/xen/arch/arm/platforms/Makefile
>> @@ -4,3 +4,4 @@ obj-$(CONFIG_ARM_32) += midway.o
>>   obj-$(CONFIG_ARM_32) += omap5.o
>>   obj-$(CONFIG_ARM_32) += sunxi.o
>>   obj-$(CONFIG_ARM_64) += xgene-storm.o
>> +obj-$(CONFIG_ARM_64) += seattle.o
>
> NIT: Could we order the platform name alphabetically? i.e move
> "seattle.o" just above "xgene-storm.o".

Done

>> diff --git a/xen/arch/arm/platforms/seattle.c b/xen/arch/arm/platforms/seattle.c
>> new file mode 100644
>> index 0000000..06d4e99
>> --- /dev/null
>> +++ b/xen/arch/arm/platforms/seattle.c
>> @@ -0,0 +1,69 @@
>> +/*
>> + * xen/arch/arm/seattle.c
>> + *
>> + * AMD Seattle specific settings
>> + *
>> + * Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> + * Copyright (c) 2014 Advance Micro Devices Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <asm/platform.h>
>
>> +#include <xen/mm.h>
>> +#include <xen/vmap.h>
>> +#include <asm/io.h>
>> +#include <asm/gic.h>
>
> I don't think those 4 includes are required here.

Done.

>> +#include <asm/psci.h>
>> +
>> +static const char * const seattle_dt_compat[] __initconst =
>> +{
>> +    "amd,seattle",
>> +    NULL
>> +};
>> +
>> +/* Seattle firmware only implements PSCI handler for
>> + * system off and system reset at this point.
>> + * This is temporary until full PSCI-0.2 is supported.
>> + * Then, these function will be removed.
>> + */
>> +static noinline void seattle_smc_psci(register_t func_id)
>> +{
>> +    asm volatile(
>> +        "smc #0"
>> +        : "+r" (func_id)
>> +        :);
>> +}
>
> We already have multiple implementation of smc in different place. Can
> we provide a common function rather than adding another one?

The only place I found this is in the arch/arm/psci.c, which is used 
mainly for the PSCI stuff. I can declare that one as non-static, and use 
it here in the seattle.c.

Suravee

Re: [V2 3/3] amd/seattle: Initial revision of AMD Seattle support

[Suravee Suthikulpanit]

On 10/02/2014 02:04 PM, Suravee Suthikulpanit wrote:
>>> +#include <asm/psci.h>
>>> +
>>> +static const char * const seattle_dt_compat[] __initconst =
>>> +{
>>> +    "amd,seattle",
>>> +    NULL
>>> +};
>>> +
>>> +/* Seattle firmware only implements PSCI handler for
>>> + * system off and system reset at this point.
>>> + * This is temporary until full PSCI-0.2 is supported.
>>> + * Then, these function will be removed.
>>> + */
>>> +static noinline void seattle_smc_psci(register_t func_id)
>>> +{
>>> +    asm volatile(
>>> +        "smc #0"
>>> +        : "+r" (func_id)
>>> +        :);
>>> +}
>>
>> We already have multiple implementation of smc in different place. Can
>> we provide a common function rather than adding another one?
>
> The only place I found this is in the arch/arm/psci.c, which is used
> mainly for the PSCI stuff. I can declare that one as non-static, and use
> it here in the seattle.c.
>
> Suravee

Julien,

Actually, think about this again, I would rather keep this independent 
from the other PSCI patch series, which might not make it into 4.5.  It 
is small enough and probably not too terrible to keep this.

Thanks,

Suravee

Re: [V2 3/3] amd/seattle: Initial revision of AMD Seattle support

[Ian Campbell]

On Thu, 2014-10-02 at 15:37 -0500, Suravee Suthikulpanit wrote:
> >>> + * This is temporary until full PSCI-0.2 is supported.
> >>> + * Then, these function will be removed.

> Actually, think about this again, I would rather keep this independent 
> from the other PSCI patch series, which might not make it into 4.5.  It 
> is small enough and probably not too terrible to keep this.

Given the comment about the temporary nature it is IMHO fine to keep
this as is until proper PSCI-0.2 support lands.

Ian.

Re: [V2 3/3] amd/seattle: Initial revision of AMD Seattle support

[Julien Grall]

On 02/10/2014 21:37, Suravee Suthikulpanit wrote:
>
>
> On 10/02/2014 02:04 PM, Suravee Suthikulpanit wrote:
>>>> +#include <asm/psci.h>
>>>> +
>>>> +static const char * const seattle_dt_compat[] __initconst =
>>>> +{
>>>> +    "amd,seattle",
>>>> +    NULL
>>>> +};
>>>> +
>>>> +/* Seattle firmware only implements PSCI handler for
>>>> + * system off and system reset at this point.
>>>> + * This is temporary until full PSCI-0.2 is supported.
>>>> + * Then, these function will be removed.
>>>> + */
>>>> +static noinline void seattle_smc_psci(register_t func_id)
>>>> +{
>>>> +    asm volatile(
>>>> +        "smc #0"
>>>> +        : "+r" (func_id)
>>>> +        :);
>>>> +}
>>>
>>> We already have multiple implementation of smc in different place. Can
>>> we provide a common function rather than adding another one?
>>
>> The only place I found this is in the arch/arm/psci.c, which is used
>> mainly for the PSCI stuff. I can declare that one as non-static, and use
>> it here in the seattle.c.
>>
>> Suravee
>
> Julien,
>
> Actually, think about this again, I would rather keep this independent
> from the other PSCI patch series, which might not make it into 4.5.  It
> is small enough and probably not too terrible to keep this.

There is also one in xen/arch/arm/platforms/exynos.c (exynos_smc).

Moving this patch could be part of this series and I don't think it 
would be a showstopper for accepting it in 4.5.

Ian, any though?

Regards,

-- 
Julien Grall