Thanks Yilun,
It looks like I'm using a mainline 6.1.55 kernel, but with Xilinx
patches over it.
One of those Xilinx kernel patches introduces loading firmware images
via the sysfs, and one of those functions calls
fpga_mgr_firmware_load() directly,
without first setting info->header_size (as fpga_mgr_load does).
I guess I'll shoot a message to the Xilinx guys.
Thanks!
-DG
On Thu, Apr 11, 2024 at 6:48 PM Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx> wrote:
>
> On Thu, Apr 11, 2024 at 02:16:24PM +1000, David Gideon wrote:
> > Hi Everyone,
> >
> > Our Xilinx FPGA driver (zynq-fpga.c) is no longer compatible with
> > fpga-mgr.c and barfs with:
> >
> > "Invalid bitstream, could not find a sync word. Bitstream must
> > be a byte swapped .bin file"
> >
> > It seems to come from here (inside fpga_mgr_write_init_buf()):
> > size_t header_size = info->header_size;
> >
> > It uses header_size to decide whether or not to call
> > fpga_mgr_write_init() with a NULL buffer and a 0 size, or use a real
> > buffer and real size, like this:
> >
> > if (header_size > count)
> > ret = -EINVAL;
> > else if (!header_size)
> > ret = fpga_mgr_write_init(mgr, info, NULL, 0);
> > else
> > ret = fpga_mgr_write_init(mgr, info, buf, count);
> >
> > The trouble is, that if I follow my code path, info->header_size isn't
> > set by anyone. So it's **ALWAYS** zero. But the value **should** be
>
> From your link below, I see info->header_size is set here:
>
> @@ -404,6 +571,8 @@ static int fpga_mgr_firmware_load(struct fpga_manager *mgr,
> */
> int fpga_mgr_load(struct fpga_manager *mgr, struct fpga_image_info *info)
> {
> + info->header_size = mgr->mops->initial_header_size;
> +
> if (info->sgt)
> return fpga_mgr_buf_load_sg(mgr, info, info->sgt);
>
> Thanks,
> Yilun
>
> > 128:
> >
> > static const struct fpga_manager_ops zynq_fpga_ops = {
> > .initial_header_size = 128,
> > ...
> > };
> >
> > The issue seems to have been introduced as part of commit 3cc624beba
> > which I have linked to here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/fpga/fpga-mgr.c?id=3cc624beba6310a8a534fb00841f22445a200d54
> >
> >
> > What I find really interesting is that the original mailing list patch
> > submission didn't have this bug, and would have worked for us:
> >
> > + if (info->header_size)
> > + header_size = info->header_size;
> > + else
> > + header_size = mgr->mops->initial_header_size;
> >
> > Could this be a viable way to ensure that the FPGA manager driver is
> > compatible with the zynq-fpga.c driver again?
> >
> >
> > - DG
> >
