Patchwork [U-Boot,2/2] ARM: rpi_b: use bcm2835 mbox driver to get memory size

login
register
mail settings
Submitter Stephen Warren
Date Oct. 16, 2012, 5:10 a.m.
Message ID <1350364236-23982-2-git-send-email-swarren@wwwdotorg.org>
Download mbox | patch
Permalink /patch/191721/
State Changes Requested
Delegated to: Albert ARIBAUD
Headers show

Comments

Stephen Warren - Oct. 16, 2012, 5:10 a.m.
The firmware running on the bcm2835 SoC's VideoCore CPU determines how
much of the system RAM is available for use by the ARM CPU. Previously,
U-Boot assumed that only 128MB was available, since this was the
smallest value configured by any public firmware. However, we can now
query the actual value at run-time from the firmware using the mbox
property protocol.

Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
---
 board/raspberrypi/rpi_b/rpi_b.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)
Albert ARIBAUD - Oct. 18, 2012, 8:28 p.m.
Hi Stephen,

On Mon, 15 Oct 2012 23:10:36 -0600, Stephen Warren
<swarren@wwwdotorg.org> wrote:

> The firmware running on the bcm2835 SoC's VideoCore CPU determines how
> much of the system RAM is available for use by the ARM CPU. Previously,
> U-Boot assumed that only 128MB was available, since this was the
> smallest value configured by any public firmware. However, we can now
> query the actual value at run-time from the firmware using the mbox
> property protocol.
> 
> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
> ---
>  board/raspberrypi/rpi_b/rpi_b.c |   20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/board/raspberrypi/rpi_b/rpi_b.c b/board/raspberrypi/rpi_b/rpi_b.c
> index 688b0aa..88f8c58 100644
> --- a/board/raspberrypi/rpi_b/rpi_b.c
> +++ b/board/raspberrypi/rpi_b/rpi_b.c
> @@ -15,13 +15,31 @@
>   */
>  
>  #include <common.h>
> +#include <asm/arch/mbox.h>
>  #include <asm/global_data.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
>  int dram_init(void)
>  {
> -	gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
> +	ALLOC_ALIGN_BUFFER(struct bcm2835_mbox_buf_get_arm_mem, buf, 1, 16);
> +
> +	memset(buf, 0, sizeof(*buf));
> +	buf->hdr.buf_size = sizeof(*buf);
> +	buf->hdr.code = BCM2835_MBOX_REQ_CODE;
> +	buf->tag_hdr.tag = BCM2835_MBOX_TAG_GET_ARM_MEMORY;
> +	buf->tag_hdr.val_buf_size = sizeof(buf->body);
> +	buf->tag_hdr.val_len = sizeof(buf->body.req);
> +
> +	bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &buf->hdr);
> +
> +	if ((buf->hdr.code != BCM2835_MBOX_RESP_CODE_SUCCESS) ||
> +	    (!(buf->tag_hdr.val_len & BCM2835_MBOX_TAG_VAL_LEN_RESPONSE))) {
> +		printf("BCM2835_MBOX_TAG_GET_ARM_MEMORY failed\n");
> +		return -1;
> +	}
> +
> +	gd->ram_size = buf->body.resp.mem_size;
>  
>  	return 0;
>  }

Either move the buffer processing into mbox (as an opaque "ask mbox for
memory size" function) or move the memory request struct in here (this
is in order to have a self-consistent mbox interface where either the
mbox API does not know at all about the mem size request, or it knows
all about it).

