Am 26.07.2013 04:58, schrieb Hu Tao:
> On Thu, Jul 25, 2013 at 01:16:51AM +0200, Andreas Färber wrote:
>> Let pl110_versatile and pl111 inherit from pl110 and use PL110() cast;
>> set their version index in an instance_init.
>>
>> Signed-off-by: Andreas Färber <afaer...@suse.de>
>> ---
>> hw/display/pl110.c | 71
>> ++++++++++++++++++++++--------------------------------
>> 1 file changed, 29 insertions(+), 42 deletions(-)
>>
>
> <...>
>
>> -static int pl110_init(SysBusDevice *dev)
>> +static int pl110_init(SysBusDevice *sbd)
>> {
>> - PL110State *s = FROM_SYSBUS(PL110State, dev);
>> + DeviceState *dev = DEVICE(sbd);
>> + PL110State *s = PL110(dev);
>>
>> memory_region_init_io(&s->iomem, OBJECT(s), &pl110_ops, s, "pl110",
>> 0x1000);
>> - sysbus_init_mmio(dev, &s->iomem);
>> - sysbus_init_irq(dev, &s->irq);
>> - qdev_init_gpio_in(&s->busdev.qdev, pl110_mux_ctrl_set, 1);
>> - s->con = graphic_console_init(DEVICE(dev), &pl110_gfx_ops, s);
>> + sysbus_init_mmio(sbd, &s->iomem);
>> + sysbus_init_irq(sbd, &s->irq);
>> + qdev_init_gpio_in(dev, pl110_mux_ctrl_set, 1);
>> + s->con = graphic_console_init(dev, &pl110_gfx_ops, s);
>> return 0;
>
> Would it be better to explicitly set PL110 version? I know PL110 is 0,
> but it's not like a normal variable which is always good to be
> initialized to 0, assuming a version number is bad. (This is not a problem
> of this patch)
Sure, I can add such a no-op line. Thanks for your watchful review!
However I'd like to point out that these series are tailored for 1.6, so
I tried to go with a "safe" set of changes (e.g., leaving out most QOM
realize conversions). I think a post-1.6 cleanup would be to move such
static instance fields into a new PL110Class, set in different
..._class_init rather than ..._init functions.
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
