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

[Stephen Warren <swarren@wwwdotorg.org>]

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