Re: [Qemu-devel] [PATCH 09/24] omap_gpmc/nseries/tusb6010: convert to memory API

[Avi Kivity <avi@redhat.com>]

On 08/09/2011 11:07 AM, Peter Maydell wrote:
> On 9 August 2011 08:41, Avi Kivity<avi@redhat.com>  wrote:
> >  On 08/09/2011 10:37 AM, Peter Maydell wrote:
> >>
> >>  On 9 August 2011 07:34, Avi Kivity<avi@redhat.com>    wrote:
> >>  >    Also, my patchset focuses on mechanical transformations.  It is already
> >>  >    risky enough in terms of regressions, I'm not going to rewrite/improve
> >>  >  all
> >>  >    of qemu; if you want those callbacks removed, you will have to remove
> >>  >  them
> >>  >    yourself.
> >>
> >>  Sure. Can I ask you to drop this one and the onenand patch, and I'll
> >>  have a go over the next week or three at incorporating a conversion into
> >>  the patchset I have for those?
>
> >  Umm... next week or three?  I'd like to move fast on this.
>
> I'm busy at least half of this week, and next week is the KVM
> Forum, so I didn't want to promise anything faster.
>
> >  Can't you base your patches on this instead?  After all, this is just a
> >  mechanical transformation, there should be no functional change.
>
> Well, maybe, but it seems a bit silly to commit something which is
> going to be half-reverted and rewritten.

It certainly won't be reverted, since the ram_addr_t based API will be 
removed.  So it will just be modified to suit however you want it to 
be.  If you want it otherwise, post a patch, either on top of this or 
instead of it, and I will incorporate it into this patchset.

Again, this is not about improving individual devices (that is left for 
device maintainers); it's just about converting to the new API with 
minimal changes.

> Also I just noticed this change:
>
> -static CPUReadMemoryFunc * const omap_gpmc_readfn[] = {
> -    omap_badwidth_read32,      /* TODO */
> -    omap_badwidth_read32,      /* TODO */
> -    omap_gpmc_read,
> -};
> -
> -static CPUWriteMemoryFunc * const omap_gpmc_writefn[] = {
> -    omap_badwidth_write32,     /* TODO */
> -    omap_badwidth_write32,     /* TODO */
> -    omap_gpmc_write,
> +static const MemoryRegionOps omap_gpmc_ops = {
> +    /* TODO: specialize<4 byte writes? */
> +    .read = omap_gpmc_read,
> +    .write = omap_gpmc_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
>   };
>
> ...isn't this just throwing away the warnings on bad-width accesses?

It is; will fix.

> (it's not clear to me from the docs what the behaviour of the new
> API on bad-width accesses is going to be.)

It's one of the things I'm collecting requirements on.  So far I think 
we'll have an inheritable, configurable policy that allows you to ignore 
writes/read ones, raise a machine check exception, or warn (in developer 
mode only).

-- 
error compiling committee.c: too many arguments to function