diff mbox

raspi: Add Raspberry Pi 1 support

Message ID 20170408054318.19830-1-omar.rizwan@gmail.com
State New
Headers show

Commit Message

Omar Rizwan April 8, 2017, 5:43 a.m. UTC
Signed-off-by: Omar Rizwan <omar.rizwan@gmail.com>
---
 hw/arm/Makefile.objs         |   2 +-
 hw/arm/bcm2835.c             | 119 +++++++++++++++++++++++++++++++++++++++++++
 hw/arm/bcm2835_peripherals.c |   1 +
 hw/arm/raspi.c               |  47 ++++++++++++++---
 include/hw/arm/bcm2835.h     |  31 +++++++++++
 5 files changed, 191 insertions(+), 9 deletions(-)
 create mode 100644 hw/arm/bcm2835.c
 create mode 100644 include/hw/arm/bcm2835.h

Comments

Omar Rizwan April 8, 2017, 6:13 a.m. UTC | #1
Hi Andrew --

On Fri, Apr 7, 2017 at 10:57 PM Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
>
> Hi Omar,
>
> > From: Omar Rizwan [mailto:omar.rizwan@gmail.com]
> > Sent: Friday, 7 April 2017 22:43
>
> Did you do any testing of this? One of the reasons I never got around to upstreaming this was that I couldn't get Raspbian working reliably (there was some problem with stalled DMA reads from the MMC controller), and I didn't have the cycles to debug it further. Also, IIRC Raspbian on pi1 depended on the system timer devices, so it might not make sense to have this board-level support without adding that first.


I did test Raspbian on the raspi machine and maybe encountered the
same issue you did (or, more likely, a systimer related issue);
it would just hang in a loop with program counter at 0xC000A000.

But my use for this is bare-metal code, which works OK.
(Is there some standard that the machine should run Linux before
getting upstreamed?)

I'd like to upstream the system timer next, anyway.

>
> > --- a/hw/arm/bcm2835_peripherals.c
> > +++ b/hw/arm/bcm2835_peripherals.c
> > @@ -34,6 +34,7 @@ static void bcm2835_peripherals_init(Object *obj)
> >      /* Internal memory region for peripheral bus addresses (not exported) */
> >      memory_region_init(&s->gpu_bus_mr, obj, "bcm2835-gpu", (uint64_t)1 <<
> > 32);
> >      object_property_add_child(obj, "gpu-bus", OBJECT(&s->gpu_bus_mr),
> > NULL);
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->gpu_bus_mr);
> >
> >      /* Internal memory region for request/response communication with
> >       * mailbox-addressable peripherals (not exported)
>
> This line looks like it might have snuck in by accident?


Do you recall why that line was removed? bcm2835_realize fails without
it, because the second sysbus_mmio_map_overlap call

sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 1,
                            0x40000000, 1);

fails this assertion in sysbus.c, in sysbus_mmio_map_common:

assert(n >= 0 && n < dev->num_mmio);

I can't easily find documentation for that 0x40000000 memory mapping,
which I figured I'd keep in; should it just be removed from bcm2835?
Am I misunderstanding something?

>
>
> Andrew


thanks,
Omar
Cameron Esfahani via April 10, 2017, 5:04 p.m. UTC | #2
> From: Omar Rizwan [mailto:omar.rizwan@gmail.com]

> Sent: Friday, 7 April 2017 23:13

> On Fri, Apr 7, 2017 at 10:57 PM Andrew Baumann

> <Andrew.Baumann@microsoft.com> wrote:

> > > From: Omar Rizwan [mailto:omar.rizwan@gmail.com]

> > > Sent: Friday, 7 April 2017 22:43


> > Did you do any testing of this? One of the reasons I never got around to

> upstreaming this was that I couldn't get Raspbian working reliably (there was

> some problem with stalled DMA reads from the MMC controller), and I didn't

> have the cycles to debug it further. Also, IIRC Raspbian on pi1 depended on the

> system timer devices, so it might not make sense to have this board-level

> support without adding that first.

> 

> I did test Raspbian on the raspi machine and maybe encountered the

> same issue you did (or, more likely, a systimer related issue);

> it would just hang in a loop with program counter at 0xC000A000.

> 

> But my use for this is bare-metal code, which works OK.

> (Is there some standard that the machine should run Linux before

> getting upstreamed?)


No, I don't know of such a standard, but it's probably worth noting the limitations in the commit message. And it might even be worth putting the systimers first...

> I'd like to upstream the system timer next, anyway.


Sounds good.

> > > --- a/hw/arm/bcm2835_peripherals.c

> > > +++ b/hw/arm/bcm2835_peripherals.c

> > > @@ -34,6 +34,7 @@ static void bcm2835_peripherals_init(Object *obj)

> > >      /* Internal memory region for peripheral bus addresses (not exported) */

