qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 02/15] hw/omap_gpio.c: Don't complain about some


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 02/15] hw/omap_gpio.c: Don't complain about some writes to r/o registers
Date: Sat, 30 Jul 2011 13:44:09 +0100

On 30 July 2011 05:53, andrzej zaborowski <address@hidden> wrote:
> I went ahead and pushed the series

Thanks.

> with the exception of this patch
> and 14/15 because I think these are the types of patches that should
> remain in downstream as a reminder, is there an argument for not
> fixing these things in Linux?

I think the only arguments are "it's too hard" and "I'd like to
run legacy images without having qemu constantly complaining and
making '-serial stdio' unusable" :-) [have you seen how much chatter
you get out of n810 running a meego image?] I think I've said before
that really we ought to have this kind of message more consistently
and conveniently enablable/disablable.

I'll leave this sort of change out of future patches; when I get
down to the point where they're the only thing left in my patch
stack (a) I'll be very happy and (b) I might look at a nicer way
to handle them.

> In patch 04 I renamed omap2_gpio_module_set to omap2_gpio_set because
> the parameter is no longer the module pointer.  By the way I think we
> should also pass the target agent pointer on creation the same way
> clocks are passed and use omap_l4_attach.

The clock passing thing is (as per comments) a bit nasty; at
the moment sysbus is the closest we have to a nice way of exposing
"I have some MMIO regions" so I'm a bit reluctant to hide it behind
another magic pointer property. I'll have a think about this, though.

> In patch 07 I bumped the vmstate version because the structure seems
> to have changed.

Oops, yes. I had a note at some point to look more carefully at
the vmstate change but I must have forgotten about it :-(
I agree we can just bump the version.

> In patch 12 I removed the
> else {
>    s->bdrv_cur = s->bdrv;
> } part because there seemed to be no reason to add it, please check
> that I haven't broken something.

That looks like it should be OK, we'll always call onenand_reset()
which will set bdrv_cur to bdrv.

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]