[Qemu-arm] [PATCH] bcm2835_property: implement "get board revision" query

[Qemu-arm] [PATCH] bcm2835_property: implement "get board revision" query

[Stephen Warren]

Return a valid value from the BCM2835 property mailbox query "get board
revision". This query is used by U-Boot. Implementing it fixes the first
obvious difference between qemu and real HW.

The value returned is currently hard-coded to match the RPi2 I own. Other
values are legal, e.g. different board manufacturer field values are
likely to exist in the wild. When support for other RPi models is added,
the value can be made dynamic.

Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
---
This looks like the only non-video-related mailbox request used by U-Boot
that wasn't implemented by qemu.

The only other feature required for U-Boot to get to its command prompt
is the BCM2835 timer.

Andrew, I assume you're actively working on mainlining all the features
in your github branch? I look forward to U-Boot working on qemu:-)
---
 hw/arm/bcm2835_peripherals.c       | 9 +++++++++
 hw/misc/bcm2835_property.c         | 4 ++--
 include/hw/misc/bcm2835_property.h | 1 +
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 18b72ecb696c..5fed6c90b048 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -148,6 +148,15 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /* When multiple Pi revisions are supported, this hard-coded number should
+     * be selected by the various raspi*_machine_init().
+     */
+    object_property_set_int(OBJECT(&s->property), 0xa21041, "board-rev", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
     object_property_set_bool(OBJECT(&s->property), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
index e42b43e72d56..45bd6c18ce90 100644
--- a/hw/misc/bcm2835_property.c
+++ b/hw/misc/bcm2835_property.c
@@ -43,8 +43,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
             resplen = 4;
             break;
         case 0x00010002: /* Get board revision */
-            qemu_log_mask(LOG_UNIMP,
-                          "bcm2835_property: %x get board revision NYI\n", tag);
+            stl_phys(&s->dma_as, value + 12, s->board_rev);
             resplen = 4;
             break;
         case 0x00010003: /* Get board MAC address */
@@ -258,6 +257,7 @@ static void bcm2835_property_realize(DeviceState *dev, Error **errp)
 }
 
 static Property bcm2835_property_props[] = {
+    DEFINE_PROP_UINT32("board-rev", BCM2835PropertyState, board_rev, 0),
     DEFINE_PROP_UINT32("ram-size", BCM2835PropertyState, ram_size, 0),
     DEFINE_PROP_END_OF_LIST()
 };
diff --git a/include/hw/misc/bcm2835_property.h b/include/hw/misc/bcm2835_property.h
index fcf5f3decafc..df889eaa08b5 100644
--- a/include/hw/misc/bcm2835_property.h
+++ b/include/hw/misc/bcm2835_property.h
@@ -23,6 +23,7 @@ typedef struct {
     MemoryRegion iomem;
     qemu_irq mbox_irq;
     MACAddr macaddr;
+    uint32_t board_rev;
     uint32_t ram_size;
     uint32_t addr;
     bool pending;
-- 
1.9.1

Re: [Qemu-arm] [PATCH] bcm2835_property: implement "get board revision" query

[Peter Maydell]

Hi; you forgot to cc qemu-devel (all patches must be cc'd there).

thanks
-- PMM

On 7 February 2016 at 05:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
> Return a valid value from the BCM2835 property mailbox query "get board
> revision". This query is used by U-Boot. Implementing it fixes the first
> obvious difference between qemu and real HW.
>
> The value returned is currently hard-coded to match the RPi2 I own. Other
> values are legal, e.g. different board manufacturer field values are
> likely to exist in the wild. When support for other RPi models is added,
> the value can be made dynamic.
>
> Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
> This looks like the only non-video-related mailbox request used by U-Boot
> that wasn't implemented by qemu.
>
> The only other feature required for U-Boot to get to its command prompt
> is the BCM2835 timer.
>
> Andrew, I assume you're actively working on mainlining all the features
> in your github branch? I look forward to U-Boot working on qemu:-)
> ---
>  hw/arm/bcm2835_peripherals.c       | 9 +++++++++
>  hw/misc/bcm2835_property.c         | 4 ++--
>  include/hw/misc/bcm2835_property.h | 1 +
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 18b72ecb696c..5fed6c90b048 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -148,6 +148,15 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +    /* When multiple Pi revisions are supported, this hard-coded number should
> +     * be selected by the various raspi*_machine_init().
> +     */
> +    object_property_set_int(OBJECT(&s->property), 0xa21041, "board-rev", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
>      object_property_set_bool(OBJECT(&s->property), true, "realized", &err);
>      if (err) {
>          error_propagate(errp, err);
> diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
> index e42b43e72d56..45bd6c18ce90 100644
> --- a/hw/misc/bcm2835_property.c
> +++ b/hw/misc/bcm2835_property.c
> @@ -43,8 +43,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
>              resplen = 4;
>              break;
>          case 0x00010002: /* Get board revision */
> -            qemu_log_mask(LOG_UNIMP,
> -                          "bcm2835_property: %x get board revision NYI\n", tag);
> +            stl_phys(&s->dma_as, value + 12, s->board_rev);
>              resplen = 4;
>              break;
>          case 0x00010003: /* Get board MAC address */
> @@ -258,6 +257,7 @@ static void bcm2835_property_realize(DeviceState *dev, Error **errp)
>  }
>
>  static Property bcm2835_property_props[] = {
> +    DEFINE_PROP_UINT32("board-rev", BCM2835PropertyState, board_rev, 0),
>      DEFINE_PROP_UINT32("ram-size", BCM2835PropertyState, ram_size, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
> diff --git a/include/hw/misc/bcm2835_property.h b/include/hw/misc/bcm2835_property.h
> index fcf5f3decafc..df889eaa08b5 100644
> --- a/include/hw/misc/bcm2835_property.h
> +++ b/include/hw/misc/bcm2835_property.h
> @@ -23,6 +23,7 @@ typedef struct {
>      MemoryRegion iomem;
>      qemu_irq mbox_irq;
>      MACAddr macaddr;
> +    uint32_t board_rev;
>      uint32_t ram_size;
>      uint32_t addr;
>      bool pending;
> --
> 1.9.1
>

Re: [Qemu-devel] [PATCH] bcm2835_property: implement "get board revision" query

[Peter Maydell]

Hi; you forgot to cc qemu-devel (all patches must be cc'd there).

thanks
-- PMM

On 7 February 2016 at 05:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
> Return a valid value from the BCM2835 property mailbox query "get board
> revision". This query is used by U-Boot. Implementing it fixes the first
> obvious difference between qemu and real HW.
>
> The value returned is currently hard-coded to match the RPi2 I own. Other
> values are legal, e.g. different board manufacturer field values are
> likely to exist in the wild. When support for other RPi models is added,
> the value can be made dynamic.
>
> Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
> This looks like the only non-video-related mailbox request used by U-Boot
> that wasn't implemented by qemu.
>
> The only other feature required for U-Boot to get to its command prompt
> is the BCM2835 timer.
>
> Andrew, I assume you're actively working on mainlining all the features
> in your github branch? I look forward to U-Boot working on qemu:-)
> ---
>  hw/arm/bcm2835_peripherals.c       | 9 +++++++++
>  hw/misc/bcm2835_property.c         | 4 ++--
>  include/hw/misc/bcm2835_property.h | 1 +
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 18b72ecb696c..5fed6c90b048 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -148,6 +148,15 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +    /* When multiple Pi revisions are supported, this hard-coded number should
> +     * be selected by the various raspi*_machine_init().
> +     */
> +    object_property_set_int(OBJECT(&s->property), 0xa21041, "board-rev", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
>      object_property_set_bool(OBJECT(&s->property), true, "realized", &err);
>      if (err) {
>          error_propagate(errp, err);
> diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
> index e42b43e72d56..45bd6c18ce90 100644
> --- a/hw/misc/bcm2835_property.c
> +++ b/hw/misc/bcm2835_property.c
> @@ -43,8 +43,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
>              resplen = 4;
>              break;
>          case 0x00010002: /* Get board revision */
> -            qemu_log_mask(LOG_UNIMP,
> -                          "bcm2835_property: %x get board revision NYI\n", tag);
> +            stl_phys(&s->dma_as, value + 12, s->board_rev);
>              resplen = 4;
>              break;
>          case 0x00010003: /* Get board MAC address */
> @@ -258,6 +257,7 @@ static void bcm2835_property_realize(DeviceState *dev, Error **errp)
>  }
>
>  static Property bcm2835_property_props[] = {
> +    DEFINE_PROP_UINT32("board-rev", BCM2835PropertyState, board_rev, 0),
>      DEFINE_PROP_UINT32("ram-size", BCM2835PropertyState, ram_size, 0),
>      DEFINE_PROP_END_OF_LIST()
>  };
> diff --git a/include/hw/misc/bcm2835_property.h b/include/hw/misc/bcm2835_property.h
> index fcf5f3decafc..df889eaa08b5 100644
> --- a/include/hw/misc/bcm2835_property.h
> +++ b/include/hw/misc/bcm2835_property.h
> @@ -23,6 +23,7 @@ typedef struct {
>      MemoryRegion iomem;
>      qemu_irq mbox_irq;
>      MACAddr macaddr;
> +    uint32_t board_rev;
>      uint32_t ram_size;
>      uint32_t addr;
>      bool pending;
> --
> 1.9.1
>

Re: [Qemu-arm] [PATCH] bcm2835_property: implement "get board revision" query

[Andrew Baumann]

Hi Stephen,

Thanks for the patch!

> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
> Sent: Sunday, 7 February 2016 4:09 PM
> 
> Return a valid value from the BCM2835 property mailbox query "get board
> revision". This query is used by U-Boot. Implementing it fixes the first
> obvious difference between qemu and real HW.
> 
> The value returned is currently hard-coded to match the RPi2 I own. Other
> values are legal, e.g. different board manufacturer field values are likely to
> exist in the wild. When support for other RPi models is added, the value can
> be made dynamic.
> 
> Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
> This looks like the only non-video-related mailbox request used by U-Boot
> that wasn't implemented by qemu.
> 
> The only other feature required for U-Boot to get to its command prompt is
> the BCM2835 timer.
> 
> Andrew, I assume you're actively working on mainlining all the features in
> your github branch? I look forward to U-Boot working on qemu:-)

That's right, although I probably won't get the next round of patches out until late Feb. However, my goal in that series is to merge support for Pi1, so:

* Do you know where we can find the appropriate values for Pi1 boards?
* Can you please propagate the board-rev property through bcm2835_peripherals and bcm2836 so that the appropriate value is being set in the board definition file (raspi.c)? This should be as simple as a few calls to object_property_add_alias(), e.g.:
https://github.com/0xabu/qemu/blob/raspi/hw/arm/bcm2836.c#L42

Thanks,
Andrew

Re: [Qemu-arm] [PATCH] bcm2835_property: implement "get board revision" query

[Stephen Warren]

On 02/07/2016 03:37 PM, Andrew Baumann wrote:
> Hi Stephen,
> 
> Thanks for the patch!
> 
>> From: Stephen Warren [mailto:swarren@wwwdotorg.org]
>> Sent: Sunday, 7 February 2016 4:09 PM
>>
>> Return a valid value from the BCM2835 property mailbox query "get board
>> revision". This query is used by U-Boot. Implementing it fixes the first
>> obvious difference between qemu and real HW.
...
> * Do you know where we can find the appropriate values for Pi1 boards?

Sure. There are two encoding schemes. For most boards, the revision
number is a simple integer that indicates the board type. A list can be
found at:

> https://github.com/AndrewFromMelbourne/raspberry_pi_revision/blob/master/README.md#prior-to-raspberry-pi-2

(There's also a bit that indicates the Pi has been over-volted in there,
so you need to mask off 0x1000000 or just use the LS byte when
extracting the revision.)

The RPi2 and RPi0 introduce a more complex multi-field encoding scheme
described at:

> https://github.com/AndrewFromMelbourne/raspberry_pi_revision/blob/master/README.md#raspberry-pi-2--model-zero

You can find how U-Boot interprets the revision fields at:

> https://github.com/swarren/u-boot/blob/rpi_dev/board/raspberrypi/rpi/rpi.c#L335

Also see the various xx_models[] arrays, and some more links at:

> https://github.com/swarren/u-boot/blob/rpi_dev/board/raspberrypi/rpi/rpi.c#L77