> > >      memory_region_init(&s->gpu_bus_mr, obj, "bcm2835-gpu", (uint64_t)1

> <<

> > > 32);

> > >      object_property_add_child(obj, "gpu-bus", OBJECT(&s->gpu_bus_mr),

> > > NULL);

> > > +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->gpu_bus_mr);

> > >

> > >      /* Internal memory region for request/response communication with

> > >       * mailbox-addressable peripherals (not exported)

> >

> > This line looks like it might have snuck in by accident?

> 

> 

> Do you recall why that line was removed? bcm2835_realize fails without

> it, because the second sysbus_mmio_map_overlap call

> 

> sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 1,

>                             0x40000000, 1);

> 

> fails this assertion in sysbus.c, in sysbus_mmio_map_common:

> 

> assert(n >= 0 && n < dev->num_mmio);

> 

> I can't easily find documentation for that 0x40000000 memory mapping,

> which I figured I'd keep in; should it just be removed from bcm2835?

> Am I misunderstanding something?


The various mapping aliases are intended to implement the bus addresses from S1.2.3 / figure on page 5 of this document:
https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf

In ARM CPU physical addresses, the RAM is mapped at 0, and the peripherals at 0x20000000

In VC CPU bus addresses (which get used for DMA), RAM is aliased four times, at 0 0x40000000 0x80000000 and 0xC0000000. Peripherals are also aliased four times at a fixed offset of 0x3e000000 from the base of the RAM alias, which is how you get the 0x7e000000 peripheral base (0x40000000 + 0x3e000000).

This is implemented as follows: bcm2835_peripherals implements/exports one memory region "peri_mr" which contains just the peripherals. It also builds a private MR for the GPU ("VC CPU" in the spec) bus addresses, "gpu_mr", and aliases RAM into it four times along with the peripheral MR at BCM2835_VC_PERI_BASE (0x7e000000). Its parent, device, either bcm2835 or bcm2836, maps the peripheral MR into the default sysbus mmio bus (i.e., the CPU's physical address space) at the relevant CPU physical base address (0x20000000 on pi1, 0x3F000000 on pi2).

Now, as to why the mapping call fails: I'm confused where you saw that call with the 0x40000000 address. The code in my tree for bcm2835.c is:

/* Peripheral base address seen by the CPU */
#define BCM2835_PERI_BASE       0x20000000
...
    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 0,
                            BCM2835_PERI_BASE, 1);

Also, note that this is mapping the peripherals MR, not the the gpu_bus_mr which is never used with any sysbus APIs, so I'm still not clear on why we need to call sysbus_init_mmio on it. It's entirely possible my code is broken, and/or that assert wasn't there when I tested it, but you might have to dig a bit deeper to convince me that what you're doing is also sane :)

BTW, I suspect nobody else will want to review this until after 2.9.

Andrew
Cameron Esfahani via April 10, 2017, 5:08 p.m. UTC | #3
> From: Andrew Baumann

> Sent: Monday, 10 April 2017 10:05

