Re: [Qemu-devel] [PATCH v3 02/14] ARM: exynos4210: CMU support

[Peter Maydell]

On 14 December 2011 17:08, Maksim E. Kozlov <address@hidden> wrote:
>>> +
>>> +/*
>>> + * There are five CMUs:
>>> + *
>>> + *  CMU_LEFTBUS
>>> + *  CMU_RIGHTBUS
>>> + *  CMU_TOP
>>> + *  CMU_DMC
>>> + *  CMU_CPU
>>> + *
>>> + *  each of them uses 16KB address space for SFRs
>>> + *
>>> + *  + 0x4000 because SFR region for CMUs starts at 0x10030000,
>>> + *  but the first CMU (CMU_LEFTBUS) starts with this offset
>>> + *
>>> + */
>>> +#define EXYNOS4210_CMU_REGS_MEM_SIZE   (0x4000 * 5 + 0x4000)
>>
>>
>> It seems a bit unlikely to me that there's really a single indivisble
>> bit of hardware with 23 kilobytes worth of register area. Is there
>> a better way of structuring this so that there are a number of
>> sub-memory-regions that cover this space each with their own
>> read/write functions (and perhaps with their own state structs)?
>> (eg instantiate five CPUs at the right locations.)
>>
>
> Dear Peter,
>
> Sorry, but I don't quite understand what do you mean. Which 23
> kilobytes you have in mind?

This:
+    memory_region_init_io(&s->iomem, &exynos4210_cmu_ops, s, "exynos4210.cmu",
+                          EXYNOS4210_CMU_REGS_MEM_SIZE);

defines a single enormous register region EXYNOS4210_CMU_REGS_MEM_SIZE
bytes long. The IO functions then implement half a dozen actual registers
and let the rest just read/write to a huge array.

> And what benefits we get from division whole region into some sub-regions?
> Current approach seems quite simple and clear. Isn't it?

The hardware docs say:
"There are five CMUs, i.e., CMU_LEFTBUS, CMU_RIGHTBUS, CMU_TOP, CMU_DMC,
and CMU_CPU, in Exynos4210" and "The internal structure of the address
space for each CMU is same for all CMUs". Then there are a bunch of
different subregions within a CMU, most of which don't have very many
actual registers.

This strongly suggests that we should be modelling things in the same
way, ie you have one device per CMU (possibly 5 instantiations of
the same device if it really is the same thing in each place).

Having "every memory location we don't implement is just reading/writing
an array" just seems very unlikely to be how the hardware actually works:
typically there's a set of registers which are at definite locations,
and if there are gaps between those registers they might read-as-zero
or they might fault, but they won't read-as-written.

-- PMM