[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] xen/arm: Initial Broadcom A15 platform support
On 11/08/2013 11:37 PM, Jon Fraser wrote:
Initial platform support for Broadcom A15/B15 platforms.
Signed-off-by: Jon Fraser <jfraser@xxxxxxxxxxxx>
Hello,
Thanks to add support for broadcom processor in Xen.
It seems you use the wrong coding style for this patch. You can read
CODING_STYLES at the root of the xen repository.
---
xen/arch/arm/platforms/Makefile | 1 +
xen/arch/arm/platforms/brcm.c | 249 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 250 insertions(+)
create mode 100644 xen/arch/arm/platforms/brcm.c
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index f0dd72c..60635b4 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -3,3 +3,4 @@ obj-$(CONFIG_ARM_32) += exynos5.o
obj-$(CONFIG_ARM_32) += midway.o
obj-$(CONFIG_ARM_32) += omap5.o
obj-$(CONFIG_ARM_32) += sunxi.o
+obj-$(CONFIG_ARM_32) += brcm.o
diff --git a/xen/arch/arm/platforms/brcm.c b/xen/arch/arm/platforms/brcm.c
new file mode 100644
index 0000000..b8d8527
--- /dev/null
+++ b/xen/arch/arm/platforms/brcm.c
@@ -0,0 +1,249 @@
+/*
+ * xen/arch/arm/platform/brcm.c
+ *
+ * Broadcom specific settings
+ *
+ * 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>
+
+
+struct brcm_plat_regs {
+ uint32_t hif_mask;
+ uint32_t hif_cpu_reset_config;
+ uint32_t hif_boot_continuation;
+ uint32_t cpu0_pwr_zone_ctrl;
+};
+
+static struct brcm_plat_regs regs;
+
+static int brcm_get_dt_node(char *compat_str, struct dt_device_node **dn,
+ u32 *reg_base)
spurious space before u32.
+{
+ struct dt_device_node *dt_dn;
+ u64 reg_base_64;
+ u64 size;
+ int rc;
+
+ dt_dn = dt_find_compatible_node(NULL, NULL, compat_str);
+ if (!dt_dn) {
if ( ... )
{
+ dprintk(XENLOG_ERR, "%s: missing \"%s\" node\n", __func__, compat_str);
+ return(-EIO);
+ }
+
+ rc = dt_device_get_address(dt_dn, 0, ®_base_64, &size);
+ if (rc) {
+ dprintk(XENLOG_ERR, "%s: missing \"reg\" prop\n", __func__);
+ return(rc);
+ }
+
+ if (dn)
if ( ... )
+ *dn = dt_dn;
+
+ if (reg_base)
+ *reg_base = (u32)(reg_base_64 & 0xffffffff);
I think you don't need to use both cast and mask. You can only use one
of theses solutions.
+
+ return(0);
+}
+
+static int brcm_populate_plat_regs(void)
+{
+ int rc;
+ struct dt_device_node *dt_dn;
As you don't modify the node, you should use const.
+ u32 reg_base;
+ u32 val;
+ char *compat_str;
Same here.
+
+ compat_str = "brcm,brcmstb-cpu-biu-ctrl";
+ rc = brcm_get_dt_node(compat_str, &dt_dn, ®_base);
+ if (rc)
+ return(rc);
+
+ dt_property_read_u32(dt_dn, "cpu-reset-config-reg", &val);
You should check the return of dt_property_read_u32
+ regs.hif_cpu_reset_config = reg_base + val;
+
+ dt_property_read_u32(dt_dn, "cpu0-pwr-zone-ctrl-reg", &val);
Same here.
+ regs.cpu0_pwr_zone_ctrl = reg_base + val;
+
+ compat_str = "brcm,brcmstb-hif-continuation";
+ rc = brcm_get_dt_node(compat_str, &dt_dn, ®_base);
+ if (rc)
+ return(rc);
+
+ dt_property_read_u32(dt_dn, "stb-boot-hi-addr0-reg", &val);
Same here.
+ regs.hif_boot_continuation = reg_base + val;
+
+ dprintk(XENLOG_INFO, "hif_cpu_reset_config : %08xh\n",
+ regs.hif_cpu_reset_config);
+ dprintk(XENLOG_INFO, "cpu0_pwr_zone_ctrl : %08xh\n",
+ regs.cpu0_pwr_zone_ctrl);
+ dprintk(XENLOG_INFO, "hif_boot_continuation : %08xh\n",
+ regs.hif_boot_continuation);
+
+ return(0);
+}
+
+#define ZONE_PWR_UP_REQ (1 << 10)
+#define ZONE_PWR_ON_STATE (1 << 26)
+
+static int brcm_cpu_power_on(void __iomem *va, int cpu)
+{
+ int rc = 0;
+ volatile u32 *reg;
+ u32 tmp;
+
+ reg = (volatile u32 *)(va + (regs.cpu0_pwr_zone_ctrl & ~PAGE_MASK) +
+ (cpu * sizeof(u32)));
+
+ /* request core power on */
+ *reg = ZONE_PWR_UP_REQ;
+ dsb();
You should use writel/readl wrapper to read/write memory mapped with
ioremap_*. It will take care of dsb.
+
+ /* wait for power to be applied to cire */
+ do {
+ tmp = *reg;
Same here.
+ } while (!(tmp & ZONE_PWR_ON_STATE));
+
+ return rc;
+}
+
+static int brcm_cpu_power_on_all(void)
+{
+ int rc = 0;
+ int cpu;
+ void __iomem *va;
+
+ va = ioremap_nocache(regs.cpu0_pwr_zone_ctrl & PAGE_MASK, PAGE_SIZE);
ioremap_* is able to cooperate with non aligned address. You don't need
to align cpu0_pwr_zone_ctrl. You should also check that va is not NULL
(it's an error).
+
+ for (cpu = 1; cpu < 4; cpu++) {
+ rc = brcm_cpu_power_on(va, cpu);
+ if (rc)
+ break;
+ }
I don't like the for loop on the cores. In the future, it's possible
that Xen decides to only use 2 cores, so you will power unused core.
What about enabling power in brcm_cpu_up, if it's not too late?
+
+ iounmap(va);
+
+ return rc;
+}
+
+static int __init brcm_smp_init(void)
+{
+ void __iomem *va;
+ volatile u32 *reg;
+ u32 target_pc;
+ dprintk(XENLOG_INFO, "%s\n",__func__);
+
+ if (regs.cpu0_pwr_zone_ctrl) {
+ dprintk(XENLOG_INFO, "applying power to remaining CPU cores\n");
+ brcm_cpu_power_on_all();
You already call brcm_cpu_power_on_all in brcm_init. Why do you need to
call it again?
+ }
+
+ va = ioremap_nocache(regs.hif_boot_continuation & PAGE_MASK, PAGE_SIZE);
+ reg = (volatile u32 *)(va + (regs.hif_boot_continuation & ~PAGE_MASK));
+
+
+ /* set boot continuation registers */
+ dprintk(XENLOG_INFO, "%s: HIF CR at 0x%p\n", __func__, reg);
+ target_pc =__pa(init_secondary);
+ reg[3] = target_pc;
+ reg[5] = target_pc;
+ reg[7] = target_pc;
+
+ iounmap(va);
+
+ va = ioremap_nocache(regs.hif_cpu_reset_config & PAGE_MASK, PAGE_SIZE);
Some comment as the previous ioremap_*
+ reg = (volatile u32 *)(va + (regs.hif_cpu_reset_config & ~PAGE_MASK));
+
+ /* now take the cpus out of reset */
+ reg[0] = 0;
writel?
+
+ iounmap(va);
+
+ return 0;
+}
+
+
+static int brcm_init(void)
+{
+ int rc;
+
+ rc = brcm_populate_plat_regs();
+ if (rc)
+ return(rc);
+
+ /*
+ * All newer 28nm chips will require poking the BPCM prior to starting
+ * the cores.
+ */
+ if (regs.cpu0_pwr_zone_ctrl) {
+ dprintk(XENLOG_INFO, "applying power to remaining CPU cores\n");
+ brcm_cpu_power_on_all();
+ }
+
+ return(0);
+}
+static int __init brcm_cpu_up(int cpu)
+{
+ /*
+ * Nothing to do here, the generic sev() will suffice to kick CPUs
+ * out of either the firmware or our own smp_up_cpu gate,
+ * depending on where they have ended up.
+ */
+
+ return 0;
+}
+
+static int brcm_specific_mapping(struct domain *d)
+{
+ /* Map the entire GISB register space 1:1 */
+ map_mmio_regions(d, 0xf0000000, 0xf0ffffff,
+ 0xf0000000);
+
+ return 0;
+}
+
+static void brcm_reset(void)
+{
+}
+
+static uint32_t brcm_quirks(void)
+{
+ return PLATFORM_QUIRK_DOM0_MAPPING_11;
+}
+
+static const char const *brcm_dt_compat[] __initdata =
const char * const ... __initconst
+{
+ "arm,brcm",
+ "brcm,brcmstb",
+ NULL
+};
+
+PLATFORM_START(brcm, "Broadcom B15")
+ .compatible = brcm_dt_compat,
+ .init = brcm_init,
+ .smp_init = brcm_smp_init,
+ .cpu_up = brcm_cpu_up,
+ .quirks = brcm_quirks,
+ .reset = brcm_reset,
+ .specific_mapping = brcm_specific_mapping,
+PLATFORM_END
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
