Subject: [PATCH] fpga zynq: Check the bitstream for validity

From: [email-masked] (Jason Gunthorpe)
Date: Fri, 28 Oct 2016 10:55:55 -0600

On Fri, Oct 28, 2016 at 06:36:06PM +0200, Matthias Brugger wrote:

> Sure but we are checking here that the bitstream passed to the kernel is
> correct.

The intent to check if it *possible* that the bitstream is
correct. Correct means that DONE will assert after programming. A 4
byte bitstream will never assert DONE.

Arguably the threshold should be larger but we don't know what the
true minimum is.

> +	/* All valid bitstreams are multiples of 32 bits */
> +	if (!count || (count % 4) != 0)
> +		return -EINVAL;
> +

Too clever for my taste.

Aside from this, is the general idea even OK? In my world I cannonize
the bitstream to start with the sync word, but others may not be doing
that.

I designed this patch based on the prior work to remove the
auto-detection, eg that the driver should be passed a canonized
bitstream.

The problem with the way it is now is how hard it is to figure out
what the right bitstream format should be. Clear instructions to
canonize by droping the header before the sync word and byteswap so
the sync word is in the given order is much clearer..

Jason