On Wed, Oct 16, 2013 at 06:06:14PM -0700, Christoffer Dall wrote:
> There's an interesting mix of space and tab indentations in this file...
>
> I assume using the kernel coding style would be approprate here...
> Actually, that could be added to the readme too. (Or whichever format we
> choose).
>
> Could you fix?
I was attempting to adopt the style from the common code lib/*, see
lib/string.c, lib/printf.c, lib/argv,c. The style used there is
1. 'four spaces'
2. 'tab'
3. 'tab + four spaces'
4. 'tab + tab'
...
However, I'd actually prefer to use the kernel style as well, and I see
that not even those three common files are consistent. argv.c only uses
spaces. Other x86 files also have a variety of styles (kernel style
being one of them). So probably the best choice is to go with kernel
style, and to add another cleanup patch that changes lib/* files as
well.
> > + for (i = 0; i < m->nr; ++i) {
> > + volatile u32 *base = (u32 *)compat_ptr(m->addrs[i]);
> > + u32 devid32 = base[VIRTIO_MMIO_DEVICEID / 4];
>
> do we have readl/writel in this framework? If not, maybe we should to
> indicate IO read/writes and ensure the compiler doesn't reorder things
> and that we have the necessary memory barriers etc...?
>
> That would apply to virtio_testdev above as well.
OK, I agree it makes sense to add readl and writel in. I'll do that, and
then rework my mmio reads to use them.
> + printf("Refusing to run with virtio-testdev major = %d, minor = %d\n",
> + major, minor);
> "Refusing to run"?
> How about "incompatible version of virtio-testdev"?
Sure. OK.
drew
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
