From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752246AbbJMFxl (ORCPT ); Tue, 13 Oct 2015 01:53:41 -0400 Received: from mx18-05.smtp.antispamcloud.com ([207.244.64.174]:55135 "EHLO mx18-05.smtp.antispamcloud.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751583AbbJMFxg convert rfc822-to-8bit (ORCPT ); Tue, 13 Oct 2015 01:53:36 -0400 X-Greylist: delayed 1161 seconds by postgrey-1.27 at vger.kernel.org; Tue, 13 Oct 2015 01:53:36 EDT Subject: Re: [PATCH 3/3] fpga manager: Adding FPGA Manager support for Xilinx Zynq 7000 To: Michal Simek , Moritz Fischer , Josh Cartwright References: <1444344307-22509-1-git-send-email-moritz.fischer@ettus.com> <1444344307-22509-4-git-send-email-moritz.fischer@ettus.com> <20151009163336.GM10631@jcartwri.amer.corp.natinst.com> <561B9687.1070103@xilinx.com> <561BA60C.6010106@topic.nl> <561BA9C7.6070003@xilinx.com> CC: , , Russell King , "pawel.moll@arm.com" , , Alan Tull , Greg KH , , , linux-arm-kernel , Kumar Gala , "dinguyen@opensource.altera.com" , =?UTF-8?Q?S=c3=b6ren_Brinkmann?= From: Mike Looijmans Organization: TOPIC Message-ID: <561C979B.6040201@topic.nl> Date: Tue, 13 Oct 2015 07:33:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <561BA9C7.6070003@xilinx.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8BIT X-Originating-IP: [192.168.80.121] X-EXCLAIMER-MD-CONFIG: 9833cda7-5b21-4d34-9a38-8d025ddc3664 X-EXCLAIMER-MD-BIFURCATION-INSTANCE: 0 X-Filter-ID: s0sct1PQhAABKnZB5plbIbbvfIHzQjPVmPLZeVYSu3xU9luQrU+8/8qthi+0Jd/W6KAUC/fjyuDn NXFr4uarw6ZD8veLNImDrLDLUiI0AsfAxQbzAqS/RrKKBO4XjlCmeiXIHO35MjHi+eMUCJO2TIYS KBStr7O7vhgxWoFv+epHsjAzwFm2j3sTkEoNu4MFB2XwnnukAbufhj1XYHptaOBfOzFu1iolzW0d Y33uxGtdxolPvBTHlpbbkINvd6Bk6kKV7RLNv7BG4By/UppOuua3jN9Ol113WVdbZbJ4n5SZL3ZK wOcZxaNA3uilioqXHcgm/PYDrgNMeTyYrm6w0A4y43vx9CMfMwdzHKPokH4t76uq2Ikiy7EGEqrS bHxU+P6wwL/Ww4aRJJTYokACQkfkeB1ZozLmEzIAIV2u3PV5DC1pPwtLyzZS88a5qsmrV9H3b3ui +WTkUNrPUTJq27t8OmX1QQGT6C+RTkXg4eT3mGwqeVvV/aoJ9vMsXgu3CSITgN0ugmTS4qKyldkj gGuolx+n8+nrio+Py/D/FV+6lXUJF8c7DPYmIHr06w4X6DLR8Bobhy37nGTUDYXtQigFalNTNgHB 4p0JraSIHbuWQAst2UnlyAFeCH1nr+hjJtn64OSEWF6VcqmDPMWxhzZQ6OtBjX2dhf8r1xeX6dF8 69ddH5pVD4vk5Nhn8kevTec4ARO/9+ZXzwpBF2qmFrnE3ugZYR5DLT24smR4NCxcWwY4V4wSoeUf 4Tc54x6CPIwfcaABxEbSYmuqEIS+ddsVLmYwaaTRpYEGBzS2lSZu1/TOKAdM9bXEcN/54aPl31/E 3ahF5MMcDI7KdpjQKfBca56p9rSaMLHvrFJ/WXoVQQwJ+hLFkhiU5Z37xpMwJsxPsHZhqPAsfqcG efkyHPGq+LDvZGJq+/55nIT0fRv2Kv2ll81eEp+cNoocKNYZp81e65KEKhwtHEfZhshBX4LXS7oI SxfhqpHGPSL93xmPOfJmdPm/r8qHL+iIE/rPxW1Tvts9rK7u4IJwgRglzGvDK3PKrieG7T6bf9hS 7hSGyAxXU04XUx7LlovNt/SF7WoFt+fybNHE73/Q19FC8dDnSshojicvqBOxdT4qb4QU3DgtRxzg sGCElWfUFIwuDpp20HY57zx02G6RARnCiHMcN6qoXPjenLhIOF1oeRZCN7W8OFUXuwrACaSQJxIT eRx6eGt6IMoFClIjgmg4vJ9znrtJX3VbIT1bQrHRaTUzuAJHjLnD6pcynQvLMFtD X-Report-Abuse-To: spam@mx99.antispamcloud.com X-Filter-Fingerprint: IFrWXGses7OKB5S5G8/dJUb3OPwsHaH0Fvg5oXltHd/JUWjZ8+qhjyB23tbDuyLOYL8Ff78gYsez 4Rl08xudmXi4esCQ0R1MchVjt7wblGlvhFgW0MjUMRkF5sMCDfftTXNFDzN17hnrWeZYOJvLq0Ic WjZ+XcEjj/7Pkld0zkmvziDInX9WdMov2kn2yXjdwv61T+KDYyYtREgszdyFwv8IxCB3p/oCKvxr eyISh3JGb7OS5oVgiO+kDxZrVPLz3MmEGC2PrUKqLq5WmHK+Nw== X-Originating-IP: 88.159.208.100 X-Spampanel-Domain: topic.nl X-Spampanel-Username: 88.159.208.100 Authentication-Results: antispamcloud.com; auth=pass smtp.auth=88.159.208.100@topic.nl X-Spampanel-Outgoing-Class: ham X-Spampanel-Outgoing-Evidence: Combined (0.00) X-Recommended-Action: accept Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 12-10-15 14:38, Michal Simek wrote: > Hi Mike, > > On 10/12/2015 02:22 PM, Mike Looijmans wrote: >> On 12-10-15 13:16, Michal Simek wrote: >>> >>>>>> +static int zynq_fpga_ops_write(struct fpga_manager *mgr, >>>>>> + const char *buf, size_t count) >>>>>> +{ >>>>>> + struct zynq_fpga_priv *priv; >>>>>> + int err; >>>>>> + char *kbuf; >>>>>> + size_t i, in_count; >>>>>> + dma_addr_t dma_addr; >>>>>> + u32 transfer_length = 0; >>>>>> + bool endian_swap = false; >>>>>> + >>>>>> + in_count = count; >>>>>> + priv = mgr->priv; >>>>>> + >>>>>> + kbuf = dma_alloc_coherent(priv->dev, count, &dma_addr, >>>>>> GFP_KERNEL); >>>>>> + if (!kbuf) >>>>>> + return -ENOMEM; >>>>>> + >>>>>> + memcpy(kbuf, buf, count); >>>>>> + >>>>>> + /* look for the sync word */ >>>>>> + for (i = 0; i < count - 4; i++) { >>>>>> + if (memcmp(kbuf + i, "\x66\x55\x99\xAA", 4) == 0) { >>>>>> + dev_dbg(priv->dev, "Found normal sync word\n"); >>>>>> + endian_swap = false; >>>>>> + break; >>>>>> + } >>> >>> This is bin format >>> >>>>>> + if (memcmp(kbuf + i, "\xAA\x99\x55\x66", 4) == 0) { >>>>>> + dev_dbg(priv->dev, "Found swapped sync word\n"); >>>>>> + endian_swap = true; >>>>>> + break; >>>>>> + } >>> >>> This is bit format from header >>> >>>>>> + } >>>>> >>>>> How much control do we have over mandating the format of firmware at >>>>> this point? It'd be swell if we could just mandate a specific >>>>> endianness, and leave this munging to usermode. >>>> >>>> That's a good question. Personally I do only care about one of both, >>>> but that's just because I get to decide for my targets... >>>> Opinions from the Xilinx guys? >>> >>> Don't know full history about this but in past bitstream in BIT format >>> was used. Which is header (partially decoding in u-boot for example) >>> with data. >>> On zynq native format is BIN which is format without header and data is >>> swapped. >>> This code just detects which format is used. If BIT, header is skipped >>> and data is swapped to BIN format. >>> >>> Back to origin question if this is something what can be handled from >>> user space. And answer is - yes it can be handled there. >>> But based on my experience it is very useful to be able to handle BIT >>> because it is built by tools by default. >>> Also with BIN format you are loosing record what this data bitstream >>> targets. Header in BIT gives you at least some ideas. >> >> People should stop using "cat" to program the FPGA and use a userspace >> tool instead. I've already released such tools under GPL, so anyone can >> pick up on it and extend it as required. > > Link? https://github.com/topic-embedded-products/dyplo-utils/blob/master/dyploprogrammer.cpp https://github.com/topic-embedded-products/libdyplo/blob/master/hardware.cpp#L261 Will need some work to combine into a single tool though. > This is fpga manager based driver where "cat" won't be used. Haven't looked into it yet, but I guess at some point one will have to stream some data from userspace into the device, right? >> The header for the "bit" format is completely ignored (you can't even >> use it to determine if the bitstream is compatible with the current >> device) so there's no point in carrying it around. > > up2you what you want to do with it. If you work with different boards > with different FPGAs it is at least helpful to know if X.bit target this > or that board. Unfortunately I am not aware about any public document > which describe what there is written. > >> On the zynq, doing >> the "swap" in userspace was measurably faster than having the driver >> handle it, and that was even without using NEON instructions for byte >> swapping. >> >> I admit that being able to do "cat static.bit > /dev/xdevcfg" has had >> its uses. But it's not something that belongs in mainline Linux. > > It is about comfort but I have really not a problem that driver will > handle just BIN format. > >> Probably one of the key reasons that the "bit" format is still popular >> is that getting the Vivado tools to create a proper "bin" that will >> actually work on the Zynq is about as easy as nailing jelly to a tree. >> We've been using a simple Python script to do the bit->bin conversion >> for that reason. > > In vivado it is one tcl cmd. But truth is that I don't really get why > BIN is not generated by default. If I recall correctly, Vivado strips the "bit" header but doesn't swap the bytes, so the resulting bin file won't work. >> Using the "bin" format in the driver keeps it simple and singular. >> Userspace tools can add whatever wrappers and headers they feel >> appropriate to it, these checks don't belong in the driver since they >> will be application specific. For example, some users would want to >> verify that a partial bitstream actually matches the static part that's >> currently in the FPGA. > > agree. > > Thanks, > Michal > > Kind regards, Mike Looijmans System Expert TOPIC Embedded Products Eindhovenseweg 32-C, NL-5683 KH Best Postbus 440, NL-5680 AK Best Telefoon: +31 (0) 499 33 69 79 Telefax: +31 (0) 499 33 69 70 E-mail: mike.looijmans@topicproducts.com Website: www.topicproducts.com Please consider the environment before printing this e-mail