xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xen/arm: Initial Broadcom A15 platform support
@ 2013-11-08 23:37 Jon Fraser
  2013-11-11 20:37 ` Julien Grall
  0 siblings, 1 reply; 7+ messages in thread
From: Jon Fraser @ 2013-11-08 23:37 UTC (permalink / raw)
  To: xen-devel; +Cc: julien.grall, ian.campbell, Jon Fraser

Initial platform support for Broadcom A15/B15 platforms.

Signed-off-by: Jon Fraser <jfraser@broadcom.com>
---
 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)
+{
+    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) {
+        dprintk(XENLOG_ERR, "%s: missing \"%s\" node\n", __func__, compat_str);
+        return(-EIO);
+    }
+
+    rc = dt_device_get_address(dt_dn, 0, &reg_base_64, &size);
+    if (rc) {
+        dprintk(XENLOG_ERR, "%s: missing \"reg\" prop\n", __func__);
+        return(rc);
+    }
+
+    if (dn)
+        *dn = dt_dn;
+
+    if (reg_base)
+        *reg_base = (u32)(reg_base_64 & 0xffffffff);
+
+    return(0);
+}
+
+static int brcm_populate_plat_regs(void)
+{
+    int rc;
+    struct dt_device_node *dt_dn;
+    u32 reg_base;
+    u32 val;
+    char *compat_str;
+
+    compat_str = "brcm,brcmstb-cpu-biu-ctrl";
+    rc = brcm_get_dt_node(compat_str, &dt_dn, &reg_base);
+    if (rc)
+        return(rc);
+
+    dt_property_read_u32(dt_dn, "cpu-reset-config-reg", &val);
+    regs.hif_cpu_reset_config = reg_base + val;
+
+    dt_property_read_u32(dt_dn, "cpu0-pwr-zone-ctrl-reg", &val);
+    regs.cpu0_pwr_zone_ctrl = reg_base + val;
+
+    compat_str = "brcm,brcmstb-hif-continuation";
+    rc = brcm_get_dt_node(compat_str, &dt_dn, &reg_base);
+    if (rc)
+        return(rc);
+
+    dt_property_read_u32(dt_dn, "stb-boot-hi-addr0-reg", &val);
+    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();
+
+    /* wait for power to be applied to cire */
+    do {
+        tmp = *reg;
+    } 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);
+
+    for (cpu = 1; cpu < 4; cpu++) {
+        rc = brcm_cpu_power_on(va, cpu);
+        if (rc)
+            break;
+    }
+
+    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();
+    }
+
+    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);
+    reg = (volatile u32 *)(va + (regs.hif_cpu_reset_config & ~PAGE_MASK));
+
+    /* now take the cpus out of reset */
+    reg[0] = 0;
+
+    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 =
+{
+    "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:
+ */
-- 
1.7.11.3

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Initial Broadcom A15 platform support
  2013-11-08 23:37 [PATCH] xen/arm: Initial Broadcom A15 platform support Jon Fraser
@ 2013-11-11 20:37 ` Julien Grall
  2013-11-12  9:57   ` Ian Campbell
  2013-11-21 18:15   ` Jon Fraser
  0 siblings, 2 replies; 7+ messages in thread
From: Julien Grall @ 2013-11-11 20:37 UTC (permalink / raw)
  To: Jon Fraser; +Cc: ian.campbell, xen-devel

On 11/08/2013 11:37 PM, Jon Fraser wrote:
> Initial platform support for Broadcom A15/B15 platforms.
>
> Signed-off-by: Jon Fraser <jfraser@broadcom.com>

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, &reg_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, &reg_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, &reg_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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Initial Broadcom A15 platform support
  2013-11-11 20:37 ` Julien Grall
@ 2013-11-12  9:57   ` Ian Campbell
  2013-11-12 12:52     ` Julien Grall
  2013-11-21 18:15   ` Jon Fraser
  1 sibling, 1 reply; 7+ messages in thread
From: Ian Campbell @ 2013-11-12  9:57 UTC (permalink / raw)
  To: Julien Grall; +Cc: Jon Fraser, xen-devel

On Mon, 2013-11-11 at 20:37 +0000, Julien Grall wrote:
> > +
> > +    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?

Not only in the future, we have command line options for these sorts of
things already, which I think the ARM port obeys.

Ian.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Initial Broadcom A15 platform support
  2013-11-12  9:57   ` Ian Campbell
@ 2013-11-12 12:52     ` Julien Grall
  2013-11-12 13:48       ` Ian Campbell
  0 siblings, 1 reply; 7+ messages in thread
From: Julien Grall @ 2013-11-12 12:52 UTC (permalink / raw)
  To: Ian Campbell; +Cc: Jon Fraser, xen-devel



On 11/12/2013 09:57 AM, Ian Campbell wrote:
> On Mon, 2013-11-11 at 20:37 +0000, Julien Grall wrote:
>>> +
>>> +    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?
>
> Not only in the future, we have command line options for these sorts of
> things already, which I think the ARM port obeys.

Are you talking about "max_cpus" parameter? If so, it's not yet 
implemented on ARM. We rely on the number of cpus node in the device tree.

-- 
Julien Grall

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Initial Broadcom A15 platform support
  2013-11-12 12:52     ` Julien Grall
