[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init |
Date: |
Sun, 27 Sep 2015 11:32:13 -0700 |
On Sun, Sep 27, 2015 at 10:51 AM, Max Filippov <address@hidden> wrote:
> On Sun, Sep 27, 2015 at 8:25 PM, Peter Crosthwaite
> <address@hidden> wrote:
>> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <address@hidden> wrote:
>>> pflash_cfi01_register always registers FLASH to the default
>>> system_memory region, which may not always be the right thing.
>>>
>>> Provide pflash_cfi01_init function that only creates FLASH device and
>>> sets its properties, leaving MMIO region registration to its caller.
>>>
>>
>> You should just use QOM to create your device inline without need for
>> a construction helper. See Vexpress's ve_pflash_cfi01_register for an
>> example.
>
> I thought about it and it looks like a lot of boilerplate code for nothing.
Not really. It self documents the code. This is instantly understandable:
qdev_prop_set_uint32(dev, "num-blocks",
VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE);
qdev_prop_set_uint64(dev, "sector-length", VEXPRESS_FLASH_SECT_SIZE);
qdev_prop_set_uint8(dev, "width", 4);
qdev_prop_set_uint8(dev, "device-width", 2);
qdev_prop_set_bit(dev, "big-endian", false);
qdev_prop_set_uint16(dev, "id0", 0x89);
qdev_prop_set_uint16(dev, "id1", 0x18);
qdev_prop_set_uint16(dev, "id2", 0x00);
qdev_prop_set_uint16(dev, "id3", 0x00);
qdev_prop_set_string(dev, "name", name);
Whereas a call to pflash_cfi_register is a jumble of numbers without
any labelling of what is what. In this scheme you don't have to open
headers to figure out what args are.
> Maybe it'd be better to remove qdev_init_nofail call from that init function
> and let ve_pflash_cfi01_register reuse it too?
>
There was a conscious decision some time ago to get rid of qdev init
helper functions as provided by devs themselves. The interface between
dev and board should be QOM.
Regards,
Peter
> --
> Thanks.
> -- Max
[Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Max Filippov, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Peter Crosthwaite, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Max Filippov, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Peter Crosthwaite, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Max Filippov, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Peter Crosthwaite, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Max Filippov, 2015/09/27
- Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores, Peter Crosthwaite, 2015/09/27