Re: [Qemu-devel] [RFC 2/5] hw/arm: add very initial support for Canon DIGIC SoC

["Andreas Färber" <afaerber@suse.de>]

Am 30.08.2013 07:07, schrieb Peter Crosthwaite:
> Hi,
> 
> 
> On Fri, Aug 30, 2013 at 6:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On 29 August 2013 20:36, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>>> On Thu, 29 Aug 2013 14:15:40 +0200
>>> Andreas Färber <afaerber@suse.de> wrote:
>>>> DigicState should be a QOM type derived from TYPE_DEVICE. Since you're
>>>> hardcoding the CPU type, please use object_initialize() to create it
>>>> in-place - note we're about to extend that function but rebase will be
>>>> trivial.
>>>
>>> I have just examinied platforms with hardcoded cpu: pxa2xx, exynos4210.
>>> They don't use object_initialize().
>>
>> You'll find that QEMU is full of old code that hasn't been updated
>> to the current "best practice". pxa2xx in particular is pretty elderly
>> and probably a bad example to copy from. object_initialize()
>> is pretty new, which is why you can't find many examples yet.
>>
>>>> Is the RAM on the board or on the SoC? It's in DigicState but
>>>> initialized from the board init. If it's on the SoC, then
>>>> _add_subregion and _register_ram_global should be in its realizefn.
>>>> Otherwise please separate it from the SoC state.
>>>
>>> It's not a trivial question!
>>> See "Digging Into 'DIGIC 4' Image Processor (2)" (http://techon.nikkeibp.co.jp/english/NEWS_EN/20090218/165866/).
>>> The authors removed the upper package with a chemical solution and exposed the chips.
>>> The 'DIGIC 4' contains 3 chips in one package:
>>>  * processor itself;
> 
> For clarity, do you mean SoC here?
> 
>>
>>>  * 64-Mbit NOR flash memory, the "K8P6415UQB" (note that I have found another flash K8P3215UQB on my Canon A1100: Manufacturer ID: 0xEC, Device ID: 0x7E0301);
>>>  * 512-Mbit SDRAM, the "K4X51323PE" (just the same 64 M RAM as I see with barebox).
>>>
>>> So we can assume taht these memory chips are inalienable parts of the SoC.
>>
>> Package-on-Package is really just a funky way of connecting
>> up separate components (the layers are only connected
>> together at PCB assembly time, wikipedia tells me), and
>> indeed you can have different component combinations
>> (as you've found with the flash memory). So I would suggest
>> that either:
>>  (a) just model them all as separate components
>>    (SoC, memory, flash) instantiated by the board
>>  (b) model them as separate components, and have a
>>    "container" component which puts them together and
>>    then the board just instantiates that.
>>
>> Whether (b) is worthwhile depends on whether we
>> expect to have lots of boards that differ but have the
>> same PoP stack. I suspect (a) is better.
> 
> I like (b). If something is sold, branded or soldered as a self
> contained package then I think its worth having its own device. FWIW,
> I want to do this with Zynq sooner or later as its a SoC that's
> modelled as a board.

Personally I tend to (a) in this case. As I understood it, this SoC/PoP
only occurs in cameras, not as a reusable component, so the amount of
RAM and variant of the SoC is uniquely identified by the machine and we
wouldn't know how else to name the PoP container object.

A reason for (b) might be if multiple cameras contain the same
pre-produced PoP - not sure if that is the case, and we only have one
board for now.

You are of course right that if we don't think about this now, we risk
having future contributors copy-and-pasting code without reconsidering;
but the struct defining the per-board RAM size is already set up for
sharing code among boards, so I think instantiating them at machine
level will not be a problem for now.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg