* [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, ®_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, ®_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, ®_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, ®_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
^ 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).