From: Rob Herring <[email protected]>
This series converts highbank to use PSCI calls for smp_ops, suspend and
cpuidle. I've dropped system reset and poweroff until we can finalize
PSCI 0.2 bindings. Otherwise, this is mostly unchanged from the previous
posting.
New in this posting is enabling cpuidle on ecx-2000.
Rob Herring (6):
cpuidle: calxeda: add support to use PSCI calls
cpuidle: calxeda: add cpu_pm_enter/exit calls
cpuidle: calxeda: add ecx-2000 support
ARM: PSCI: remove unnecessary include of arm-gic.h
ARM: highbank: adapt to use ARM PSCI calls
dts: calxeda: add ARM PSCI binding
arch/arm/boot/dts/ecx-common.dtsi | 8 +++++
arch/arm/kernel/psci_smp.c | 1 -
arch/arm/mach-highbank/Kconfig | 1 +
arch/arm/mach-highbank/Makefile | 2 --
arch/arm/mach-highbank/core.h | 4 ---
arch/arm/mach-highbank/highbank.c | 16 +--------
arch/arm/mach-highbank/hotplug.c | 37 ---------------------
arch/arm/mach-highbank/platsmp.c | 68 ---------------------------------------
arch/arm/mach-highbank/pm.c | 27 ++++++----------
drivers/cpuidle/Kconfig.arm | 2 +-
drivers/cpuidle/cpuidle-calxeda.c | 46 +++++++-------------------
11 files changed, 32 insertions(+), 180 deletions(-)
delete mode 100644 arch/arm/mach-highbank/hotplug.c
delete mode 100644 arch/arm/mach-highbank/platsmp.c
--
1.8.1.2
From: Rob Herring <[email protected]>
This adapts highbank to use psci for smp_ops and the cpu_suspend function
for suspend/resume.
Signed-off-by: Rob Herring <[email protected]>
---
arch/arm/mach-highbank/Kconfig | 1 +
arch/arm/mach-highbank/Makefile | 2 --
arch/arm/mach-highbank/core.h | 4 ---
arch/arm/mach-highbank/highbank.c | 16 +--------
arch/arm/mach-highbank/hotplug.c | 37 ---------------------
arch/arm/mach-highbank/platsmp.c | 68 ---------------------------------------
arch/arm/mach-highbank/pm.c | 27 ++++++----------
7 files changed, 11 insertions(+), 144 deletions(-)
delete mode 100644 arch/arm/mach-highbank/hotplug.c
delete mode 100644 arch/arm/mach-highbank/platsmp.c
diff --git a/arch/arm/mach-highbank/Kconfig b/arch/arm/mach-highbank/Kconfig
index 8e8437d..965623d 100644
--- a/arch/arm/mach-highbank/Kconfig
+++ b/arch/arm/mach-highbank/Kconfig
@@ -10,6 +10,7 @@ config ARCH_HIGHBANK
select ARM_ERRATA_775420
select ARM_ERRATA_798181
select ARM_GIC
+ select ARM_PSCI
select ARM_TIMER_SP804
select CACHE_L2X0
select CLKDEV_LOOKUP
diff --git a/arch/arm/mach-highbank/Makefile b/arch/arm/mach-highbank/Makefile
index 8a1ef57..55840f4 100644
--- a/arch/arm/mach-highbank/Makefile
+++ b/arch/arm/mach-highbank/Makefile
@@ -3,6 +3,4 @@ obj-y := highbank.o system.o smc.o
plus_sec := $(call as-instr,.arch_extension sec,+sec)
AFLAGS_smc.o :=-Wa,-march=armv7-a$(plus_sec)
-obj-$(CONFIG_SMP) += platsmp.o
-obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
obj-$(CONFIG_PM_SLEEP) += pm.o
diff --git a/arch/arm/mach-highbank/core.h b/arch/arm/mach-highbank/core.h
index aea1ec5..7ec5edc 100644
--- a/arch/arm/mach-highbank/core.h
+++ b/arch/arm/mach-highbank/core.h
@@ -3,7 +3,6 @@
#include <linux/reboot.h>
-extern void highbank_set_cpu_jump(int cpu, void *jump_addr);
extern void highbank_restart(enum reboot_mode, const char *);
extern void __iomem *scu_base_addr;
@@ -14,8 +13,5 @@ static inline void highbank_pm_init(void) {}
#endif
extern void highbank_smc1(int fn, int arg);
-extern void highbank_cpu_die(unsigned int cpu);
-
-extern struct smp_operations highbank_smp_ops;
#endif
diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c
index 8e63ccd..d15f24b 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -26,9 +26,7 @@
#include <linux/amba/bus.h>
#include <linux/clk-provider.h>
-#include <asm/cacheflush.h>
-#include <asm/cputype.h>
-#include <asm/smp_plat.h>
+#include <asm/psci.h>
#include <asm/hardware/cache-l2x0.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
@@ -49,17 +47,6 @@ static void __init highbank_scu_map_io(void)
scu_base_addr = ioremap(base, SZ_4K);
}
-#define HB_JUMP_TABLE_PHYS(cpu) (0x40 + (0x10 * (cpu)))
-#define HB_JUMP_TABLE_VIRT(cpu) phys_to_virt(HB_JUMP_TABLE_PHYS(cpu))
-
-void highbank_set_cpu_jump(int cpu, void *jump_addr)
-{
- cpu = MPIDR_AFFINITY_LEVEL(cpu_logical_map(cpu), 0);
- writel(virt_to_phys(jump_addr), HB_JUMP_TABLE_VIRT(cpu));
- __cpuc_flush_dcache_area(HB_JUMP_TABLE_VIRT(cpu), 16);
- outer_clean_range(HB_JUMP_TABLE_PHYS(cpu),
- HB_JUMP_TABLE_PHYS(cpu) + 15);
-}
static void highbank_l2x0_disable(void)
{
@@ -174,7 +161,6 @@ DT_MACHINE_START(HIGHBANK, "Highbank")
#if defined(CONFIG_ZONE_DMA) && defined(CONFIG_ARM_LPAE)
.dma_zone_size = (4ULL * SZ_1G),
#endif
- .smp = smp_ops(highbank_smp_ops),
.init_irq = highbank_init_irq,
.init_time = highbank_timer_init,
.init_machine = highbank_init,
diff --git a/arch/arm/mach-highbank/hotplug.c b/arch/arm/mach-highbank/hotplug.c
deleted file mode 100644
index a019e4e..0000000
--- a/arch/arm/mach-highbank/hotplug.c
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Copyright 2011 Calxeda, Inc.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope 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.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program. If not, see <http://www.gnu.org/licenses/>.
- */
-#include <linux/kernel.h>
-#include <asm/cacheflush.h>
-
-#include "core.h"
-#include "sysregs.h"
-
-extern void secondary_startup(void);
-
-/*
- * platform-specific code to shutdown a CPU
- *
- */
-void __ref highbank_cpu_die(unsigned int cpu)
-{
- highbank_set_cpu_jump(cpu, phys_to_virt(0));
-
- flush_cache_louis();
- highbank_set_core_pwr();
-
- while (1)
- cpu_do_idle();
-}
diff --git a/arch/arm/mach-highbank/platsmp.c b/arch/arm/mach-highbank/platsmp.c
deleted file mode 100644
index 32d75cf5..0000000
--- a/arch/arm/mach-highbank/platsmp.c
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright 2010-2011 Calxeda, Inc.
- * Based on platsmp.c, Copyright (C) 2002 ARM Ltd.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
- *
- * This program is distributed in the hope 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.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program. If not, see <http://www.gnu.org/licenses/>.
- */
-#include <linux/init.h>
-#include <linux/smp.h>
-#include <linux/io.h>
-
-#include <asm/smp_scu.h>
-
-#include "core.h"
-
-extern void secondary_startup(void);
-
-static int highbank_boot_secondary(unsigned int cpu, struct task_struct *idle)
-{
- highbank_set_cpu_jump(cpu, secondary_startup);
- arch_send_wakeup_ipi_mask(cpumask_of(cpu));
- return 0;
-}
-
-/*
- * Initialise the CPU possible map early - this describes the CPUs
- * which may be present or become present in the system.
- */
-static void __init highbank_smp_init_cpus(void)
-{
- unsigned int i, ncores = 4;
-
- /* sanity check */
- if (ncores > NR_CPUS) {
- printk(KERN_WARNING
- "highbank: no. of cores (%d) greater than configured "
- "maximum of %d - clipping\n",
- ncores, NR_CPUS);
- ncores = NR_CPUS;
- }
-
- for (i = 0; i < ncores; i++)
- set_cpu_possible(i, true);
-}
-
-static void __init highbank_smp_prepare_cpus(unsigned int max_cpus)
-{
- if (scu_base_addr)
- scu_enable(scu_base_addr);
-}
-
-struct smp_operations highbank_smp_ops __initdata = {
- .smp_init_cpus = highbank_smp_init_cpus,
- .smp_prepare_cpus = highbank_smp_prepare_cpus,
- .smp_boot_secondary = highbank_boot_secondary,
-#ifdef CONFIG_HOTPLUG_CPU
- .cpu_die = highbank_cpu_die,
-#endif
-};
diff --git a/arch/arm/mach-highbank/pm.c b/arch/arm/mach-highbank/pm.c
index 04eddb4..7f2bd85 100644
--- a/arch/arm/mach-highbank/pm.c
+++ b/arch/arm/mach-highbank/pm.c
@@ -16,27 +16,19 @@
#include <linux/cpu_pm.h>
#include <linux/init.h>
-#include <linux/io.h>
#include <linux/suspend.h>
-#include <asm/cacheflush.h>
-#include <asm/proc-fns.h>
#include <asm/suspend.h>
-
-#include "core.h"
-#include "sysregs.h"
+#include <asm/psci.h>
static int highbank_suspend_finish(unsigned long val)
{
- outer_flush_all();
- outer_disable();
-
- highbank_set_pwr_suspend();
-
- cpu_do_idle();
+ const struct psci_power_state ps = {
+ .type = PSCI_POWER_STATE_TYPE_POWER_DOWN,
+ .affinity_level = 1,
+ };
- highbank_clear_pwr_request();
- return 0;
+ return psci_ops.cpu_suspend(ps, __pa(cpu_resume));
}
static int highbank_pm_enter(suspend_state_t state)
@@ -44,15 +36,11 @@ static int highbank_pm_enter(suspend_state_t state)
cpu_pm_enter();
cpu_cluster_pm_enter();
- highbank_set_cpu_jump(0, cpu_resume);
cpu_suspend(0, highbank_suspend_finish);
cpu_cluster_pm_exit();
cpu_pm_exit();
- highbank_smc1(0x102, 0x1);
- if (scu_base_addr)
- scu_enable(scu_base_addr);
return 0;
}
@@ -63,5 +51,8 @@ static const struct platform_suspend_ops highbank_pm_ops = {
void __init highbank_pm_init(void)
{
+ if (!psci_ops.cpu_suspend)
+ return;
+
suspend_set_ops(&highbank_pm_ops);
}
--
1.8.1.2
From: Rob Herring <[email protected]>
Add cpuidle support for Calxeda ecx-2000 SoC.
Signed-off-by: Rob Herring <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
---
drivers/cpuidle/cpuidle-calxeda.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
index b04767a..c6e9d80 100644
--- a/drivers/cpuidle/cpuidle-calxeda.c
+++ b/drivers/cpuidle/cpuidle-calxeda.c
@@ -66,7 +66,9 @@ static struct cpuidle_driver calxeda_idle_driver = {
static int __init calxeda_cpuidle_init(void)
{
- if (!of_machine_is_compatible("calxeda,highbank") || !psci_ops.cpu_suspend)
+ if (!of_machine_is_compatible("calxeda,highbank") ||
+ !of_machine_is_compatible("calxeda,ecx-2000") ||
+ !psci_ops.cpu_suspend)
return -ENODEV;
return cpuidle_register(&calxeda_idle_driver, NULL);
--
1.8.1.2
From: Rob Herring <[email protected]>
Add the PSCI binding node for Calxeda SOCs. Only claiming "arm,psci"
support since there is no agreement on 0.2 binding definition.
Signed-off-by: Rob Herring <[email protected]>
Cc: Pawel Moll <[email protected]>
Cc: Mark Rutland <[email protected]>
Cc: Stephen Warren <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: [email protected]
---
arch/arm/boot/dts/ecx-common.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/ecx-common.dtsi b/arch/arm/boot/dts/ecx-common.dtsi
index e8559b7..bc22557 100644
--- a/arch/arm/boot/dts/ecx-common.dtsi
+++ b/arch/arm/boot/dts/ecx-common.dtsi
@@ -19,6 +19,14 @@
bootargs = "console=ttyAMA0";
};
+ psci {
+ compatible = "arm,psci";
+ method = "smc";
+ cpu_suspend = <0x84000002>;
+ cpu_off = <0x84000004>;
+ cpu_on = <0x84000006>;
+ };
+
soc {
#address-cells = <1>;
#size-cells = <1>;
--
1.8.1.2
From: Rob Herring <[email protected]>
Now that gic_secondary_init is no longer needed to be called by SMP init
functions, the header is not needed.
Signed-off-by: Rob Herring <[email protected]>
Cc: Russell King <[email protected]>
---
arch/arm/kernel/psci_smp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/kernel/psci_smp.c b/arch/arm/kernel/psci_smp.c
index 70ded3f..570a48c 100644
--- a/arch/arm/kernel/psci_smp.c
+++ b/arch/arm/kernel/psci_smp.c
@@ -14,7 +14,6 @@
*/
#include <linux/init.h>
-#include <linux/irqchip/arm-gic.h>
#include <linux/smp.h>
#include <linux/of.h>
--
1.8.1.2
From: Rob Herring <[email protected]>
Wnen powergating the core, we need to call cpu pm notifiers to save VFP
state (!SMP only) and resetting the breakpoint h/w.
Signed-off-by: Rob Herring <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
---
drivers/cpuidle/cpuidle-calxeda.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
index 01cfecf3..b04767a 100644
--- a/drivers/cpuidle/cpuidle-calxeda.c
+++ b/drivers/cpuidle/cpuidle-calxeda.c
@@ -21,6 +21,7 @@
*/
#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
#include <linux/init.h>
#include <linux/of.h>
#include <asm/cpuidle.h>
@@ -39,7 +40,10 @@ static int calxeda_pwrdown_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
+ cpu_pm_enter();
cpu_suspend(0, calxeda_idle_finish);
+ cpu_pm_exit();
+
return index;
}
--
1.8.1.2
From: Rob Herring <[email protected]>
This updates the Calxeda cpuidle driver to use PSCI calls to powergate
cores. This is needed to enable cpuidle for the ECX-2000.
This could possibly become a generic PSCI driver, but there are no other
PSCI users in the kernel other than mach-virt.
Signed-off-by: Rob Herring <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Daniel Lezcano <[email protected]>
Cc: [email protected]
---
drivers/cpuidle/Kconfig.arm | 2 +-
drivers/cpuidle/cpuidle-calxeda.c | 40 ++++++---------------------------------
2 files changed, 7 insertions(+), 35 deletions(-)
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 8e36603..d6f57d5 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -4,7 +4,7 @@
config ARM_HIGHBANK_CPUIDLE
bool "CPU Idle Driver for Calxeda processors"
- depends on ARCH_HIGHBANK
+ depends on ARM_PSCI
select ARM_CPU_SUSPEND
help
Select this to enable cpuidle on Calxeda processors.
diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
index 3460584..01cfecf3 100644
--- a/drivers/cpuidle/cpuidle-calxeda.c
+++ b/drivers/cpuidle/cpuidle-calxeda.c
@@ -22,51 +22,23 @@
#include <linux/cpuidle.h>
#include <linux/init.h>
-#include <linux/io.h>
#include <linux/of.h>
-#include <linux/time.h>
-#include <linux/delay.h>
-#include <linux/suspend.h>
#include <asm/cpuidle.h>
-#include <asm/proc-fns.h>
-#include <asm/smp_scu.h>
#include <asm/suspend.h>
-#include <asm/cacheflush.h>
-#include <asm/cp15.h>
-
-extern void highbank_set_cpu_jump(int cpu, void *jump_addr);
-extern void __iomem *scu_base_addr;
-
-static noinline void calxeda_idle_restore(void)
-{
- set_cr(get_cr() | CR_C);
- set_auxcr(get_auxcr() | 0x40);
- scu_power_mode(scu_base_addr, SCU_PM_NORMAL);
-}
+#include <asm/psci.h>
static int calxeda_idle_finish(unsigned long val)
{
- /* Already flushed cache, but do it again as the outer cache functions
- * dirty the cache with spinlocks */
- flush_cache_all();
-
- set_auxcr(get_auxcr() & ~0x40);
- set_cr(get_cr() & ~CR_C);
-
- scu_power_mode(scu_base_addr, SCU_PM_DORMANT);
-
- cpu_do_idle();
-
- /* Restore things if we didn't enter power-gating */
- calxeda_idle_restore();
- return 1;
+ const struct psci_power_state ps = {
+ .type = PSCI_POWER_STATE_TYPE_POWER_DOWN,
+ };
+ return psci_ops.cpu_suspend(ps, __pa(cpu_resume));
}
static int calxeda_pwrdown_idle(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
- highbank_set_cpu_jump(smp_processor_id(), cpu_resume);
cpu_suspend(0, calxeda_idle_finish);
return index;
}
@@ -90,7 +62,7 @@ static struct cpuidle_driver calxeda_idle_driver = {
static int __init calxeda_cpuidle_init(void)
{
- if (!of_machine_is_compatible("calxeda,highbank"))
+ if (!of_machine_is_compatible("calxeda,highbank") || !psci_ops.cpu_suspend)
return -ENODEV;
return cpuidle_register(&calxeda_idle_driver, NULL);
--
1.8.1.2
On 09/27/2013 04:24 AM, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> This updates the Calxeda cpuidle driver to use PSCI calls to powergate
> cores. This is needed to enable cpuidle for the ECX-2000.
>
> This could possibly become a generic PSCI driver, but there are no other
> PSCI users in the kernel other than mach-virt.
>
> Signed-off-by: Rob Herring <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: [email protected]
Acked-by: Daniel Lezcano <[email protected]>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 09/27/2013 04:24 AM, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> Wnen powergating the core, we need to call cpu pm notifiers to save VFP
> state (!SMP only) and resetting the breakpoint h/w.
>
> Signed-off-by: Rob Herring <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: [email protected]
Acked-by: Daniel Lezcano <[email protected]>
Shouldn't this patch be the first in the series ?
> ---
> drivers/cpuidle/cpuidle-calxeda.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
> index 01cfecf3..b04767a 100644
> --- a/drivers/cpuidle/cpuidle-calxeda.c
> +++ b/drivers/cpuidle/cpuidle-calxeda.c
> @@ -21,6 +21,7 @@
> */
>
> #include <linux/cpuidle.h>
> +#include <linux/cpu_pm.h>
> #include <linux/init.h>
> #include <linux/of.h>
> #include <asm/cpuidle.h>
> @@ -39,7 +40,10 @@ static int calxeda_pwrdown_idle(struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
> int index)
> {
> + cpu_pm_enter();
> cpu_suspend(0, calxeda_idle_finish);
> + cpu_pm_exit();
> +
> return index;
> }
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 09/27/2013 04:24 AM, Rob Herring wrote:
> From: Rob Herring <[email protected]>
>
> Add cpuidle support for Calxeda ecx-2000 SoC.
>
> Signed-off-by: Rob Herring <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Daniel Lezcano <[email protected]>
> Cc: [email protected]
Acked-by: Daniel Lezcano <[email protected]>
Shall I pick these three patches in my tree ?
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Fri, Sep 27, 2013 at 3:47 AM, Daniel Lezcano
<[email protected]> wrote:
> On 09/27/2013 04:24 AM, Rob Herring wrote:
>>
>> From: Rob Herring <[email protected]>
>>
>> Add cpuidle support for Calxeda ecx-2000 SoC.
>>
>> Signed-off-by: Rob Herring <[email protected]>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>> Cc: Daniel Lezcano <[email protected]>
>> Cc: [email protected]
>
>
> Acked-by: Daniel Lezcano <[email protected]>
Thanks.
> Shall I pick these three patches in my tree ?
No, the highbank changes are dependent on this to go in first.
Rob