> > From: Omar Rizwan [mailto:omar.rizwan@gmail.com]

> > Sent: Friday, 7 April 2017 23:13


> > I can't easily find documentation for that 0x40000000 memory mapping,

> > which I figured I'd keep in; should it just be removed from bcm2835?

> > Am I misunderstanding something?

> 

> The various mapping aliases are intended to implement the bus addresses from

> S1.2.3 / figure on page 5 of this document:

> https://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-

> Peripherals.pdf

> 

> In ARM CPU physical addresses, the RAM is mapped at 0, and the peripherals at

> 0x20000000

> 

> In VC CPU bus addresses (which get used for DMA), RAM is aliased four times,

> at 0 0x40000000 0x80000000 and 0xC0000000. Peripherals are also aliased

> four times at a fixed offset of 0x3e000000 from the base of the RAM alias,

> which is how you get the 0x7e000000 peripheral base (0x40000000 +

> 0x3e000000).

> 

> This is implemented as follows: bcm2835_peripherals implements/exports one

> memory region "peri_mr" which contains just the peripherals. It also builds a

> private MR for the GPU ("VC CPU" in the spec) bus addresses, "gpu_mr", and

> aliases RAM into it four times along with the peripheral MR at

> BCM2835_VC_PERI_BASE (0x7e000000). Its parent, device, either bcm2835 or

> bcm2836, maps the peripheral MR into the default sysbus mmio bus (i.e., the

> CPU's physical address space) at the relevant CPU physical base address

> (0x20000000 on pi1, 0x3F000000 on pi2).

> 

> Now, as to why the mapping call fails: I'm confused where you saw that call

> with the 0x40000000 address. The code in my tree for bcm2835.c is:

> 

> /* Peripheral base address seen by the CPU */

> #define BCM2835_PERI_BASE       0x20000000

> ...

>     sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 0,

>                             BCM2835_PERI_BASE, 1);


Ok, I just looked at your patch, and it looks like you added the 0x40000000 mapping. It's definitely not in my tree (https://github.com/0xabu/qemu/blob/raspi/hw/arm/bcm2835.c). I would suggest taking it back out again and sanity-checking your patch against the code in my tree for any other spurious changes that may have snuck in...

Andrew
no-reply@patchew.org April 12, 2017, 11:57 p.m. UTC | #4
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20170408054318.19830-1-omar.rizwan@gmail.com
Subject: [Qemu-devel] [PATCH] raspi: Add Raspberry Pi 1 support
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
082bb61 raspi: Add Raspberry Pi 1 support

=== OUTPUT BEGIN ===
Checking PATCH 1/1: raspi: Add Raspberry Pi 1 support...
ERROR: line over 90 characters
#189: FILE: hw/arm/raspi.c:34:
+ * https://github.com/AndrewFromMelbourne/raspberry_pi_revision/blob/2764c19983fb7b1ef8cb21031e94c293703afa2e/README.md

total: 1 errors, 0 warnings, 258 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Eric Blake April 13, 2017, 12:04 a.m. UTC | #5
On 04/12/2017 06:57 PM, no-reply@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:

> === OUTPUT BEGIN ===
> Checking PATCH 1/1: raspi: Add Raspberry Pi 1 support...
> ERROR: line over 90 characters
> #189: FILE: hw/arm/raspi.c:34:
> + * https://github.com/AndrewFromMelbourne/raspberry_pi_revision/blob/2764c19983fb7b1ef8cb21031e94c293703afa2e/README.md

You can shorten this:

https://github.com/AndrewFromMelbourne/raspberry_pi_revision/blob/2764c199/README.md