Amicalement,
Stephen Warren - Oct. 18, 2012, 9:51 p.m.
On 10/18/2012 02:28 PM, Albert ARIBAUD wrote:
> Hi Stephen,
> 
> On Mon, 15 Oct 2012 23:10:36 -0600, Stephen Warren
> <swarren@wwwdotorg.org> wrote:
> 
>> The firmware running on the bcm2835 SoC's VideoCore CPU determines how
>> much of the system RAM is available for use by the ARM CPU. Previously,
>> U-Boot assumed that only 128MB was available, since this was the
>> smallest value configured by any public firmware. However, we can now
>> query the actual value at run-time from the firmware using the mbox
>> property protocol.
>>
>> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
>> ---
>>  board/raspberrypi/rpi_b/rpi_b.c |   20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/board/raspberrypi/rpi_b/rpi_b.c b/board/raspberrypi/rpi_b/rpi_b.c
>> index 688b0aa..88f8c58 100644
>> --- a/board/raspberrypi/rpi_b/rpi_b.c
>> +++ b/board/raspberrypi/rpi_b/rpi_b.c
>> @@ -15,13 +15,31 @@
>>   */
>>  
>>  #include <common.h>
>> +#include <asm/arch/mbox.h>
>>  #include <asm/global_data.h>
>>  
>>  DECLARE_GLOBAL_DATA_PTR;
>>  
>>  int dram_init(void)
>>  {
>> -	gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
>> +	ALLOC_ALIGN_BUFFER(struct bcm2835_mbox_buf_get_arm_mem, buf, 1, 16);
>> +
>> +	memset(buf, 0, sizeof(*buf));
>> +	buf->hdr.buf_size = sizeof(*buf);
>> +	buf->hdr.code = BCM2835_MBOX_REQ_CODE;
>> +	buf->tag_hdr.tag = BCM2835_MBOX_TAG_GET_ARM_MEMORY;
>> +	buf->tag_hdr.val_buf_size = sizeof(buf->body);
>> +	buf->tag_hdr.val_len = sizeof(buf->body.req);
>> +
>> +	bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &buf->hdr);
>> +
>> +	if ((buf->hdr.code != BCM2835_MBOX_RESP_CODE_SUCCESS) ||
>> +	    (!(buf->tag_hdr.val_len & BCM2835_MBOX_TAG_VAL_LEN_RESPONSE))) {
>> +		printf("BCM2835_MBOX_TAG_GET_ARM_MEMORY failed\n");
>> +		return -1;
>> +	}
>> +
>> +	gd->ram_size = buf->body.resp.mem_size;
>>  
>>  	return 0;
>>  }
> 
> Either move the buffer processing into mbox (as an opaque "ask mbox for
> memory size" function)

I don't like that idea, because soon there will be a bunch of other
message types (e.g. display configuration, power on/off), and multiple
"tags" can be packed into a single "message", and it should really be up
to the client driver to decide which sets of messages it sends at once,
not the mbox driver.

> ... or move the memory request struct in here (this
> is in order to have a self-consistent mbox interface where either the
> mbox API does not know at all about the mem size request, or it knows
> all about it).

I also don't entirely like that idea - what if multiple different
drivers want to send the same message (e.g. power on for I2C and power
on for SPI both use the same message structure, just with a different
device ID). Putting the message buffer definition into a single location
makes sense here.

I suppose one could define a "bcm2835 power" driver for that case, and
only define the message structure there? I guess if we go that route,
moving all the struct definitions out of the mbox driver would be
do-able, although I still like the idea of a single central list so it
can be easily matched up with the documentation...

Patch

diff --git a/board/raspberrypi/rpi_b/rpi_b.c b/board/raspberrypi/rpi_b/rpi_b.c
index 688b0aa..88f8c58 100644
--- a/board/raspberrypi/rpi_b/rpi_b.c
+++ b/board/raspberrypi/rpi_b/rpi_b.c
@@ -15,13 +15,31 @@ 
  */
 
 #include <common.h>
+#include <asm/arch/mbox.h>
 #include <asm/global_data.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
 int dram_init(void)
 {
-	gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
+	ALLOC_ALIGN_BUFFER(struct bcm2835_mbox_buf_get_arm_mem, buf, 1, 16);
+
+	memset(buf, 0, sizeof(*buf));
+	buf->hdr.buf_size = sizeof(*buf);
+	buf->hdr.code = BCM2835_MBOX_REQ_CODE;
+	buf->tag_hdr.tag = BCM2835_MBOX_TAG_GET_ARM_MEMORY;
+	buf->tag_hdr.val_buf_size = sizeof(buf->body);
+	buf->tag_hdr.val_len = sizeof(buf->body.req);
+
+	bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &buf->hdr);
+
+	if ((buf->hdr.code != BCM2835_MBOX_RESP_CODE_SUCCESS) ||
+	    (!(buf->tag_hdr.val_len & BCM2835_MBOX_TAG_VAL_LEN_RESPONSE))) {
+		printf("BCM2835_MBOX_TAG_GET_ARM_MEMORY failed\n");
+		return -1;
+	}
+
+	gd->ram_size = buf->body.resp.mem_size;
 
 	return 0;
 }