Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Wed, 2015-08-05 at 10:44 +0100, Russell King - ARM Linux wrote: > On Tue, Aug 04, 2015 at 09:54:21PM +0800, Scott Shu wrote: > > Add support for cpu enable-method "mediatek,mt6580-smp" for booting > > secondary CPUs on MT6580. > > If you have CPU power domain support, and you power up and power down the > CPUs, why do you need the boot_lock and pen_release stuff? Isn't keeping > the power off on a CPU sufficient to prevent it entering the kernel? > Thanks Russell. Yes, the system works well after removing boot_lock and pen_release. Will be fixed in next version. Scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Thu, 2015-08-06 at 08:19 +0200, Sascha Hauer wrote:
> On Wed, Aug 05, 2015 at 06:47:03PM +0200, Matthias Brugger wrote:
> > On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> > > Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> > > secondary CPUs on MT6580.
> > >
> > > Signed-off-by: Scott Shu
> > > ---
> > > arch/arm/mach-mediatek/platsmp.c | 137
> > > ++ 1 file changed, 137 insertions(+)
> > > +
> > > +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> > > +{
> > > + static void __iomem *infracfg_ao_base;
> > > +
> > > + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> > > + if (!infracfg_ao_base) {
> > > + pr_err("%s: Unable to map I/O memory\n", __func__);
> > > + return;
> > > + }
> > > +
> > > + /* Enable bootrom power down mode */
> > > + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> > > +infracfg_ao_base + 0x804);
> >
> > Please use a define for the offset.
> > I prefer to not cascade reads and writes but to use a temporary variable.
> > But
> > if this is fine with the kernel coding style (I doubt, but I'm not sure)
> > then
> > this is fine.
>
> For what it's worth I also prefer
>
> val = readl(infracfg_ao_base + 0x804);
> val |= SW_ROM_PD:
> writel_relaxed(val, infracfg_ao_base + 0x804);
>
> I find this better readable. I don't think though that there's common
> agreement that this style is better.
>
> Sascha
>
Fixed in next version. Thanks.
Scott
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Wed, 2015-08-05 at 18:47 +0200, Matthias Brugger wrote:
> On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> > Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> > secondary CPUs on MT6580.
> >
> > Signed-off-by: Scott Shu
> > ---
> > arch/arm/mach-mediatek/platsmp.c | 137
> > ++ 1 file changed, 137 insertions(+)
> >
> > diff --git a/arch/arm/mach-mediatek/platsmp.c
> > b/arch/arm/mach-mediatek/platsmp.c index a5bc108..94f7865 100644
> > --- a/arch/arm/mach-mediatek/platsmp.c
> > +++ b/arch/arm/mach-mediatek/platsmp.c
> > @@ -21,10 +21,16 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > +
> > +#include
> >
> > #define MTK_MAX_CPU8
> > #define MTK_SMP_REG_SIZE 0x1000
> >
> > +static DEFINE_SPINLOCK(boot_lock);
> > +
> > struct mtk_smp_boot_info {
> > unsigned long smp_base;
> > unsigned int jump_reg;
> > @@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
> > __initconst = { static void __iomem *mtk_smp_base;
> > static const struct mtk_smp_boot_info *mtk_smp_info;
> >
> > +#ifdef CONFIG_HOTPLUG_CPU
> > +static int mt6580_cpu_kill(unsigned cpu)
> > +{
> > + int ret;
> > +
> > + ret = spm_cpu_mtcmos_off(cpu, 1);
>
> is this function ever called with wfi == 0?
Yes, wfi == 0 at MT6582 "special" dual-core platform, but for MT6580,
wfi is always 1.
> > + if (ret < 0)
> > + return 0;
> > +
> > + return 1;
> > +}
> > +
> > +static void mt6580_cpu_die(unsigned int cpu)
> > +{
> > + for (;;)
> > + cpu_do_idle();
> > +}
> > +#endif
> > +
> > +static void write_pen_release(int val)
> > +{
> > + pen_release = val;
> > + /* Make sure this is visible to other CPUs */
> > + smp_wmb();
> > + sync_cache_w(_release);
> > +}
> > +
> > +/*
> > + * Refer common "pen" secondary release method
> > + */
> > +static int mt6580_boot_secondary(unsigned int cpu, struct task_struct
> > *idle) +{
> > + unsigned long timeout;
> > + int ret;
> > +
> > + /*
> > +* Set synchronisation state between this boot processor
> > +* and the secondary one
> > +*/
> > + spin_lock(_lock);
> > +
> > + /*
> > +* The secondary processor is waiting to be released from
> > +* the holding pen - release it, then wait for it to flag
> > +* that it has been released by resetting pen_release.
> > +*
> > +* Note that "pen_release" is the hardware CPU ID, whereas
> > +* "cpu" is Linux's internal ID.
> > +*/
> > + write_pen_release(cpu);
> > +
> > + /*
> > +* CPU power on control by SPM
> > +*/
> > + ret = spm_cpu_mtcmos_on(cpu);
> > + if (ret < 0) {
> > + spin_unlock(_lock);
> > + return -ENXIO;
> > + }
> > +
> > + timeout = jiffies + (1 * HZ);
> > + while (time_before(jiffies, timeout)) {
> > + /* Read barrier */
> > + smp_rmb();
> > +
> > + if (pen_release == -1)
> > + break;
> > +
> > + udelay(10);
> > + }
> > +
> > + /*
> > +* Now the secondary core is starting up let it run its
> > +* calibrations, then wait for it to finish
> > +*/
> > + spin_unlock(_lock);
> > +
> > + return (pen_release != -1 ? -ENXIO : 0);
> > +}
> > +
> > +static void mt6580_secondary_init(unsigned int cpu)
> > +{
> > + /*
> > +* Let the primary processor know we're out of the
> > +* pen, then head off into the C entry point
> > +*/
> > + write_pen_release(-1);
> > +
> > + /*
> > +* Synchronise with the boot thread.
> > +*/
> > + spin_lock(_lock);
> > + spin_unlock(_lock);
> > +}
> > +
> > +#define MT6580_INFRACFG_AO 0x10001000
> > +#define SW_ROM_PD BIT(31)
>
> Please put the defines on the beginning of the file.
> SW_ROM_PD bit is the same on other SoCs? I wasn't able to reveal that from
> the
> data sheets. If this is MT6580 only, the define should be renamed.
>
OK, fixed in next version.
> > +
> > +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > + static void __iomem *infracfg_ao_base;
> > +
> > + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> > + if (!infracfg_ao_base) {
> > + pr_err("%s: Unable to map I/O memory\n", __func__);
> > + return;
> > + }
> > +
> > + /* Enable bootrom power down mode */
> > + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> > + infracfg_ao_base + 0x804);
>
> Please use a define for the offset.
> I prefer to not cascade reads and writes but to use a temporary variable. But
> if this is fine with the kernel coding style (I doubt, but I'm not sure) then
> this is fine.
>
OK, fixed in next version.
Thanks,
Scott
> > +
> > + /* Write the address of slave startup into boot address
> > + register for bootrom power down mode */
> > + writel_relaxed(virt_to_phys(secondary_startup_arm),
> > + infracfg_ao_base + 0x800);
>
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Wed, Aug 05, 2015 at 06:47:03PM +0200, Matthias Brugger wrote:
> On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> > Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> > secondary CPUs on MT6580.
> >
> > Signed-off-by: Scott Shu
> > ---
> > arch/arm/mach-mediatek/platsmp.c | 137
> > ++ 1 file changed, 137 insertions(+)
> > +
> > +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> > +{
> > + static void __iomem *infracfg_ao_base;
> > +
> > + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> > + if (!infracfg_ao_base) {
> > + pr_err("%s: Unable to map I/O memory\n", __func__);
> > + return;
> > + }
> > +
> > + /* Enable bootrom power down mode */
> > + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> > + infracfg_ao_base + 0x804);
>
> Please use a define for the offset.
> I prefer to not cascade reads and writes but to use a temporary variable. But
> if this is fine with the kernel coding style (I doubt, but I'm not sure) then
> this is fine.
For what it's worth I also prefer
val = readl(infracfg_ao_base + 0x804);
val |= SW_ROM_PD:
writel_relaxed(val, infracfg_ao_base + 0x804);
I find this better readable. I don't think though that there's common
agreement that this style is better.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Wed, 2015-08-05 at 10:44 +0100, Russell King - ARM Linux wrote: On Tue, Aug 04, 2015 at 09:54:21PM +0800, Scott Shu wrote: Add support for cpu enable-method mediatek,mt6580-smp for booting secondary CPUs on MT6580. If you have CPU power domain support, and you power up and power down the CPUs, why do you need the boot_lock and pen_release stuff? Isn't keeping the power off on a CPU sufficient to prevent it entering the kernel? Thanks Russell. Yes, the system works well after removing boot_lock and pen_release. Will be fixed in next version. Scott -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Thu, 2015-08-06 at 08:19 +0200, Sascha Hauer wrote:
On Wed, Aug 05, 2015 at 06:47:03PM +0200, Matthias Brugger wrote:
On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
Add support for cpu enable-method mediatek,mt6580-smp for booting
secondary CPUs on MT6580.
Signed-off-by: Scott Shu scott@mediatek.com
---
arch/arm/mach-mediatek/platsmp.c | 137
++ 1 file changed, 137 insertions(+)
+
+static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
+{
+ static void __iomem *infracfg_ao_base;
+
+ infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
+ if (!infracfg_ao_base) {
+ pr_err(%s: Unable to map I/O memory\n, __func__);
+ return;
+ }
+
+ /* Enable bootrom power down mode */
+ writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
+infracfg_ao_base + 0x804);
Please use a define for the offset.
I prefer to not cascade reads and writes but to use a temporary variable.
But
if this is fine with the kernel coding style (I doubt, but I'm not sure)
then
this is fine.
For what it's worth I also prefer
val = readl(infracfg_ao_base + 0x804);
val |= SW_ROM_PD:
writel_relaxed(val, infracfg_ao_base + 0x804);
I find this better readable. I don't think though that there's common
agreement that this style is better.
Sascha
Fixed in next version. Thanks.
Scott
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Wed, Aug 05, 2015 at 06:47:03PM +0200, Matthias Brugger wrote:
On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
Add support for cpu enable-method mediatek,mt6580-smp for booting
secondary CPUs on MT6580.
Signed-off-by: Scott Shu scott@mediatek.com
---
arch/arm/mach-mediatek/platsmp.c | 137
++ 1 file changed, 137 insertions(+)
+
+static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
+{
+ static void __iomem *infracfg_ao_base;
+
+ infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
+ if (!infracfg_ao_base) {
+ pr_err(%s: Unable to map I/O memory\n, __func__);
+ return;
+ }
+
+ /* Enable bootrom power down mode */
+ writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
+ infracfg_ao_base + 0x804);
Please use a define for the offset.
I prefer to not cascade reads and writes but to use a temporary variable. But
if this is fine with the kernel coding style (I doubt, but I'm not sure) then
this is fine.
For what it's worth I also prefer
val = readl(infracfg_ao_base + 0x804);
val |= SW_ROM_PD:
writel_relaxed(val, infracfg_ao_base + 0x804);
I find this better readable. I don't think though that there's common
agreement that this style is better.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Wed, 2015-08-05 at 18:47 +0200, Matthias Brugger wrote:
On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
Add support for cpu enable-method mediatek,mt6580-smp for booting
secondary CPUs on MT6580.
Signed-off-by: Scott Shu scott@mediatek.com
---
arch/arm/mach-mediatek/platsmp.c | 137
++ 1 file changed, 137 insertions(+)
diff --git a/arch/arm/mach-mediatek/platsmp.c
b/arch/arm/mach-mediatek/platsmp.c index a5bc108..94f7865 100644
--- a/arch/arm/mach-mediatek/platsmp.c
+++ b/arch/arm/mach-mediatek/platsmp.c
@@ -21,10 +21,16 @@
#include linux/of_address.h
#include linux/string.h
#include linux/threads.h
+#include linux/delay.h
+#include asm/cacheflush.h
+
+#include linux/soc/mediatek/scpsys.h
#define MTK_MAX_CPU8
#define MTK_SMP_REG_SIZE 0x1000
+static DEFINE_SPINLOCK(boot_lock);
+
struct mtk_smp_boot_info {
unsigned long smp_base;
unsigned int jump_reg;
@@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
__initconst = { static void __iomem *mtk_smp_base;
static const struct mtk_smp_boot_info *mtk_smp_info;
+#ifdef CONFIG_HOTPLUG_CPU
+static int mt6580_cpu_kill(unsigned cpu)
+{
+ int ret;
+
+ ret = spm_cpu_mtcmos_off(cpu, 1);
is this function ever called with wfi == 0?
Yes, wfi == 0 at MT6582 special dual-core platform, but for MT6580,
wfi is always 1.
+ if (ret 0)
+ return 0;
+
+ return 1;
+}
+
+static void mt6580_cpu_die(unsigned int cpu)
+{
+ for (;;)
+ cpu_do_idle();
+}
+#endif
+
+static void write_pen_release(int val)
+{
+ pen_release = val;
+ /* Make sure this is visible to other CPUs */
+ smp_wmb();
+ sync_cache_w(pen_release);
+}
+
+/*
+ * Refer common pen secondary release method
+ */
+static int mt6580_boot_secondary(unsigned int cpu, struct task_struct
*idle) +{
+ unsigned long timeout;
+ int ret;
+
+ /*
+* Set synchronisation state between this boot processor
+* and the secondary one
+*/
+ spin_lock(boot_lock);
+
+ /*
+* The secondary processor is waiting to be released from
+* the holding pen - release it, then wait for it to flag
+* that it has been released by resetting pen_release.
+*
+* Note that pen_release is the hardware CPU ID, whereas
+* cpu is Linux's internal ID.
+*/
+ write_pen_release(cpu);
+
+ /*
+* CPU power on control by SPM
+*/
+ ret = spm_cpu_mtcmos_on(cpu);
+ if (ret 0) {
+ spin_unlock(boot_lock);
+ return -ENXIO;
+ }
+
+ timeout = jiffies + (1 * HZ);
+ while (time_before(jiffies, timeout)) {
+ /* Read barrier */
+ smp_rmb();
+
+ if (pen_release == -1)
+ break;
+
+ udelay(10);
+ }
+
+ /*
+* Now the secondary core is starting up let it run its
+* calibrations, then wait for it to finish
+*/
+ spin_unlock(boot_lock);
+
+ return (pen_release != -1 ? -ENXIO : 0);
+}
+
+static void mt6580_secondary_init(unsigned int cpu)
+{
+ /*
+* Let the primary processor know we're out of the
+* pen, then head off into the C entry point
+*/
+ write_pen_release(-1);
+
+ /*
+* Synchronise with the boot thread.
+*/
+ spin_lock(boot_lock);
+ spin_unlock(boot_lock);
+}
+
+#define MT6580_INFRACFG_AO 0x10001000
+#define SW_ROM_PD BIT(31)
Please put the defines on the beginning of the file.
SW_ROM_PD bit is the same on other SoCs? I wasn't able to reveal that from
the
data sheets. If this is MT6580 only, the define should be renamed.
OK, fixed in next version.
+
+static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
+{
+ static void __iomem *infracfg_ao_base;
+
+ infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
+ if (!infracfg_ao_base) {
+ pr_err(%s: Unable to map I/O memory\n, __func__);
+ return;
+ }
+
+ /* Enable bootrom power down mode */
+ writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
+ infracfg_ao_base + 0x804);
Please use a define for the offset.
I prefer to not cascade reads and writes but to use a temporary variable. But
if this is fine with the kernel coding style (I doubt, but I'm not sure) then
this is fine.
OK, fixed in next version.
Thanks,
Scott
+
+ /* Write the address of slave startup into boot address
+ register for bootrom power down mode */
+ writel_relaxed(virt_to_phys(secondary_startup_arm),
+ infracfg_ao_base + 0x800);
+
+ iounmap(infracfg_ao_base);
+}
+
static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
if (!mtk_smp_base)
@@ -142,3 +268,14 @@
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
> Add support for cpu enable-method "mediatek,mt6580-smp" for booting
> secondary CPUs on MT6580.
>
> Signed-off-by: Scott Shu
> ---
> arch/arm/mach-mediatek/platsmp.c | 137
> ++ 1 file changed, 137 insertions(+)
>
> diff --git a/arch/arm/mach-mediatek/platsmp.c
> b/arch/arm/mach-mediatek/platsmp.c index a5bc108..94f7865 100644
> --- a/arch/arm/mach-mediatek/platsmp.c
> +++ b/arch/arm/mach-mediatek/platsmp.c
> @@ -21,10 +21,16 @@
> #include
> #include
> #include
> +#include
> +#include
> +
> +#include
>
> #define MTK_MAX_CPU 8
> #define MTK_SMP_REG_SIZE 0x1000
>
> +static DEFINE_SPINLOCK(boot_lock);
> +
> struct mtk_smp_boot_info {
> unsigned long smp_base;
> unsigned int jump_reg;
> @@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
> __initconst = { static void __iomem *mtk_smp_base;
> static const struct mtk_smp_boot_info *mtk_smp_info;
>
> +#ifdef CONFIG_HOTPLUG_CPU
> +static int mt6580_cpu_kill(unsigned cpu)
> +{
> + int ret;
> +
> + ret = spm_cpu_mtcmos_off(cpu, 1);
is this function ever called with wfi == 0?
> + if (ret < 0)
> + return 0;
> +
> + return 1;
> +}
> +
> +static void mt6580_cpu_die(unsigned int cpu)
> +{
> + for (;;)
> + cpu_do_idle();
> +}
> +#endif
> +
> +static void write_pen_release(int val)
> +{
> + pen_release = val;
> + /* Make sure this is visible to other CPUs */
> + smp_wmb();
> + sync_cache_w(_release);
> +}
> +
> +/*
> + * Refer common "pen" secondary release method
> + */
> +static int mt6580_boot_secondary(unsigned int cpu, struct task_struct
> *idle) +{
> + unsigned long timeout;
> + int ret;
> +
> + /*
> + * Set synchronisation state between this boot processor
> + * and the secondary one
> + */
> + spin_lock(_lock);
> +
> + /*
> + * The secondary processor is waiting to be released from
> + * the holding pen - release it, then wait for it to flag
> + * that it has been released by resetting pen_release.
> + *
> + * Note that "pen_release" is the hardware CPU ID, whereas
> + * "cpu" is Linux's internal ID.
> + */
> + write_pen_release(cpu);
> +
> + /*
> + * CPU power on control by SPM
> + */
> + ret = spm_cpu_mtcmos_on(cpu);
> + if (ret < 0) {
> + spin_unlock(_lock);
> + return -ENXIO;
> + }
> +
> + timeout = jiffies + (1 * HZ);
> + while (time_before(jiffies, timeout)) {
> + /* Read barrier */
> + smp_rmb();
> +
> + if (pen_release == -1)
> + break;
> +
> + udelay(10);
> + }
> +
> + /*
> + * Now the secondary core is starting up let it run its
> + * calibrations, then wait for it to finish
> + */
> + spin_unlock(_lock);
> +
> + return (pen_release != -1 ? -ENXIO : 0);
> +}
> +
> +static void mt6580_secondary_init(unsigned int cpu)
> +{
> + /*
> + * Let the primary processor know we're out of the
> + * pen, then head off into the C entry point
> + */
> + write_pen_release(-1);
> +
> + /*
> + * Synchronise with the boot thread.
> + */
> + spin_lock(_lock);
> + spin_unlock(_lock);
> +}
> +
> +#define MT6580_INFRACFG_AO 0x10001000
> +#define SW_ROM_PDBIT(31)
Please put the defines on the beginning of the file.
SW_ROM_PD bit is the same on other SoCs? I wasn't able to reveal that from the
data sheets. If this is MT6580 only, the define should be renamed.
> +
> +static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
> +{
> + static void __iomem *infracfg_ao_base;
> +
> + infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
> + if (!infracfg_ao_base) {
> + pr_err("%s: Unable to map I/O memory\n", __func__);
> + return;
> + }
> +
> + /* Enable bootrom power down mode */
> + writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
> +infracfg_ao_base + 0x804);
Please use a define for the offset.
I prefer to not cascade reads and writes but to use a temporary variable. But
if this is fine with the kernel coding style (I doubt, but I'm not sure) then
this is fine.
> +
> + /* Write the address of slave startup into boot address
> +register for bootrom power down mode */
> + writel_relaxed(virt_to_phys(secondary_startup_arm),
> +infracfg_ao_base + 0x800);
> +
> + iounmap(infracfg_ao_base);
> +}
> +
> static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
> {
> if (!mtk_smp_base)
> @@ -142,3 +268,14 @@ static struct smp_operations mt6589_smp_ops __initdata
> = { .smp_boot_secondary = mtk_boot_secondary,
> };
> CPU_METHOD_OF_DECLARE(mt6589_smp, "mediatek,mt6589-smp", _smp_ops);
> +
> +static struct smp_operations
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Tue, Aug 04, 2015 at 09:54:21PM +0800, Scott Shu wrote: > Add support for cpu enable-method "mediatek,mt6580-smp" for booting > secondary CPUs on MT6580. If you have CPU power domain support, and you power up and power down the CPUs, why do you need the boot_lock and pen_release stuff? Isn't keeping the power off on a CPU sufficient to prevent it entering the kernel? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Tue, Aug 04, 2015 at 09:54:21PM +0800, Scott Shu wrote: Add support for cpu enable-method mediatek,mt6580-smp for booting secondary CPUs on MT6580. If you have CPU power domain support, and you power up and power down the CPUs, why do you need the boot_lock and pen_release stuff? Isn't keeping the power off on a CPU sufficient to prevent it entering the kernel? -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
On Tuesday, August 04, 2015 09:54:21 PM Scott Shu wrote:
Add support for cpu enable-method mediatek,mt6580-smp for booting
secondary CPUs on MT6580.
Signed-off-by: Scott Shu scott@mediatek.com
---
arch/arm/mach-mediatek/platsmp.c | 137
++ 1 file changed, 137 insertions(+)
diff --git a/arch/arm/mach-mediatek/platsmp.c
b/arch/arm/mach-mediatek/platsmp.c index a5bc108..94f7865 100644
--- a/arch/arm/mach-mediatek/platsmp.c
+++ b/arch/arm/mach-mediatek/platsmp.c
@@ -21,10 +21,16 @@
#include linux/of_address.h
#include linux/string.h
#include linux/threads.h
+#include linux/delay.h
+#include asm/cacheflush.h
+
+#include linux/soc/mediatek/scpsys.h
#define MTK_MAX_CPU 8
#define MTK_SMP_REG_SIZE 0x1000
+static DEFINE_SPINLOCK(boot_lock);
+
struct mtk_smp_boot_info {
unsigned long smp_base;
unsigned int jump_reg;
@@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
__initconst = { static void __iomem *mtk_smp_base;
static const struct mtk_smp_boot_info *mtk_smp_info;
+#ifdef CONFIG_HOTPLUG_CPU
+static int mt6580_cpu_kill(unsigned cpu)
+{
+ int ret;
+
+ ret = spm_cpu_mtcmos_off(cpu, 1);
is this function ever called with wfi == 0?
+ if (ret 0)
+ return 0;
+
+ return 1;
+}
+
+static void mt6580_cpu_die(unsigned int cpu)
+{
+ for (;;)
+ cpu_do_idle();
+}
+#endif
+
+static void write_pen_release(int val)
+{
+ pen_release = val;
+ /* Make sure this is visible to other CPUs */
+ smp_wmb();
+ sync_cache_w(pen_release);
+}
+
+/*
+ * Refer common pen secondary release method
+ */
+static int mt6580_boot_secondary(unsigned int cpu, struct task_struct
*idle) +{
+ unsigned long timeout;
+ int ret;
+
+ /*
+ * Set synchronisation state between this boot processor
+ * and the secondary one
+ */
+ spin_lock(boot_lock);
+
+ /*
+ * The secondary processor is waiting to be released from
+ * the holding pen - release it, then wait for it to flag
+ * that it has been released by resetting pen_release.
+ *
+ * Note that pen_release is the hardware CPU ID, whereas
+ * cpu is Linux's internal ID.
+ */
+ write_pen_release(cpu);
+
+ /*
+ * CPU power on control by SPM
+ */
+ ret = spm_cpu_mtcmos_on(cpu);
+ if (ret 0) {
+ spin_unlock(boot_lock);
+ return -ENXIO;
+ }
+
+ timeout = jiffies + (1 * HZ);
+ while (time_before(jiffies, timeout)) {
+ /* Read barrier */
+ smp_rmb();
+
+ if (pen_release == -1)
+ break;
+
+ udelay(10);
+ }
+
+ /*
+ * Now the secondary core is starting up let it run its
+ * calibrations, then wait for it to finish
+ */
+ spin_unlock(boot_lock);
+
+ return (pen_release != -1 ? -ENXIO : 0);
+}
+
+static void mt6580_secondary_init(unsigned int cpu)
+{
+ /*
+ * Let the primary processor know we're out of the
+ * pen, then head off into the C entry point
+ */
+ write_pen_release(-1);
+
+ /*
+ * Synchronise with the boot thread.
+ */
+ spin_lock(boot_lock);
+ spin_unlock(boot_lock);
+}
+
+#define MT6580_INFRACFG_AO 0x10001000
+#define SW_ROM_PDBIT(31)
Please put the defines on the beginning of the file.
SW_ROM_PD bit is the same on other SoCs? I wasn't able to reveal that from the
data sheets. If this is MT6580 only, the define should be renamed.
+
+static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
+{
+ static void __iomem *infracfg_ao_base;
+
+ infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
+ if (!infracfg_ao_base) {
+ pr_err(%s: Unable to map I/O memory\n, __func__);
+ return;
+ }
+
+ /* Enable bootrom power down mode */
+ writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
+infracfg_ao_base + 0x804);
Please use a define for the offset.
I prefer to not cascade reads and writes but to use a temporary variable. But
if this is fine with the kernel coding style (I doubt, but I'm not sure) then
this is fine.
+
+ /* Write the address of slave startup into boot address
+register for bootrom power down mode */
+ writel_relaxed(virt_to_phys(secondary_startup_arm),
+infracfg_ao_base + 0x800);
+
+ iounmap(infracfg_ao_base);
+}
+
static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
if (!mtk_smp_base)
@@ -142,3 +268,14 @@ static struct smp_operations mt6589_smp_ops __initdata
= { .smp_boot_secondary = mtk_boot_secondary,
};
CPU_METHOD_OF_DECLARE(mt6589_smp, mediatek,mt6589-smp, mt6589_smp_ops);
+
+static struct smp_operations mt6580_smp_ops __initdata = {
[PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
Add support for cpu enable-method "mediatek,mt6580-smp" for booting
secondary CPUs on MT6580.
Signed-off-by: Scott Shu
---
arch/arm/mach-mediatek/platsmp.c | 137 ++
1 file changed, 137 insertions(+)
diff --git a/arch/arm/mach-mediatek/platsmp.c b/arch/arm/mach-mediatek/platsmp.c
index a5bc108..94f7865 100644
--- a/arch/arm/mach-mediatek/platsmp.c
+++ b/arch/arm/mach-mediatek/platsmp.c
@@ -21,10 +21,16 @@
#include
#include
#include
+#include
+#include
+
+#include
#define MTK_MAX_CPU8
#define MTK_SMP_REG_SIZE 0x1000
+static DEFINE_SPINLOCK(boot_lock);
+
struct mtk_smp_boot_info {
unsigned long smp_base;
unsigned int jump_reg;
@@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
__initconst = {
static void __iomem *mtk_smp_base;
static const struct mtk_smp_boot_info *mtk_smp_info;
+#ifdef CONFIG_HOTPLUG_CPU
+static int mt6580_cpu_kill(unsigned cpu)
+{
+ int ret;
+
+ ret = spm_cpu_mtcmos_off(cpu, 1);
+ if (ret < 0)
+ return 0;
+
+ return 1;
+}
+
+static void mt6580_cpu_die(unsigned int cpu)
+{
+ for (;;)
+ cpu_do_idle();
+}
+#endif
+
+static void write_pen_release(int val)
+{
+ pen_release = val;
+ /* Make sure this is visible to other CPUs */
+ smp_wmb();
+ sync_cache_w(_release);
+}
+
+/*
+ * Refer common "pen" secondary release method
+ */
+static int mt6580_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ unsigned long timeout;
+ int ret;
+
+ /*
+* Set synchronisation state between this boot processor
+* and the secondary one
+*/
+ spin_lock(_lock);
+
+ /*
+* The secondary processor is waiting to be released from
+* the holding pen - release it, then wait for it to flag
+* that it has been released by resetting pen_release.
+*
+* Note that "pen_release" is the hardware CPU ID, whereas
+* "cpu" is Linux's internal ID.
+*/
+ write_pen_release(cpu);
+
+ /*
+* CPU power on control by SPM
+*/
+ ret = spm_cpu_mtcmos_on(cpu);
+ if (ret < 0) {
+ spin_unlock(_lock);
+ return -ENXIO;
+ }
+
+ timeout = jiffies + (1 * HZ);
+ while (time_before(jiffies, timeout)) {
+ /* Read barrier */
+ smp_rmb();
+
+ if (pen_release == -1)
+ break;
+
+ udelay(10);
+ }
+
+ /*
+* Now the secondary core is starting up let it run its
+* calibrations, then wait for it to finish
+*/
+ spin_unlock(_lock);
+
+ return (pen_release != -1 ? -ENXIO : 0);
+}
+
+static void mt6580_secondary_init(unsigned int cpu)
+{
+ /*
+* Let the primary processor know we're out of the
+* pen, then head off into the C entry point
+*/
+ write_pen_release(-1);
+
+ /*
+* Synchronise with the boot thread.
+*/
+ spin_lock(_lock);
+ spin_unlock(_lock);
+}
+
+#define MT6580_INFRACFG_AO 0x10001000
+#define SW_ROM_PD BIT(31)
+
+static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
+{
+ static void __iomem *infracfg_ao_base;
+
+ infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
+ if (!infracfg_ao_base) {
+ pr_err("%s: Unable to map I/O memory\n", __func__);
+ return;
+ }
+
+ /* Enable bootrom power down mode */
+ writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
+ infracfg_ao_base + 0x804);
+
+ /* Write the address of slave startup into boot address
+ register for bootrom power down mode */
+ writel_relaxed(virt_to_phys(secondary_startup_arm),
+ infracfg_ao_base + 0x800);
+
+ iounmap(infracfg_ao_base);
+}
+
static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
if (!mtk_smp_base)
@@ -142,3 +268,14 @@ static struct smp_operations mt6589_smp_ops __initdata = {
.smp_boot_secondary = mtk_boot_secondary,
};
CPU_METHOD_OF_DECLARE(mt6589_smp, "mediatek,mt6589-smp", _smp_ops);
+
+static struct smp_operations mt6580_smp_ops __initdata = {
+ .smp_prepare_cpus = mt6580_smp_prepare_cpus,
+ .smp_secondary_init = mt6580_secondary_init,
+ .smp_boot_secondary = mt6580_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_kill = mt6580_cpu_kill,
+ .cpu_die = mt6580_cpu_die,
+#endif
+};
+CPU_METHOD_OF_DECLARE(mt6580_smp, "mediatek,mt6580-smp", _smp_ops);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 5/8] ARM: mediatek: add smp bringup code for MT6580
Add support for cpu enable-method mediatek,mt6580-smp for booting
secondary CPUs on MT6580.
Signed-off-by: Scott Shu scott@mediatek.com
---
arch/arm/mach-mediatek/platsmp.c | 137 ++
1 file changed, 137 insertions(+)
diff --git a/arch/arm/mach-mediatek/platsmp.c b/arch/arm/mach-mediatek/platsmp.c
index a5bc108..94f7865 100644
--- a/arch/arm/mach-mediatek/platsmp.c
+++ b/arch/arm/mach-mediatek/platsmp.c
@@ -21,10 +21,16 @@
#include linux/of_address.h
#include linux/string.h
#include linux/threads.h
+#include linux/delay.h
+#include asm/cacheflush.h
+
+#include linux/soc/mediatek/scpsys.h
#define MTK_MAX_CPU8
#define MTK_SMP_REG_SIZE 0x1000
+static DEFINE_SPINLOCK(boot_lock);
+
struct mtk_smp_boot_info {
unsigned long smp_base;
unsigned int jump_reg;
@@ -56,6 +62,126 @@ static const struct of_device_id mtk_smp_boot_infos[]
__initconst = {
static void __iomem *mtk_smp_base;
static const struct mtk_smp_boot_info *mtk_smp_info;
+#ifdef CONFIG_HOTPLUG_CPU
+static int mt6580_cpu_kill(unsigned cpu)
+{
+ int ret;
+
+ ret = spm_cpu_mtcmos_off(cpu, 1);
+ if (ret 0)
+ return 0;
+
+ return 1;
+}
+
+static void mt6580_cpu_die(unsigned int cpu)
+{
+ for (;;)
+ cpu_do_idle();
+}
+#endif
+
+static void write_pen_release(int val)
+{
+ pen_release = val;
+ /* Make sure this is visible to other CPUs */
+ smp_wmb();
+ sync_cache_w(pen_release);
+}
+
+/*
+ * Refer common pen secondary release method
+ */
+static int mt6580_boot_secondary(unsigned int cpu, struct task_struct *idle)
+{
+ unsigned long timeout;
+ int ret;
+
+ /*
+* Set synchronisation state between this boot processor
+* and the secondary one
+*/
+ spin_lock(boot_lock);
+
+ /*
+* The secondary processor is waiting to be released from
+* the holding pen - release it, then wait for it to flag
+* that it has been released by resetting pen_release.
+*
+* Note that pen_release is the hardware CPU ID, whereas
+* cpu is Linux's internal ID.
+*/
+ write_pen_release(cpu);
+
+ /*
+* CPU power on control by SPM
+*/
+ ret = spm_cpu_mtcmos_on(cpu);
+ if (ret 0) {
+ spin_unlock(boot_lock);
+ return -ENXIO;
+ }
+
+ timeout = jiffies + (1 * HZ);
+ while (time_before(jiffies, timeout)) {
+ /* Read barrier */
+ smp_rmb();
+
+ if (pen_release == -1)
+ break;
+
+ udelay(10);
+ }
+
+ /*
+* Now the secondary core is starting up let it run its
+* calibrations, then wait for it to finish
+*/
+ spin_unlock(boot_lock);
+
+ return (pen_release != -1 ? -ENXIO : 0);
+}
+
+static void mt6580_secondary_init(unsigned int cpu)
+{
+ /*
+* Let the primary processor know we're out of the
+* pen, then head off into the C entry point
+*/
+ write_pen_release(-1);
+
+ /*
+* Synchronise with the boot thread.
+*/
+ spin_lock(boot_lock);
+ spin_unlock(boot_lock);
+}
+
+#define MT6580_INFRACFG_AO 0x10001000
+#define SW_ROM_PD BIT(31)
+
+static void __init mt6580_smp_prepare_cpus(unsigned int max_cpus)
+{
+ static void __iomem *infracfg_ao_base;
+
+ infracfg_ao_base = ioremap(MT6580_INFRACFG_AO, 0x1000);
+ if (!infracfg_ao_base) {
+ pr_err(%s: Unable to map I/O memory\n, __func__);
+ return;
+ }
+
+ /* Enable bootrom power down mode */
+ writel_relaxed(readl(infracfg_ao_base + 0x804) | SW_ROM_PD,
+ infracfg_ao_base + 0x804);
+
+ /* Write the address of slave startup into boot address
+ register for bootrom power down mode */
+ writel_relaxed(virt_to_phys(secondary_startup_arm),
+ infracfg_ao_base + 0x800);
+
+ iounmap(infracfg_ao_base);
+}
+
static int mtk_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
if (!mtk_smp_base)
@@ -142,3 +268,14 @@ static struct smp_operations mt6589_smp_ops __initdata = {
.smp_boot_secondary = mtk_boot_secondary,
};
CPU_METHOD_OF_DECLARE(mt6589_smp, mediatek,mt6589-smp, mt6589_smp_ops);
+
+static struct smp_operations mt6580_smp_ops __initdata = {
+ .smp_prepare_cpus = mt6580_smp_prepare_cpus,
+ .smp_secondary_init = mt6580_secondary_init,
+ .smp_boot_secondary = mt6580_boot_secondary,
+#ifdef CONFIG_HOTPLUG_CPU
+ .cpu_kill = mt6580_cpu_kill,
+ .cpu_die = mt6580_cpu_die,
+#endif
+};
+CPU_METHOD_OF_DECLARE(mt6580_smp, mediatek,mt6580-smp, mt6580_smp_ops);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info