@ 2013-11-12 13:48       ` Ian Campbell
  0 siblings, 0 replies; 7+ messages in thread
From: Ian Campbell @ 2013-11-12 13:48 UTC (permalink / raw)
  To: Julien Grall; +Cc: Jon Fraser, xen-devel

On Tue, 2013-11-12 at 12:52 +0000, Julien Grall wrote:
> 
> On 11/12/2013 09:57 AM, Ian Campbell wrote:
> > On Mon, 2013-11-11 at 20:37 +0000, Julien Grall wrote:
> >>> +
> >>> +    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?
> >
> > Not only in the future, we have command line options for these sorts of
> > things already, which I think the ARM port obeys.
> 
> Are you talking about "max_cpus" parameter?

Yes.

>  If so, it's not yet 
> implemented on ARM. We rely on the number of cpus node in the device tree.

I guess I should have checked...

In any case you are right that hardcoding 4 here is not correct and it
would be better to do it in one of the provided per-cpu callbacks.

Ian.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Initial Broadcom A15 platform support
  2013-11-11 20:37 ` Julien Grall
  2013-11-12  9:57   ` Ian Campbell
@ 2013-11-21 18:15   ` Jon Fraser
  2013-11-22  9:58     ` Ian Campbell
  1 sibling, 1 reply; 7+ messages in thread
From: Jon Fraser @ 2013-11-21 18:15 UTC (permalink / raw)
  To: Julien Grall; +Cc: jfraser, ian.campbell, xen-devel

On Mon, 2013-11-11 at 20:37 +0000, Julien Grall wrote:
> On 11/08/2013 11:37 PM, Jon Fraser wrote:
> > Initial platform support for Broadcom A15/B15 platforms.
> >
> > Signed-off-by: Jon Fraser <jfraser@broadcom.com>
> 
> 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.


Habits are hard to break!

Is there a xen equivalent of the linux checkpatch?
Or a set of command line options for indent that is acceptable
to the xen community?  Switching back and forth between xen and linux
kernel, I don't want to keep making formatting mistakes.  Well, I know
I'll make them, I just want to catch them.

I'll address the rest of the issues and submit a new version
as soon as I can.

Thanks,
Jon

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Initial Broadcom A15 platform support
  2013-11-21 18:15   ` Jon Fraser
@ 2013-11-22  9:58     ` Ian Campbell
  0 siblings, 0 replies; 7+ messages in thread
From: Ian Campbell @ 2013-11-22  9:58 UTC (permalink / raw)
  To: jfraser; +Cc: Julien Grall, xen-devel

On Thu, 2013-11-21 at 13:15 -0500, Jon Fraser wrote:
> On Mon, 2013-11-11 at 20:37 +0000, Julien Grall wrote:
> > On 11/08/2013 11:37 PM, Jon Fraser wrote:
> > > Initial platform support for Broadcom A15/B15 platforms.
> > >
> > > Signed-off-by: Jon Fraser <jfraser@broadcom.com>
> > 
> > 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.
> 
> 
> Habits are hard to break!
> 
> Is there a xen equivalent of the linux checkpatch?

I'm afraid not.

> Or a set of command line options for indent that is acceptable
> to the xen community?

The closest is the emacs magic blocks at the bottom of most files, but
they only really cover the 4-space indentation and not all of the
whitespace differences etc. 

The bigs things are:
4 space indents
Spaces inside the if expre. eg. "if ( foo == bar )" not "if (foo ==
bar)"
The { comes on the next line not at the end so:
        if ( foo == bar )
        {
and not
        if ( foo == bar ) {
(similarly for while, for etc)

> Switching back and forth between xen and linux
> kernel,

Yeah, I do it all the time as well.

> I don't want to keep making formatting mistakes.  Well, I know
> I'll make them, I just want to catch them.
> 
> I'll address the rest of the issues and submit a new version
> as soon as I can.
> 
> Thanks,
> Jon
> 
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2013-11-22  9:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-08 23:37 [PATCH] xen/arm: Initial Broadcom A15 platform support Jon Fraser
2013-11-11 20:37 ` Julien Grall
2013-11-12  9:57   ` Ian Campbell
2013-11-12 12:52     ` Julien Grall
2013-11-12 13:48       ` Ian Campbell
2013-11-21 18:15   ` Jon Fraser
2013-11-22  9:58     ` Ian Campbell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).