Re: [PATCH 2/2] New driver: Xillybus generic interface for FPGA(programmable logic)

From: Eli Billauer
Date: Fri Nov 30 2012 - 12:24:05 EST


On 11/30/2012 06:32 PM, Greg KH wrote:

As we need to review the user/kernel api here, putting the docs as part
of the driver submission is a good idea :)

I didn't know, nor do I trust, that a random web site would have the
correct documentation for a kernel driver.
OK. I'll add a file in Documentation/misc-devices/.
+#if (PAGE_SIZE< 4096)
+#error Your processor architecture has a page size smaller than 4096
+#endif
That can never happen. Even if it does, you don't care about that in
the driver.

I removed this check because it can't happen. But the driver *does*
care about this, since it creates a lot of buffers with different
alignments, hence depending on the pages' alignment.
Alignment is different than the size of a page. What happens if your
driver runs on a machine with a page size bigger than 4K? You need to
be able to handle that properly, so perhaps you should check that?
The problem is if the page size *smaller* than 4kB. The buffers allocated by the driver must not cross a 4kB boundary, and it's assumed that anything returned by __get_free_pages() is 4 kB-aligned. Otherwise the FPGA will generate illegal PCIe packets by crossing that boundary.

If the page boundary is bigger than 4k, the driver handles that well.


It is no problem to create dozens of misc devices. It makes your driver
smaller, contain less code that I have to audit and you have to ensure
you got right, and it removes another user of 'struct class' which we
are trying to get rid of anyway. So please, move to use a misc device.
Very well. I'll remove that.

Thanks again for your comments. I'll prepare a v3.

Eli

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/