to fit under 90, while still resolving to the same place (collisions
with just an 8-char prefix are rare enough to generally not be an issue
to git)
Peter Maydell April 20, 2017, 1:20 p.m. UTC | #6
On 8 April 2017 at 06:43, Omar Rizwan <omar.rizwan@gmail.com> wrote:
> Signed-off-by: Omar Rizwan <omar.rizwan@gmail.com>
> ---
>  hw/arm/Makefile.objs         |   2 +-
>  hw/arm/bcm2835.c             | 119 +++++++++++++++++++++++++++++++++++++++++++
>  hw/arm/bcm2835_peripherals.c |   1 +
>  hw/arm/raspi.c               |  47 ++++++++++++++---
>  include/hw/arm/bcm2835.h     |  31 +++++++++++
>  5 files changed, 191 insertions(+), 9 deletions(-)
>  create mode 100644 hw/arm/bcm2835.c
>  create mode 100644 include/hw/arm/bcm2835.h

Hi; thanks for this patch. I have a few comments below to add to the ones
from Andrew.


> +/* Peripheral base address seen by the CPU */
> +#define BCM2835_PERI_BASE       0x20000000
> +
> +static void bcm2835_init(Object *obj)
> +{
> +    BCM2835State *s = BCM2835(obj);
> +
> +    object_initialize(&s->cpu, sizeof(s->cpu), "arm1176-" TYPE_ARM_CPU);
> +    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), &error_abort);
> +
> +    object_initialize(&s->peripherals, sizeof(s->peripherals),
> +                      TYPE_BCM2835_PERIPHERALS);
> +    object_property_add_child(obj, "peripherals", OBJECT(&s->peripherals),
> +                              &error_abort);
> +    object_property_add_alias(obj, "board-rev", OBJECT(&s->peripherals),
> +                              "board-rev", &error_abort);
> +    object_property_add_alias(obj, "vcram-size", OBJECT(&s->peripherals),
> +                              "vcram-size", &error_abort);
> +    qdev_set_parent_bus(DEVICE(&s->peripherals), sysbus_get_default());
> +}
> +
> +static void bcm2835_realize(DeviceState *dev, Error **errp)
> +{
> +    BCM2835State *s = BCM2835(dev);
> +    Object *obj;
> +    Error *err = NULL;
> +
> +    /* common peripherals from bcm2835 */
> +    obj = object_property_get_link(OBJECT(dev), "ram", &err);
> +    if (obj == NULL) {
> +        error_setg(errp, "%s: required ram link not found: %s",
> +                   __func__, error_get_pretty(err));
> +        return;
> +    }
> +
> +    object_property_add_const_link(OBJECT(&s->peripherals), "ram", obj, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    object_property_set_bool(OBJECT(&s->peripherals), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(&s->peripherals),
> +                              "sd-bus", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 0,
> +                            BCM2835_PERI_BASE, 1);
> +
> +    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 1,
> +                            0x40000000, 1);

If this mapping is correct, can we have a #define for this address, like we
do for BCM2835_PERI_BASE ?

(I see from Andrew's review that it's not clear whether we should be
mapping this here.)

> +
> +    object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
> +    if (err) {
> +        error_report_err(err);
> +        exit(1);
> +    }
> +
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0,
> +                       qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
> +                       qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ));
> +
> +}
> +
> +static void bcm2835_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +
> +    dc->realize = bcm2835_realize;
> +
> +    /*
> +     * Reason: creates an ARM CPU, thus use after free(), see
> +     * arm_cpu_class_init()
> +     */
> +    dc->cannot_destroy_with_object_finalize_yet = true;

This assignment and its attached comment can be deleted, because
the issue with arm_cpu_class_init() has now been fixed upstream
(there's a patchset on the list that removes the DeviceClass
field entirely).

