Re: [Qemu-arm] [PATCH v3 3/3] i.MX: Add an i.MX25 specific CCM class/instance.

[Peter Crosthwaite]

On Wed, Nov 25, 2015 at 11:14 PM, Jean-Christophe DUBOIS
<address@hidden> wrote:
> Le 25/11/2015 06:51, Peter Crosthwaite a écrit :
>>
>> On Thu, Nov 19, 2015 at 12:40 PM, Jean-Christophe Dubois
>> <address@hidden> wrote:
>>>
>>> Signed-off-by: Jean-Christophe Dubois <address@hidden>
>>> ---
>>>
>>> Changes since v1:
>>>   * rework loging to match other i.MX drivers
>>>
>>> Changes since v2:
>>>   * We moved to an inheritance QOM scheme
>>>
>>>   hw/arm/fsl-imx25.c          |   2 +-
>>>   hw/misc/Makefile.objs       |   1 +
>>>   hw/misc/imx25_ccm.c         | 243
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>   include/hw/arm/fsl-imx25.h  |   4 +-
>>>   include/hw/misc/imx25_ccm.h |  59 +++++++++++
>>>   5 files changed, 306 insertions(+), 3 deletions(-)
>>>   create mode 100644 hw/misc/imx25_ccm.c
>>>   create mode 100644 include/hw/misc/imx25_ccm.h
>>>

>>> +    s->mcr = 0x43000000;
>>> +    s->lpimr[0] = 0;
>>> +    s->lpimr[1] = 0;
>>> +
>>> +    /* default ROM boot will change the reset values */
>>> +    s->cctl |= INSERT(1, ARM_SRC);
>>> +    s->cctl |= INSERT(1, AHB_CLK_DIV);
>>> +}
>>> +
>>> +static uint64_t imx25_ccm_read(void *opaque, hwaddr offset, unsigned
>>> size)
>>> +{
>>> +    IMX25CCMState *s = (IMX25CCMState *)opaque;
>>> +    uint32_t *reg = &s->mpctl;
>>
>> If you want the best of both worlds for the register indexing scheme
>> (e.g. array + set of names) an anonymous union-struct would work
>> better. There are some in tree precedents. E.g. from hcd-ehci.h:
>>
>>      union {
>>          uint32_t opreg[0x44/sizeof(uint32_t)];
>>          struct {
>>              uint32_t usbcmd;
>>              uint32_t usbsts;
>>              uint32_t usbintr;
>>              uint32_t frindex;
>>              uint32_t ctrldssegment;
>>              uint32_t periodiclistbase;
>>              uint32_t asynclistaddr;
>>              uint32_t notused[9];
>>              uint32_t configflag;
>>          };
>>      };
>
>
> Well, I think I'll move later to an uint32_t array only with REG name values
> as index.
> But I'll do it for both i.MX25 and i.MX31 at the same time.
>

So I am having a look at 31, and it seems the register map tightly
packed (suitable for arrayification), however the original model only
chooses to define a handful a registers interspersed through the
memory region. I have a two patch series that adds the missing
registers and converts i.MX31 CCM to union-struct. I have tested this,
at least booting Linux as far as:

[   20.934484] VFS: Cannot open root device ...
[   20.935476] Please append a correct "root=" boot option; here are
the available partitions:

One problem with cleaning this up later is conversion to array changes
the fields as known to VMSD requiring a version bump. I think it is
better to lead out with arrayification for i.MX25 if that is the long
term solution. My patches for 31 are here:

https://github.com/pcrost/qemu/tree/imx-ccm.next

Regards,
Peter