[PATCH] fpga zynq: Check the bitstream for validity

[moritz.fischer@ettus.com (Moritz Fischer)]

On Fri, Oct 28, 2016 at 10:55:55AM -0600, Jason Gunthorpe wrote:
> 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.

I'm fine with checking for boundary cases where the bitstreams are
obviously too small or wrong size, I'm not convinced that checking using
internal knowledge about the bistream format inside the kernel is the
right place.

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

I don't know about you, but for my designs I can just use what drops out
of my Vivado build. Are you unhappy with the way we document which
format to use, or that we don't slice off the beginning (that gets
ignored by hardware?).

Thanks for addressing the issues with the length calculations,

Moritz