[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH v3 4/7] bcm2835_peripherals: add roll
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH v3 4/7] bcm2835_peripherals: add rollup device for bcm2835 peripherals |
Date: |
Wed, 6 Jan 2016 05:32:45 -0800 |
On Tue, Jan 5, 2016 at 10:07 PM, Andrew Baumann
<address@hidden> wrote:
>> From: Alistair Francis [mailto:address@hidden
>> Sent: Tuesday, 5 January 2016 18:14
>> On Thu, Dec 31, 2015 at 4:31 PM, Andrew Baumann
>> <address@hidden> wrote:
>> > This device maintains all the non-CPU peripherals on bcm2835 (Pi1)
>> > which are also present on bcm2836 (Pi2). It also implements the
>> > private address spaces used for DMA and mailboxes.
> [...]
>> > + 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;
>> > + }
>>
>> I only had a quick read of this patch, but this RAM linking looks fine
>> to me. Out of curiosity is there a reason you use
>> object_property_get_link() instead of object_property_add_link() in
>> the init?
>
The const link system removes the need for the object to have storage
for the link pointer in state. This means you don't need the state
field or add_link(), but the only way to get the pointer for your own
use is to get_link() on yourself. This is slightly simpler but has the
disadvantage that you cannot unlink and then relink something else (I
think?).
I don't have an opinion over which way is more correct so both are
fine for me but if the QOM people have a preferred style we should
probably make the two patches consistent.
Regards,
Peter
> I'm not sure I understand your question... it wouldn't work the other way. I
> allocate the ram and add the link using object_property_add_const_link() in
> hw/arm/raspi.c. This file needs to consume the ram to setup alias mappings,
> so it is using get_link(). (Note there's also level of indirection; raspi
> creates bcm2836, which does nothing but get the link set by its parent and
> add it to its bcm2835_peripherals child.)
>
> I suppose I could do it the other way around (allocate and set link in
> bcm2835_peripherals, based on a size passed from the board), but it seemed
> more logical to treat the RAM as created/owned of the board rather than the
> SoC.
>
> Cheers,
> Andrew