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

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(-)

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,

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...