Re: [Qemu-devel] [RFC PATCH 0/7] QEMU patches to generate FDT from qdevs

[Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>]

Thanks for the comments.  Replies below...

On Wed, Apr 7, 2010 at 1:10 PM, Blue Swirl <blauwirbel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 4/7/10, Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org> wrote:
>> Hi everyone,
>>
>>  This is an experimental set of patches for populating the flattened
>>  device tree (fdt) data from the actual set of qdevs in the platform.
>>  I'm not expecting this to get merged anytime soon, but I wanted to get
>>  it out there to solicit comments.  My target for this is testing
>>  device tree support on ARM.
>>
>>  The approach is by no means final, and there are still a number of
>>  details to work out, but I've got it working well with the versatile
>>  platform and I've got the kernel using fdt data to bind against the
>>  smc91x device driver.  I'll be posting my associated kernel patches
>>  real-soon-now.
>
> It looks like the hook is only used to add some additional properties.
> I think a data driven approach (fields in qdev structure) would be
> simpler because the handling code would be only in qdev instead of all
> devices.

Hmmm, maybe.  I don't really have a good answer yet as I'm still
experimenting.  It is possible that I'll need more complex behaviour
as I add new kinds of devices or for handling different bus types.
For example, right now I've got code to change the node name for mmio
devices, and that cannot be filled in until the register base is
defined and the best name format may be different for different
busses/devices.  I'm not familiar enough with qemu to know how best to
handle this with properties.

> Something like:
> static SysBusDeviceInfo smc91c111_info = {
>    .init = smc91c111_init1,
>    .qdev.name  = "smc91c111",
>    .qdev.size  = sizeof(smc91c111_state),
>    .qdev.props = (Property[]) {
>        DEFINE_NIC_PROPERTIES(smc91c111_state, conf),
>        DEFINE_PROP_END_OF_LIST(),
>    },
>    .qdev.fdt_props = (FDTProperty[]) {
>        DEFINE_FDT_NIC_MACADDR("local-mac-address", smc91c111_state,
> conf.macaddr),
>        DEFINE_FDT_PROP_STR("compatible", "smc,91c111"),
>        DEFINE_FDT_PROP_END_OF_LIST(),
>    }
> };
>
> A separate question is whether the properties should be visible to
> user, for example shown by "info qdev". If yes, then the FDT
> properties should be a special case of qdev properties:
>    .qdev.props = (Property[]) {
>        DEFINE_NIC_PROPERTIES(smc91c111_state, conf),
>        DEFINE_FDT_NIC_MACADDR("local-mac-address", smc91c111_state,
> conf.macaddr),
>        DEFINE_FDT_PROP_STR("compatible", "smc,91c111"),
>        DEFINE_PROP_END_OF_LIST(),
>    }

It would certainly be useful to see the device node(s) associated with a qdev.

g.