> +}
> +
> +static const TypeInfo bcm2835_type_info = {
> +    .name = TYPE_BCM2835,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(BCM2835State),
> +    .instance_init = bcm2835_init,
> +    .class_init = bcm2835_class_init,
> +};
> +
> +static void bcm2835_register_types(void)
> +{
> +    type_register_static(&bcm2835_type_info);
> +}
> +
> +type_init(bcm2835_register_types)
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 369ef1e..a6cd54a 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -34,6 +34,7 @@ static void bcm2835_peripherals_init(Object *obj)
>      /* Internal memory region for peripheral bus addresses (not exported) */
>      memory_region_init(&s->gpu_bus_mr, obj, "bcm2835-gpu", (uint64_t)1 << 32);
>      object_property_add_child(obj, "gpu-bus", OBJECT(&s->gpu_bus_mr), NULL);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->gpu_bus_mr);

If we determine that this is correct, then we should delete the "(not exported)"
part of the comment above, because this added call to sysbus_init_mmio()
is exporting the memory region.

>      /* Internal memory region for request/response communication with
>       * mailbox-addressable peripherals (not exported)
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 2b295f1..dca6077 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -2,9 +2,11 @@
>   * Raspberry Pi emulation (c) 2012 Gregory Estrade
>   * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
>   *
> - * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft
> + * Raspberry Pi 2 emulation Copyright (c) 2015, Microsoft
>   * Written by Andrew Baumann
>   *
> + * Raspberry Pi 1 rebase onto master (c) 2017, Omar Rizwan
> + *
>   * This code is licensed under the GNU GPLv2 and later.
>   */
>
> @@ -12,6 +14,7 @@
>  #include "qapi/error.h"
>  #include "qemu-common.h"
>  #include "cpu.h"
> +#include "hw/arm/bcm2835.h"
>  #include "hw/arm/bcm2836.h"
>  #include "qemu/error-report.h"
>  #include "hw/boards.h"
> @@ -27,6 +30,11 @@
>  /* Table of Linux board IDs for different Pi versions */
>  static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43};
>
> +/* Table of board revisions:
> + * https://github.com/AndrewFromMelbourne/raspberry_pi_revision/blob/2764c19983fb7b1ef8cb21031e94c293703afa2e/README.md
> + */
> +static const uint32_t raspi_boardrev[] = {[1] = 0x10, [2] = 0xa21041};

So we have three things here that are per-board:
 * the boardid value
 * the boardrev value
 * the soc_type value

which we have stored in two arrays plus passing a different parameter
to the raspi_machine_init function.

Perhaps this would be better as a
typedef struct RPiInfo {
    const char *soc_type;
    uint32_t boardrev;
    uint32_t boardid;
} RPiInfo;

static const RPiInfo rpi_boards[] = {
    {
         /* Raspberry Pi 1 */
         .soc_type = TYPE_BCM2835,
         .boardrev = 0x10,
         .boardid = 0xc42,
    }, {
         /* Raspberry Pi 2 */
         ...
    }
};

?

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 4c5c4ee..77ef47f 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -11,7 +11,7 @@  obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
 obj-$(CONFIG_DIGIC) += digic.o
 obj-y += omap1.o omap2.o strongarm.o
 obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
-obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o
+obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2835.o bcm2836.o raspi.o
 obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
 obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
