On Thu, Oct 17, 2013 at 11:51:26AM +0200, Andrew Jones wrote:
> 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'
> ...
that sounds like the most horrible broken coding style I have ever heard
of. At least it reads like sh*t in mutt.
>
> 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];
Yeah, I think it's is by far the easiest and most convenient for
everyone.
> >
> > 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
