On Thu, Jan 02, 2014 at 09:23:38AM -0800, Christoffer Dall wrote:
> On Thu, Jan 02, 2014 at 05:04:48PM +0100, Andrew Jones wrote:
> > On Sat, Dec 28, 2013 at 10:30:54PM -0800, Christoffer Dall wrote:
> > > > +struct iomap {
> > > > + const char *type;
> > > > + const char *compats[5];
> > >
> > > I would name the field compatible to be more in-line with the
> > > DT-representation, but OK.
> > >
> > > it looks from the above like the array must be null terminated?
> > >
> > > how about #define IOMAP_MAX_COMPATS 5
> >
> > defines are good.
> >
> > > and then turn your loop above into:
> > >
> > > for (i = 0; i < IOMAP_MAX_COMPATS; i++, m++) {
> > > if (!m->compats[i])
> > > break;
> >
> > Doesn't improve the loop conditions much, IMO.
> >
>
> I think it's much simpler to read a for loop of the typical form of
> until "i < CONSTANT"; what you had before was a data structure with an
> assumption of null-termination that could only be realized by looking at
> the code that deals with it.
>
> > > if (strcmp(m->compats[i], compat) == 0)
> > > return m;
> > > }
> > >
> > > > + u32 nr;
> > > > + u32 addrs[64];
> > >
> > > why are we limiting ourselves to a 32 bit physical address space?
> >
> > I have had this mentally noted from the beginning as something I'll
> > need to deal with when getting aarch64 going. Anyway, I think a lot
> > of this is going to change as I teach kvm-unit-tests more and more
> > about DT.
>
> Shouldn't be a big change to just support u64 in your iomap instead.
>
> >
> > <snip>
> > > > +print "{\n\t.type = NULL,\n},\n";
> > > > +print "};\n";
> > > > +exit 0;
> > >
> > > This script doesn't work on either of the Ubuntu distros I run. The
> > > reasons are that the dumpfdt tool is not processing the multi-compatible
> > > strings output from the dtb correctly and the fdtget utility included
> > > does not yet have the -l option to list subnodes.
> > >
> > > I spent a fair amount of time trying to fix this, changing dumpfdt to
> > > 'dtc -I dtb -O dts', and I tried it on the newest Ubuntu distro, tried
> > > compiling fdtget from the kernel sources etc. and failed miserably.
> > >
> > > I think at the very least we need to check the tools available on the
> > > build machine as part of the configure script or test it a little
> > > broader.
> > >
> >
> > Drat. I've preferred the idea of using the tools to parse the DT during
> > the build - avoiding the need for [cross-compiled] libfdt as a dep, but
> > I guess as we need more and more from the DT, or when particular tests
> > want to look at the DT itself, and there are already dependency problems,
> > then it's looking more and more like we should just use libfdt.
> >
> Yes, after working with this for a while that's the same conclusion I
> came to. From the ARM perspective, anything running in a VM will be
> device-tree driven (this is not the time for anybody to say anything
> about ACPI), so we could just feed the VM the device tree directly if we
> support libfdt in the guest and avoid another point of failure. I can
> try to find some time to help you hack that up if you want.
I'll start this work right away. Peter's import libfdt idea sounds like
the perfect solution.
drew
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
