Re: [PATCH 6/9] Introduce a simple iomap structure

[Andrew Jones <drjones@redhat.com>]

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.

> 	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.

<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.

drew