diff --git a/hw/arm/bcm2835.c b/hw/arm/bcm2835.c
new file mode 100644
index 0000000..b19b86c
--- /dev/null
+++ b/hw/arm/bcm2835.c
@@ -0,0 +1,119 @@ 
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Raspberry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * Rebase onto master (c) 2017 Omar Rizwan
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "hw/arm/bcm2835.h"
+#include "hw/arm/raspi_platform.h"
+#include "hw/sysbus.h"
+#include "exec/address-spaces.h"
+
+/* Peripheral base address seen by the CPU */
+#define BCM2835_PERI_BASE       0x20000000
+
+static void bcm2835_init(Object *obj)
+{
+    BCM2835State *s = BCM2835(obj);
+
+    object_initialize(&s->cpu, sizeof(s->cpu), "arm1176-" TYPE_ARM_CPU);
+    object_property_add_child(obj, "cpu", OBJECT(&s->cpu), &error_abort);
+
+    object_initialize(&s->peripherals, sizeof(s->peripherals),
+                      TYPE_BCM2835_PERIPHERALS);
+    object_property_add_child(obj, "peripherals", OBJECT(&s->peripherals),
+                              &error_abort);
+    object_property_add_alias(obj, "board-rev", OBJECT(&s->peripherals),
+                              "board-rev", &error_abort);
+    object_property_add_alias(obj, "vcram-size", OBJECT(&s->peripherals),
+                              "vcram-size", &error_abort);
+    qdev_set_parent_bus(DEVICE(&s->peripherals), sysbus_get_default());
+}
+
+static void bcm2835_realize(DeviceState *dev, Error **errp)
+{
+    BCM2835State *s = BCM2835(dev);
+    Object *obj;
+    Error *err = NULL;
+
+    /* common peripherals from bcm2835 */
+    obj = object_property_get_link(OBJECT(dev), "ram", &err);
+    if (obj == NULL) {
+        error_setg(errp, "%s: required ram link not found: %s",
+                   __func__, error_get_pretty(err));
+        return;
+    }
+
+    object_property_add_const_link(OBJECT(&s->peripherals), "ram", obj, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    object_property_set_bool(OBJECT(&s->peripherals), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    object_property_add_alias(OBJECT(s), "sd-bus", OBJECT(&s->peripherals),
+                              "sd-bus", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 0,
+                            BCM2835_PERI_BASE, 1);
+
+    sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 1,
+                            0x40000000, 1);
+
+    object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
+    if (err) {
+        error_report_err(err);
+        exit(1);
+    }
+
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0,
+                       qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 1,
+                       qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ));
+
+}
+
+static void bcm2835_class_init(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = bcm2835_realize;
+
+    /*
+     * Reason: creates an ARM CPU, thus use after free(), see
+     * arm_cpu_class_init()
+     */
+    dc->cannot_destroy_with_object_finalize_yet = true;
+}
+
+static const TypeInfo bcm2835_type_info = {
+    .name = TYPE_BCM2835,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(BCM2835State),
+    .instance_init = bcm2835_init,
+    .class_init = bcm2835_class_init,
+};
+
+static void bcm2835_register_types(void)
+{
+    type_register_static(&bcm2835_type_info);
+}
+
+type_init(bcm2835_register_types)
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 369ef1e..a6cd54a 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -34,6 +34,7 @@  static void bcm2835_peripherals_init(Object *obj)
     /* Internal memory region for peripheral bus addresses (not exported) */
     memory_region_init(&s->gpu_bus_mr, obj, "bcm2835-gpu", (uint64_t)1 << 32);
     object_property_add_child(obj, "gpu-bus", OBJECT(&s->gpu_bus_mr), NULL);
+    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->gpu_bus_mr);
 
     /* Internal memory region for request/response communication with
      * mailbox-addressable peripherals (not exported)
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 2b295f1..dca6077 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -2,9 +2,11 @@ 
  * Raspberry Pi emulation (c) 2012 Gregory Estrade
  * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
  *
- * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft
+ * Raspberry Pi 2 emulation Copyright (c) 2015, Microsoft
  * Written by Andrew Baumann
  *
+ * Raspberry Pi 1 rebase onto master (c) 2017, Omar Rizwan
+ *
  * This code is licensed under the GNU GPLv2 and later.
  */
 
@@ -12,6 +14,7 @@ 
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
+#include "hw/arm/bcm2835.h"
 #include "hw/arm/bcm2836.h"
 #include "qemu/error-report.h"
 #include "hw/boards.h"
@@ -27,6 +30,11 @@ 
 /* Table of Linux board IDs for different Pi versions */
 static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43};
 
+/* Table of board revisions:
+ * https://github.com/AndrewFromMelbourne/raspberry_pi_revision/blob/2764c19983fb7b1ef8cb21031e94c293703afa2e/README.md
+ */
+static const uint32_t raspi_boardrev[] = {[1] = 0x10, [2] = 0xa21041};
+
 typedef struct RasPiState {
     BCM2836State soc;
     MemoryRegion ram;
@@ -113,7 +121,8 @@  static void setup_boot(MachineState *machine, int version, size_t ram_size)
     arm_load_kernel(ARM_CPU(first_cpu), &binfo);
 }
 
-static void raspi2_init(MachineState *machine)
+static void raspi_machine_init(MachineState *machine, int version,
+                               const char *soc_type)
 {
     RasPiState *s = g_new0(RasPiState, 1);
     uint32_t vcram_size;
@@ -122,7 +131,7 @@  static void raspi2_init(MachineState *machine)
     BusState *bus;
     DeviceState *carddev;
 
-    object_initialize(&s->soc, sizeof(s->soc), TYPE_BCM2836);
+    object_initialize(&s->soc, sizeof(s->soc), soc_type);
     object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
                               &error_abort);
 
@@ -135,10 +144,12 @@  static void raspi2_init(MachineState *machine)
     /* Setup the SOC */
     object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
                                    &error_abort);
-    object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
-                            &error_abort);
-    object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev",
-                            &error_abort);
+    if (version == 2) {
+        object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
+                                &error_abort);
+    }
+    object_property_set_int(OBJECT(&s->soc), raspi_boardrev[version],
+                            "board-rev", &error_abort);
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_abort);
 
     /* Create and plug in the SD cards */
@@ -155,9 +166,29 @@  static void raspi2_init(MachineState *machine)
 
     vcram_size = object_property_get_int(OBJECT(&s->soc), "vcram-size",
                                          &error_abort);
-    setup_boot(machine, 2, machine->ram_size - vcram_size);
+    setup_boot(machine, version, machine->ram_size - vcram_size);
+}
+
+static void raspi1_init(MachineState *machine)
+{
+    raspi_machine_init(machine, 1, TYPE_BCM2835);
 }
+static void raspi1_machine_init(MachineClass *mc)
+{
+    mc->desc = "Raspberry Pi";
+    mc->init = raspi1_init;
+    mc->block_default_type = IF_SD;
+    mc->no_parallel = 1;
+    mc->no_floppy = 1;
+    mc->no_cdrom = 1;
+    mc->default_ram_size = 512 * 1024 * 1024;
+};
+DEFINE_MACHINE("raspi", raspi1_machine_init)
 
+static void raspi2_init(MachineState *machine)
+{
+    raspi_machine_init(machine, 2, TYPE_BCM2836);
+}
 static void raspi2_machine_init(MachineClass *mc)
 {
     mc->desc = "Raspberry Pi 2";
diff --git a/include/hw/arm/bcm2835.h b/include/hw/arm/bcm2835.h
new file mode 100644
index 0000000..c93c4ef
--- /dev/null
+++ b/include/hw/arm/bcm2835.h
@@ -0,0 +1,31 @@ 
+/*
+ * Raspberry Pi emulation (c) 2012 Gregory Estrade
+ * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
+ *
+ * Raspberry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
+ * Written by Andrew Baumann
+ *
+ * Rebase onto master (c) 2017 Omar Rizwan
+ *
+ * This code is licensed under the GNU GPLv2 and later.
+ */
+
+#ifndef BCM2835_H
+#define BCM2835_H
+
+#include "hw/arm/arm.h"
+#include "hw/arm/bcm2835_peripherals.h"
+
+#define TYPE_BCM2835 "bcm2835"
+#define BCM2835(obj) OBJECT_CHECK(BCM2835State, (obj), TYPE_BCM2835)
+
+typedef struct BCM2835State {
+    /*< private >*/
+    DeviceState parent_obj;
+    /*< public >*/
+
+    ARMCPU cpu;
+    BCM2835PeripheralState peripherals;
+} BCM2835State;
+
+#endif /* BCM2835_H */