Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
> On 23. Feb 2023, at 09:28, Ulf Samuelsson wrote: > > CAUTION: External Email!! > > > > On 2023-02-22 20:51, Steffen Dirkwinkel wrote: >> Hi! >> On Tue, 2023-02-21 at 11:58 +0100, Ulf Samuelsson wrote: >>> CAUTION: External Email!! >>> Hi Guys, >>> We are currently discussing how to support passive serial configuration >>> of FPGAs in u-boot. >>> >>> Checking the u-boot source code reveals that only your boards >>> actually supports this. >>> >>> board/beckhoff/mx53cx9020/mx53cx9020.c:...Altera_CYC2_Passive_Serial ... >>> $ scripts/get_maintainer.pl board/beckhoff/mx53cx9020/mx53cx9020.c >>> Patrick Bruenn (maintainer:MX53 CX9020) >> This doesn't seem immediately relevant to our board as we don't really use >> spi, >> but the (w)eim interface as far as i can tell right now. (has been a while >> since I looked >> at it, have not read the imx datasheet again to see if this is spi in >> disguise) >> If anything the binding should be shared between u-boot and the kernel. We >> just had a bug >> because of differences between the two. >> Something like the "altr,fpga-passive-serial" in >> arch/arm/boot/dts/imx6q-evi.dts in the kernel would match the >> usecase. >> We wouldn't be opposed to changing the dt and also just repaired our CI for >> these devices and can test changes quickly. > I had a look at your code again. > It appears that you are using the Altera "passive serial" interface in > U-Boot, but your board routine appears to write 8-bit bytes to the external > bus. > > The only two reasons for this I can think of is that you are either > using an external serializer converting the 8 bit words to > a serial stream or you are actually using the passive "parallell > interface" on the FPGA, but use the "passive serial" routines > in u-boot. > > Can you please confirm how the hardware works? I checked the schematic, we’re using fast_passive_parallel. Since both are using the same codepaths right now it didn’t matter. I’ll send a patch > > > The maintainer of the other board using "passive serial" > (w.weg...@astro-kom.de ) > bounced as an unknown user. This means that the "board/astro/mcf5373" does > not have an active MAINTAINER. > > The board was added in 2010, and has not seen any updates from > https://nospamproxywebp.beckhoff.com/enqsig/link?id=BCBGd7m3lfMOLOIhx3uNR_1Qav4Ffx8-jQB0torD82ts_G8AAAB_Q13kS5en5WiHBszxeeaYjAQwTPXiCeF8gx_zVjByVYRebkfOfUTW4VnpSmQ2VlvWczqoVxWwP7qF1C4VccAGIB-3QCgtp-eMVgM5VcCkSRCJT8HehzvaoUnQ6YYu2ofkD0GMhgmICiFDb8Cv_2k1 > since then. All the maintenance has been by people maintaining subsystems. > > Best Regards > Ulf Samuelsson > > >> Best Regards >> Steffen Dirkwinkel >>> >>> board/astro/mcf5373l/fpga.c:Altera_CYC2_Passive_Serial ... >>> $ scripts/get_maintainer.pl board/astro/mcf5373l/fpga.c >>> Wolfgang Wegner (maintainer:MCF5373L BOARD) >>> >>> The proposed ideas is to have the FPGA as an SPI peripheral >>> in the device tree. >>> >>> I have a working version, where the FPGA is an MTD device >>> and the general idea is to add it to the FPGA manager, >>> which unfortunately needs a major rewrite to support device tree. >>> >>> Your board support configuring the FPGA in the 'board' files. >>> >>> >>> * Are these boards still active and will require updates of u-boot? >>> >>> * If so, would it be a problem porting the FPGA configuration to the >>> device tree? >>> >>> Marek does not want two drivers supporting passive serial, >>> and supporting both 'board' drivers and devicetree drivers >>> seems to result in unclean code. >>> >>> Best Regards >>> Ulf Samuelsson >> This email contains confidential information. If you have received it in >> error, you must not read, use, copy or pass on this e-mail or its >> attachments. If you have received the e-mail in error, please inform me >> immediately by reply e-mail and then delete this e-mail from your system. >> Thank you >> Diese E-Mail enthält vertrauliche Informationen. Sollten Sie sie irrtümlich >> erhalten haben, dürfen Sie diese E-Mail oder ihre Anhänge nicht lesen, >> verwenden, kopieren oder weitergeben. Sollten Sie die Mail versehentlich >> erhalten haben, teilen Sie mir dies bitte umgehend per Antwort-E-Mail mit >> und löschen Sie diese E-Mail dann aus Ihrem System. Vielen Dank >> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans >> Beckhoff >> Registered office: Verl, Germany | Register court: Guetersloh HRA 7075 This email contains confidential information. If you have received it in error, you must not read, use, copy or pass on this e-mail or its attachments. If you have received the e-mail in error, please inform me immediately by reply e-mail and then delete this e-mail from your system. Thank you Diese E-Mail enthält vertrauliche Informationen. Sollten Sie sie irrtümlich erhalten haben, dürfen Sie diese E-Mail oder ihre Anhänge nicht lesen, verwenden, kopieren oder weitergeben. Sollten Sie die Mail versehentlich erhalten haben, teilen Sie mir dies
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
On 2023-02-22 20:51, Steffen Dirkwinkel wrote: Hi! On Tue, 2023-02-21 at 11:58 +0100, Ulf Samuelsson wrote: CAUTION: External Email!! Hi Guys, We are currently discussing how to support passive serial configuration of FPGAs in u-boot. Checking the u-boot source code reveals that only your boards actually supports this. board/beckhoff/mx53cx9020/mx53cx9020.c:...Altera_CYC2_Passive_Serial ... $ scripts/get_maintainer.pl board/beckhoff/mx53cx9020/mx53cx9020.c Patrick Bruenn (maintainer:MX53 CX9020) This doesn't seem immediately relevant to our board as we don't really use spi, but the (w)eim interface as far as i can tell right now. (has been a while since I looked at it, have not read the imx datasheet again to see if this is spi in disguise) If anything the binding should be shared between u-boot and the kernel. We just had a bug because of differences between the two. Something like the "altr,fpga-passive-serial" in arch/arm/boot/dts/imx6q-evi.dts in the kernel would match the usecase. We wouldn't be opposed to changing the dt and also just repaired our CI for these devices and can test changes quickly. I had a look at your code again. It appears that you are using the Altera "passive serial" interface in U-Boot, but your board routine appears to write 8-bit bytes to the external bus. The only two reasons for this I can think of is that you are either using an external serializer converting the 8 bit words to a serial stream or you are actually using the passive "parallell interface" on the FPGA, but use the "passive serial" routines in u-boot. Can you please confirm how the hardware works? The maintainer of the other board using "passive serial" (w.weg...@astro-kom.de ) bounced as an unknown user. This means that the "board/astro/mcf5373" does not have an active MAINTAINER. The board was added in 2010, and has not seen any updates from astro-kom.de since then. All the maintenance has been by people maintaining subsystems. Best Regards Ulf Samuelsson Best Regards Steffen Dirkwinkel board/astro/mcf5373l/fpga.c:Altera_CYC2_Passive_Serial ... $ scripts/get_maintainer.pl board/astro/mcf5373l/fpga.c Wolfgang Wegner (maintainer:MCF5373L BOARD) The proposed ideas is to have the FPGA as an SPI peripheral in the device tree. I have a working version, where the FPGA is an MTD device and the general idea is to add it to the FPGA manager, which unfortunately needs a major rewrite to support device tree. Your board support configuring the FPGA in the 'board' files. * Are these boards still active and will require updates of u-boot? * If so, would it be a problem porting the FPGA configuration to the device tree? Marek does not want two drivers supporting passive serial, and supporting both 'board' drivers and devicetree drivers seems to result in unclean code. Best Regards Ulf Samuelsson This email contains confidential information. If you have received it in error, you must not read, use, copy or pass on this e-mail or its attachments. If you have received the e-mail in error, please inform me immediately by reply e-mail and then delete this e-mail from your system. Thank you Diese E-Mail enthält vertrauliche Informationen. Sollten Sie sie irrtümlich erhalten haben, dürfen Sie diese E-Mail oder ihre Anhänge nicht lesen, verwenden, kopieren oder weitergeben. Sollten Sie die Mail versehentlich erhalten haben, teilen Sie mir dies bitte umgehend per Antwort-E-Mail mit und löschen Sie diese E-Mail dann aus Ihrem System. Vielen Dank Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Hi! On Tue, 2023-02-21 at 11:58 +0100, Ulf Samuelsson wrote: > CAUTION: External Email!! > Hi Guys, > We are currently discussing how to support passive serial configuration > of FPGAs in u-boot. > > Checking the u-boot source code reveals that only your boards > actually supports this. > > board/beckhoff/mx53cx9020/mx53cx9020.c:...Altera_CYC2_Passive_Serial ... > $ scripts/get_maintainer.pl board/beckhoff/mx53cx9020/mx53cx9020.c > Patrick Bruenn (maintainer:MX53 CX9020) This doesn't seem immediately relevant to our board as we don't really use spi, but the (w)eim interface as far as i can tell right now. (has been a while since I looked at it, have not read the imx datasheet again to see if this is spi in disguise) If anything the binding should be shared between u-boot and the kernel. We just had a bug because of differences between the two. Something like the "altr,fpga-passive-serial" in arch/arm/boot/dts/imx6q-evi.dts in the kernel would match the usecase. We wouldn't be opposed to changing the dt and also just repaired our CI for these devices and can test changes quickly. Best Regards Steffen Dirkwinkel > > board/astro/mcf5373l/fpga.c:Altera_CYC2_Passive_Serial ... > $ scripts/get_maintainer.pl board/astro/mcf5373l/fpga.c > Wolfgang Wegner (maintainer:MCF5373L BOARD) > > The proposed ideas is to have the FPGA as an SPI peripheral > in the device tree. > > I have a working version, where the FPGA is an MTD device > and the general idea is to add it to the FPGA manager, > which unfortunately needs a major rewrite to support device tree. > > Your board support configuring the FPGA in the 'board' files. > > > * Are these boards still active and will require updates of u-boot? > > * If so, would it be a problem porting the FPGA configuration to the > device tree? > > Marek does not want two drivers supporting passive serial, > and supporting both 'board' drivers and devicetree drivers > seems to result in unclean code. > > Best Regards > Ulf Samuelsson This email contains confidential information. If you have received it in error, you must not read, use, copy or pass on this e-mail or its attachments. If you have received the e-mail in error, please inform me immediately by reply e-mail and then delete this e-mail from your system. Thank you Diese E-Mail enthält vertrauliche Informationen. Sollten Sie sie irrtümlich erhalten haben, dürfen Sie diese E-Mail oder ihre Anhänge nicht lesen, verwenden, kopieren oder weitergeben. Sollten Sie die Mail versehentlich erhalten haben, teilen Sie mir dies bitte umgehend per Antwort-E-Mail mit und löschen Sie diese E-Mail dann aus Ihrem System. Vielen Dank Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
If the use of MTD is restricted to passive serial, this is OK with me.
Yeah, but that is not how upstream things work. You need to also think
of any other use cases.
These are the things I want to achieve.
* transfer data using the SPI driver and not use board files.
For that, the FPGA should be under the SPI in the device tree.
So to me it sounds like you actually want a "altera passive spi"
driver which uses the spi subsytem (and not the MTD layer).
* The size of the FPGA bitstream should be specified in the device
tree.
I'm not experienced with the fpga bindings, but maybe they have
something like that. Or you can deduce it from the compatible string.
What's your usecase here? It sounds like a u-boot limitation to me (if
you don't use files).
* The FPGA should be accessed as a device, not an address.
* I want a list of the FPGAs connected in the system
* Minimize code.
* Simplify user interface.
agreed.
From the CPU point of view, the FPGA is just a RAM location on the SPI
bus. It cannot be read, but it can be written.
This is not surprising, because it is simply a RAM.
It happens to have side effects, but that is not important.
Can't follow you here. How can the FPGA be a location in RAM if it's
behind an SPI bus? Thats not how SPI works.
The MTD subsystem supplies everything I need, and adding the driver
there is much less work than anything else.
Yeah but just because it has everything you need, doesn't mean
it is a good fit for your use case.
Again. I think you want to have a passive serial driver which
represents your fpga as a device which then can be configured.
There is already an empty (sigh) FPGA_UCLASS. That should probably
be extended. Ideally, this should be probed by the linux fpga
bindings.
The additional features of MTD simplifies the user interaction by
exposing a device and
providing info on the device.
Which would that be? What are the features of MTD which you need
here? You've mentioned MTD partitions, but I'm not sure these really
apply to partial reconfigurations. I know that Linux has a different
device tree binding for it. So making u-boot use mtd partitions (if
thats possible at all) and not the linux bindings means they'll
diverge, which is something we want to avoid.
You cannot do partial reconfiguration on passive serial,
so it is only of interest if someone feels a need to expand
the MTD to other configuration methods.
So.. what MTD features are then used by your approach?
Here is how an FPGA region looks in Linux.
If I understand things correctly, you have one memory region per
partial configuration. This looks quite a lot like partitions.
_region0 {
#address-cells = <1>;
#size-cells = <1>;
firmware-name = "soc_system.rbf";
fpga-bridges = <_bridge1>;
ranges = <0x2 0xff20 0x10>,
<0x0 0xc000 0x2000>;
gpio@10040 {
compatible = "altr,pio-1.0";
reg = <0x10040 0x20>;
altr,ngpio = <4>;
#gpio-cells = <2>;
clocks = <2>;
gpio-controller;
};
onchip-memory {
device_type = "memory";
compatible = "altr,onchipmem-15.1";
reg = <0x0 0x1>;
};
};
The only difference the MTD subsystem would see is
that there is a new subdirectory "fpga" with drivers
and the Kconfig + Makefile changes to support the directory.
Otherwise it plugs right in.
From a user point of view this is really confusing. Why would
you sometimes configure an fpga with the mtd command and sometimes
with the fpga command depening whether it is using this passive
programming thingy.
I think that most board designers would select one method of
configuring
the FPGA and they would only have either the FPGA or the MTD command
set. I would be surprised if an engineer would be confused by this.
As a user I'd expect one command for any similar devices. As a board
integrator, the fpga would be loaded by the board file anyway if needed
during u-boot/bootup.
Just my two cents as a spi-nor reviewer in linux. You'd have to
convince the u-boot maintainers. There is already an FPGA subsystem
in linux so I doubt your approach would make it into linux.
-michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-21 kl. 12:19, skrev Michael Walle: Am 2023-02-21 11:42, schrieb Ulf Samuelsson: Den 2023-02-21 kl. 10:08, skrev Michael Walle: If it is right or wrong to use that as an MTD is a matter of opinion. I am still hoping the MTD maintainer would provide input here. I might be missing something, but what is the reasoning here, to add this to the mtd subsystem? One is saving space, but I agree with Marek, this isn't a valid argument to just put any (unrelated) stuff into the MTD subsystem. Also, as Marek pointed out, there are many different 'programming' solutions for CPLDs/FPGAs and most of them don't share anything with MTD. You seem to be just focusing on the "passive serial" one. Yes, the passive serial is very different from the other ways of configuring an FPGA. It is write only. You cannot read back the configuration and no partial reconfiguration. You do not have to go through AXI/PCIe busses. You access through an SPI device, but todays solution does not support using the SPI driver. I can't follow you here. You have a chip that is connected to an SPI bus using SCK, MOSI, nCS. All the accesses are handled by the SPI controller driver outside the chip. Now, I saw you mentioned that | the current passive serial solutions does not use the existing SPI | drivers. What do you mean with SPI drivers? SPI flash drivers or SPI controller drivers? Does the "passive serial solution" expose an SPI bus and you can access the SPI flash on it in a generic way? Then the solution should be to write a SPI (controller) driver, the flash should then be automatically be detected. The proposed solution is similar to an SPI flash driver. The FPGA bitstream is typically stored in the CPU boot flash. The CPU reads the bitstream and does an SPI transfer to the FPGA. The FPGA is not connected to any flash memory. Ah I missed that. So you don't want to program the SPI flash behind the FPGA, but the FPGA directly. You are loading a bitstream from somewhere and configure the FPGA. Using a SPI like interface, i.e. it is write only as it lacks the DO. Exactly. The FPGA chip exposes (in SPI terms) SCK, nCS and MOSI, but not MISO so you cannot read back anything. it should read "This FPGA chip". I expect there are much more variants how to configure an FPGA. Would these also use your proposed solution? There are the following methods I am aware of. Active Serial - the FPGA is connected to an SPI flash, and loads this flash at reset. The CPU may or may not have the capability to program the flash, and then it is a normal MTD SPI Flash. The FPGA manager is not involved at all. Passive Serial: The CPU SPI controller (or similar) is connected to pins on the FPGA and the only operation allowed is writing a fixed size to the FPGA. The user needs to know the size of the FPGA. The support for this in u-boot is very old-fashioned and new users get very little help. The user needs to write an SPI access routine as the SPI driver structure is incompatible with the FPGA manager code for passive serial. Passive Parallel: The CPU loads the FPGA through the parallell bus. I doubt this is rarely used. SOC:The Intel/Altera and AMD/Xilinx have internal FPGA configuration interfaces. It is parallell. They are supported in the FPGA manager. If the use of MTD is restricted to passive serial, this is OK with me. The driver will return an empty block on read. The SPI transfer to configure the FPGA is doing * toggle a GPIO signal (nCONFIG) * Do an SPI write (using the SPI controller driver) * After the SPI transfer complete, you check the status of some GPIO (nSTATUS etc.). This is all hidden from the MTD. What the MTD subsystem sees is a "write-only memory" that has to be written with exactly 'n' bytes. MTD is mainly around erasable blocks, although there are some exotic things like mtdram. I fail to see how this would apply to your FPGA. These are the things I want to achieve. * transfer data using the SPI driver and not use board files. For that, the FPGA should be under the SPI in the device tree. * The size of the FPGA bitstream should be specified in the device tree. * The FPGA should be accessed as a device, not an address. * I want a list of the FPGAs connected in the system * Minimize code. * Simplify user interface. From the CPU point of view, the FPGA is just a RAM location on the SPI bus. It cannot be read, but it can be written. This is not surprising, because it is simply a RAM. It happens to have side effects, but that is not important. The MTD subsystem supplies everything I need, and adding the driver there is much less work than anything else. The additional features of MTD simplifies the user interaction by exposing a device and providing info on the device. Which would that be? What are the features of MTD which you need here? You've
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-21 kl. 10:08, skrev Michael Walle: If it is right or wrong to use that as an MTD is a matter of opinion. I am still hoping the MTD maintainer would provide input here. I might be missing something, but what is the reasoning here, to add this to the mtd subsystem? One is saving space, but I agree with Marek, this isn't a valid argument to just put any (unrelated) stuff into the MTD subsystem. Also, as Marek pointed out, there are many different 'programming' solutions for CPLDs/FPGAs and most of them don't share anything with MTD. You seem to be just focusing on the "passive serial" one. Yes, the passive serial is very different from the other ways of configuring an FPGA. It is write only. You cannot read back the configuration and no partial reconfiguration. You do not have to go through AXI/PCIe busses. You access through an SPI device, but todays solution does not support using the SPI driver. Now, I saw you mentioned that | the current passive serial solutions does not use the existing SPI | drivers. What do you mean with SPI drivers? SPI flash drivers or SPI controller drivers? Does the "passive serial solution" expose an SPI bus and you can access the SPI flash on it in a generic way? Then the solution should be to write a SPI (controller) driver, the flash should then be automatically be detected. The proposed solution is similar to an SPI flash driver. The FPGA bitstream is typically stored in the CPU boot flash. The CPU reads the bitstream and does an SPI transfer to the FPGA. The FPGA is not connected to any flash memory. The FPGA chip exposes (in SPI terms) SCK, nCS and MOSI, but not MISO so you cannot read back anything. The driver will return an empty block on read. The SPI transfer to configure the FPGA is doing * toggle a GPIO signal (nCONFIG) * Do an SPI write (using the SPI controller driver) * After the SPI transfer complete, you check the status of some GPIO (nSTATUS etc.). This is all hidden from the MTD. What the MTD subsystem sees is a "write-only memory" that has to be written with exactly 'n' bytes. The additional features of MTD simplifies the user interaction by exposing a device and providing info on the device. The only difference the MTD subsystem would see is that there is a new subdirectory "fpga" with drivers and the Kconfig + Makefile changes to support the directory. Otherwise it plugs right in. The current solutions for passive serial cannot use the SPI controller driver so each board implements SPI controller inside their 'board' files. You cannot reuse this code in practice, so every one that wants to have passive serial has to write their own SPI access routines. The FPGA manager does not support device tree, and I will not be able to spend time on introducing this, as Marek advices. Best Regards Ulf Samuelsson -michael -- Best Regards, Ulf Samuelsson eMagii +46 722 427437 u...@emagii.com
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-21 kl. 02:13, skrev Marek Vasut: On 2/21/23 00:47, Ulf Samuelsson wrote: Den 2023-02-20 kl. 23:34, skrev Marek Vasut: On 2/20/23 22:29, Ulf Samuelsson wrote: [...] To sum it up: - The only part of the MTD subsystem that is in use by this driver is the write callback, everything else is unused. That does not make MTD subsystem the right tool. No, the MTD subsystem is compatible with the device tree. That does not justify overloading the subsystem with unrelated hardware support for which there is an existing subsystem with existing drivers for the functionality you would duplicate in the MTD subsystem. There are existing DT bindings for FPGAs, see Linux Documentation/devicetree/bindings/fpga/ , use those and add support for them into the FPGA subsystem if needed . The current u-boot FPGA support is incompatible with the linux devicetree for FPGAs. There is simply nothing there. That is the reason for the proposal in the first place. Add it, that seems like the best approach here. As I said, this is not an option for me. You can also get information about the FPGA when you list the mtd devices. No, you cannot, because the information you may get out of the FPGA in some cases does not map to MTD devices . Please give some examples. Which FPGA bus bridge(s) are enabled for example . This is something present on SoCFPGA solutions , e.g. an AXI bridge into the fabric . Is that not something which you handle by placing your FPGA configuration block under the bus bridge in the device tree? In the proposed driver, the FPGA is under the SPI bus and the FPGA driver calls the access functions of the bus. It allows you to support partial reconfiguration through the partition feature. You can write any size block to any part of the MTD, and you can express that in a device tree. That is what is needed. I am afraid it is not as simple as that, see Linux Documentation/devicetree/bindings/fpga/fpga-region.txt . Furthermore, those DT bindings are already part of Linux and Linux expects the DT to contain exactly those bindings, U-Boot tries not to diverge from what Linux does in DT. The device tree in my driver is a superset to what linux does. It tests more status pins, and provides a name for the FPGA. It would not be difficult to make it 100% compatible. Extend the Linux DT bindings if they are insufficient, but please don't try and reinvent the wheel. That is the plan. - There is existing FPGA subsystem both in U-Boot and Linux, extend it if necessary. That also covers the usecases which go past a simple full bitstream upload. And for use, that introduces a command set, which we do not need and adds 50-100kB to the image. If you were to turn everything that needs read/write callback into an MTD device, you would be able to save even more space, but that does not make it right. You could try and reduce the FPGA command size if there is code which is unused for some usecases, that can be selected via Kconfig at build time. From my point of view, Having the FPGA command set duplicates the functionality of the MTD. That is the major contributor to code space expansion. The thing you are writing to is SRAM which is memory. There are flash based FPGAs , like Altera MAXV. There are CPLDs which are not even flash based. AFAIK, they are never configured by a CPU. They are not supported by U-Boot. If it is right or wrong to use that as an MTD is a matter of opinion. I am still hoping the MTD maintainer would provide input here. There are maintainers of MTD memories but there is no maintainer of the MTD uclass in MAINTAINERS.. The last major contributors were NameYearlines Heiko Schocher 2014-2015 1600 Stefan Roese2009 811 Boris Brezillon 2017-2018600 Miquel Raynal 2018-2019564 Sergej Lapin2013 215 Kyungmin Park 2008 197 Marek Behún 2021 178 Miquel Raynal is already on copy. Obviously, someone would have to take the time to do it, but it is certainly possible. You could make the exact same argument about making the FPGA driver a MISC device, CHAR device, BLOCK device, they all have the write callback, but that does not mean it is the right fit. That is because you see the "write" as the only function. Yes, because that is the only callback this patch implements, the rest are empty. You cannot read back the configuration from a Cyclone 10. Therefore, the entire read part of whatever subsystem is unnecessary. Erase as well for most FPGAs which are SRAM based, even for flash based FPGA, per-sector erase is not available, since there is no concept of sectors or erase blocks at all. A driver for an FPGA where configuration could be read back of course would have a read routine as well. The MTD subsystem can display the
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Sorry, I didn't follow this too closely. Do you have some pointers? I just saw your latest mail. Thanks. -michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Am 2023-02-21 11:42, schrieb Ulf Samuelsson: Den 2023-02-21 kl. 10:08, skrev Michael Walle: If it is right or wrong to use that as an MTD is a matter of opinion. I am still hoping the MTD maintainer would provide input here. I might be missing something, but what is the reasoning here, to add this to the mtd subsystem? One is saving space, but I agree with Marek, this isn't a valid argument to just put any (unrelated) stuff into the MTD subsystem. Also, as Marek pointed out, there are many different 'programming' solutions for CPLDs/FPGAs and most of them don't share anything with MTD. You seem to be just focusing on the "passive serial" one. Yes, the passive serial is very different from the other ways of configuring an FPGA. It is write only. You cannot read back the configuration and no partial reconfiguration. You do not have to go through AXI/PCIe busses. You access through an SPI device, but todays solution does not support using the SPI driver. I can't follow you here. Now, I saw you mentioned that | the current passive serial solutions does not use the existing SPI | drivers. What do you mean with SPI drivers? SPI flash drivers or SPI controller drivers? Does the "passive serial solution" expose an SPI bus and you can access the SPI flash on it in a generic way? Then the solution should be to write a SPI (controller) driver, the flash should then be automatically be detected. The proposed solution is similar to an SPI flash driver. The FPGA bitstream is typically stored in the CPU boot flash. The CPU reads the bitstream and does an SPI transfer to the FPGA. The FPGA is not connected to any flash memory. Ah I missed that. So you don't want to program the SPI flash behind the FPGA, but the FPGA directly. You are loading a bitstream from somewhere and configure the FPGA. Using a SPI like interface, i.e. it is write only as it lacks the DO. The FPGA chip exposes (in SPI terms) SCK, nCS and MOSI, but not MISO so you cannot read back anything. it should read "This FPGA chip". I expect there are much more variants how to configure an FPGA. Would these also use your proposed solution? The driver will return an empty block on read. The SPI transfer to configure the FPGA is doing * toggle a GPIO signal (nCONFIG) * Do an SPI write (using the SPI controller driver) * After the SPI transfer complete, you check the status of some GPIO (nSTATUS etc.). This is all hidden from the MTD. What the MTD subsystem sees is a "write-only memory" that has to be written with exactly 'n' bytes. MTD is mainly around erasable blocks, although there are some exotic things like mtdram. I fail to see how this would apply to your FPGA. The additional features of MTD simplifies the user interaction by exposing a device and providing info on the device. Which would that be? What are the features of MTD which you need here? You've mentioned MTD partitions, but I'm not sure these really apply to partial reconfigurations. I know that Linux has a different device tree binding for it. So making u-boot use mtd partitions (if thats possible at all) and not the linux bindings means they'll diverge, which is something we want to avoid. The only difference the MTD subsystem would see is that there is a new subdirectory "fpga" with drivers and the Kconfig + Makefile changes to support the directory. Otherwise it plugs right in. From a user point of view this is really confusing. Why would you sometimes configure an fpga with the mtd command and sometimes with the fpga command depening whether it is using this passive programming thingy. The current solutions for passive serial cannot use the SPI controller driver so each board implements SPI controller inside their 'board' files. You cannot reuse this code in practice, so every one that wants to have passive serial has to write their own SPI access routines. Sorry, I didn't follow this too closely. Do you have some pointers? -michael The FPGA manager does not support device tree, and I will not be able to spend time on introducing this, as Marek advices. Best Regards Ulf Samuelsson -michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Hi Guys, We are currently discussing how to support passive serial configuration of FPGAs in u-boot. Checking the u-boot source code reveals that only your boards actually supports this. board/beckhoff/mx53cx9020/mx53cx9020.c:...Altera_CYC2_Passive_Serial ... $ scripts/get_maintainer.pl board/beckhoff/mx53cx9020/mx53cx9020.c Patrick Bruenn (maintainer:MX53 CX9020) board/astro/mcf5373l/fpga.c:Altera_CYC2_Passive_Serial ... $ scripts/get_maintainer.pl board/astro/mcf5373l/fpga.c Wolfgang Wegner (maintainer:MCF5373L BOARD) The proposed ideas is to have the FPGA as an SPI peripheral in the device tree. I have a working version, where the FPGA is an MTD device and the general idea is to add it to the FPGA manager, which unfortunately needs a major rewrite to support device tree. Your board support configuring the FPGA in the 'board' files. * Are these boards still active and will require updates of u-boot? * If so, would it be a problem porting the FPGA configuration to the device tree? Marek does not want two drivers supporting passive serial, and supporting both 'board' drivers and devicetree drivers seems to result in unclean code. Best Regards Ulf Samuelsson
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
>> If it is right or wrong to use that as an MTD is a matter of opinion. > > I am still hoping the MTD maintainer would provide input here. I might be missing something, but what is the reasoning here, to add this to the mtd subsystem? One is saving space, but I agree with Marek, this isn't a valid argument to just put any (unrelated) stuff into the MTD subsystem. Also, as Marek pointed out, there are many different 'programming' solutions for CPLDs/FPGAs and most of them don't share anything with MTD. You seem to be just focusing on the "passive serial" one. Now, I saw you mentioned that | the current passive serial solutions does not use the existing SPI | drivers. What do you mean with SPI drivers? SPI flash drivers or SPI controller drivers? Does the "passive serial solution" expose an SPI bus and you can access the SPI flash on it in a generic way? Then the solution should be to write a SPI (controller) driver, the flash should then be automatically be detected. -michael
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
On 2/21/23 00:47, Ulf Samuelsson wrote: Den 2023-02-20 kl. 23:34, skrev Marek Vasut: On 2/20/23 22:29, Ulf Samuelsson wrote: [...] To sum it up: - The only part of the MTD subsystem that is in use by this driver is the write callback, everything else is unused. That does not make MTD subsystem the right tool. No, the MTD subsystem is compatible with the device tree. That does not justify overloading the subsystem with unrelated hardware support for which there is an existing subsystem with existing drivers for the functionality you would duplicate in the MTD subsystem. There are existing DT bindings for FPGAs, see Linux Documentation/devicetree/bindings/fpga/ , use those and add support for them into the FPGA subsystem if needed . The current u-boot FPGA support is incompatible with the linux devicetree for FPGAs. There is simply nothing there. That is the reason for the proposal in the first place. Add it, that seems like the best approach here. You can also get information about the FPGA when you list the mtd devices. No, you cannot, because the information you may get out of the FPGA in some cases does not map to MTD devices . Please give some examples. Which FPGA bus bridge(s) are enabled for example . This is something present on SoCFPGA solutions , e.g. an AXI bridge into the fabric . It allows you to support partial reconfiguration through the partition feature. You can write any size block to any part of the MTD, and you can express that in a device tree. That is what is needed. I am afraid it is not as simple as that, see Linux Documentation/devicetree/bindings/fpga/fpga-region.txt . Furthermore, those DT bindings are already part of Linux and Linux expects the DT to contain exactly those bindings, U-Boot tries not to diverge from what Linux does in DT. The device tree in my driver is a superset to what linux does. It tests more status pins, and provides a name for the FPGA. It would not be difficult to make it 100% compatible. Extend the Linux DT bindings if they are insufficient, but please don't try and reinvent the wheel. - There is existing FPGA subsystem both in U-Boot and Linux, extend it if necessary. That also covers the usecases which go past a simple full bitstream upload. And for use, that introduces a command set, which we do not need and adds 50-100kB to the image. If you were to turn everything that needs read/write callback into an MTD device, you would be able to save even more space, but that does not make it right. You could try and reduce the FPGA command size if there is code which is unused for some usecases, that can be selected via Kconfig at build time. From my point of view, Having the FPGA command set duplicates the functionality of the MTD. That is the major contributor to code space expansion. The thing you are writing to is SRAM which is memory. There are flash based FPGAs , like Altera MAXV. There are CPLDs which are not even flash based. If it is right or wrong to use that as an MTD is a matter of opinion. I am still hoping the MTD maintainer would provide input here. Obviously, someone would have to take the time to do it, but it is certainly possible. You could make the exact same argument about making the FPGA driver a MISC device, CHAR device, BLOCK device, they all have the write callback, but that does not mean it is the right fit. That is because you see the "write" as the only function. Yes, because that is the only callback this patch implements, the rest are empty. You cannot read back the configuration from a Cyclone 10. Therefore, the entire read part of whatever subsystem is unnecessary. Erase as well for most FPGAs which are SRAM based, even for flash based FPGA, per-sector erase is not available, since there is no concept of sectors or erase blocks at all. A driver for an FPGA where configuration could be read back of course would have a read routine as well. The MTD subsystem can display the information the driver provides. That is an important piece that would be missing from a MISC or CHAR driver. You also have a "device" to work with. All this is already in the FPGA subsystem, and it is designed toward the FPGA use case. [...] What would work is to keep the driver as is, replacing the MTD stuff internally with an fpga_add after generating a struct based on the device tree. Then it would be part of the FPGA subsystem. To add this as an Altera passive serial is high risk, since it cannot be tested on boards using that. It would simplify things a lot to add it as another driver. No, that would make things MUCH worse, because we would have a one-off driver used by one system, therefore poorly tested, which duplicates functionality of existing driver used by multiple systems and better tested. This also increases maintenance burden. Today you have a custom functionality to access the FPGA in each board package. That only gets
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-20 kl. 23:34, skrev Marek Vasut: On 2/20/23 22:29, Ulf Samuelsson wrote: [...] To sum it up: - The only part of the MTD subsystem that is in use by this driver is the write callback, everything else is unused. That does not make MTD subsystem the right tool. No, the MTD subsystem is compatible with the device tree. That does not justify overloading the subsystem with unrelated hardware support for which there is an existing subsystem with existing drivers for the functionality you would duplicate in the MTD subsystem. There are existing DT bindings for FPGAs, see Linux Documentation/devicetree/bindings/fpga/ , use those and add support for them into the FPGA subsystem if needed . The current u-boot FPGA support is incompatible with the linux devicetree for FPGAs. There is simply nothing there. That is the reason for the proposal in the first place. You can also get information about the FPGA when you list the mtd devices. No, you cannot, because the information you may get out of the FPGA in some cases does not map to MTD devices . Please give some examples. It allows you to support partial reconfiguration through the partition feature. You can write any size block to any part of the MTD, and you can express that in a device tree. That is what is needed. I am afraid it is not as simple as that, see Linux Documentation/devicetree/bindings/fpga/fpga-region.txt . Furthermore, those DT bindings are already part of Linux and Linux expects the DT to contain exactly those bindings, U-Boot tries not to diverge from what Linux does in DT. The device tree in my driver is a superset to what linux does. It tests more status pins, and provides a name for the FPGA. It would not be difficult to make it 100% compatible. - There is existing FPGA subsystem both in U-Boot and Linux, extend it if necessary. That also covers the usecases which go past a simple full bitstream upload. And for use, that introduces a command set, which we do not need and adds 50-100kB to the image. If you were to turn everything that needs read/write callback into an MTD device, you would be able to save even more space, but that does not make it right. You could try and reduce the FPGA command size if there is code which is unused for some usecases, that can be selected via Kconfig at build time. From my point of view, Having the FPGA command set duplicates the functionality of the MTD. That is the major contributor to code space expansion. The thing you are writing to is SRAM which is memory. If it is right or wrong to use that as an MTD is a matter of opinion. Obviously, someone would have to take the time to do it, but it is certainly possible. You could make the exact same argument about making the FPGA driver a MISC device, CHAR device, BLOCK device, they all have the write callback, but that does not mean it is the right fit. That is because you see the "write" as the only function. Yes, because that is the only callback this patch implements, the rest are empty. You cannot read back the configuration from a Cyclone 10. A driver for an FPGA where configuration could be read back of course would have a read routine as well. The MTD subsystem can display the information the driver provides. That is an important piece that would be missing from a MISC or CHAR driver. You also have a "device" to work with. [...] What would work is to keep the driver as is, replacing the MTD stuff internally with an fpga_add after generating a struct based on the device tree. Then it would be part of the FPGA subsystem. To add this as an Altera passive serial is high risk, since it cannot be tested on boards using that. It would simplify things a lot to add it as another driver. No, that would make things MUCH worse, because we would have a one-off driver used by one system, therefore poorly tested, which duplicates functionality of existing driver used by multiple systems and better tested. This also increases maintenance burden. Today you have a custom functionality to access the FPGA in each board package. That only gets tested when building that board. So if you have 'n' boards, you have 'n' different ways of accessing the FPGA. Each poorly tested, if number of users are the measurement. The only thing in common is the multiplexing code. Anyone that designs a new system, would have to choose between using the new driver, or write their own access method. They cannot use what is in u-boot today, because it is in the board directory. Even if they have the same chip as a board with existing support, they cannot add it in, since it is in the board directory. Only solution is to duplicate the code in their own board directory, but the likelyhood of using the same CPU is slim. Are you happy with this? I wager that anyone wanting to use passive serial would adopt the new driver. One solution is of course to ask the maintainers of those
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
On 2/20/23 22:29, Ulf Samuelsson wrote: [...] To sum it up: - The only part of the MTD subsystem that is in use by this driver is the write callback, everything else is unused. That does not make MTD subsystem the right tool. No, the MTD subsystem is compatible with the device tree. That does not justify overloading the subsystem with unrelated hardware support for which there is an existing subsystem with existing drivers for the functionality you would duplicate in the MTD subsystem. There are existing DT bindings for FPGAs, see Linux Documentation/devicetree/bindings/fpga/ , use those and add support for them into the FPGA subsystem if needed . You can also get information about the FPGA when you list the mtd devices. No, you cannot, because the information you may get out of the FPGA in some cases does not map to MTD devices . It allows you to support partial reconfiguration through the partition feature. You can write any size block to any part of the MTD, and you can express that in a device tree. That is what is needed. I am afraid it is not as simple as that, see Linux Documentation/devicetree/bindings/fpga/fpga-region.txt . Furthermore, those DT bindings are already part of Linux and Linux expects the DT to contain exactly those bindings, U-Boot tries not to diverge from what Linux does in DT. - There is existing FPGA subsystem both in U-Boot and Linux, extend it if necessary. That also covers the usecases which go past a simple full bitstream upload. And for use, that introduces a command set, which we do not need and adds 50-100kB to the image. If you were to turn everything that needs read/write callback into an MTD device, you would be able to save even more space, but that does not make it right. You could try and reduce the FPGA command size if there is code which is unused for some usecases, that can be selected via Kconfig at build time. Obviously, someone would have to take the time to do it, but it is certainly possible. You could make the exact same argument about making the FPGA driver a MISC device, CHAR device, BLOCK device, they all have the write callback, but that does not mean it is the right fit. That is because you see the "write" as the only function. Yes, because that is the only callback this patch implements, the rest are empty. [...] What would work is to keep the driver as is, replacing the MTD stuff internally with an fpga_add after generating a struct based on the device tree. Then it would be part of the FPGA subsystem. To add this as an Altera passive serial is high risk, since it cannot be tested on boards using that. It would simplify things a lot to add it as another driver. No, that would make things MUCH worse, because we would have a one-off driver used by one system, therefore poorly tested, which duplicates functionality of existing driver used by multiple systems and better tested. This also increases maintenance burden. One single driver for this common functionality is always better as it gets more testing. The result would of course be something which adds both complexity and code size for us, without any benefit. Please also consider the MTD maintainer point of view. [...]
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-20 kl. 17:17, skrev Marek Vasut:
On 2/13/23 10:30, Ulf Samuelsson wrote:
Den 2023-02-12 kl. 23:40, skrev Marek Vasut:
On 2/12/23 23:07, Ulf Samuelsson wrote:
Den 2023-02-12 kl. 21:01, skrev Marek Vasut:
> On 2/12/23 20:52, Ulf Samuelsson wrote:
>>
>>
>> Den 2023-02-12 kl. 20:31, skrev Marek Vasut:
>> > On 2/11/23 11:07, u-b...@emagii.com wrote:
>> >
>> > [...]
>> >
>> >> +static int cyc10_write(struct mtd_info *mtd, loff_t to,
size_t len,
>> >> + size_t *retlen, const u_char *buf)
>> >> +{
>> >> + struct udevice *dev = mtd->dev;
>> >> + struct spi_slave *slave = dev_get_parent_priv(dev);
>> >> + struct cyc10_plat *fpga = dev_get_plat(dev);
>> >> + int ret;
>> >
>> > Do I read this right, that the 'write' callback is the only
one doing
>> > meaningful work, all the other callbacks are just empty stubs ?
>> > Yes, you cannot read back the configuration data.
>
> That makes it look like any framework which supports "write"
callback would be suitable, not just MTD framework.
>
>> > Why not update drivers/fpga/cyclon2.c which is Passive Serial
>> > implementation already present in U-Boot for Altera Cyclone
II FPGA ,
>> > with Cyclone 10 FPGA support ? I believe the PS protocol
changed very
>> > little.
>> Since the MTD command set is enough to configure the FPGA, the
FPGA commands can be removed from the build. The FPGA command set
requires you to supply addresses, but the MTD command set uses devices.
>>
>> So:
>> * Smaller U-Boot image
>
> The MTD framework is rather large, compared to the trivial FPGA
framework. Can you back this claim with any numbers ?
I am assuming that the MTD framework is needed anyway.
FPGA and MTD support is orthogonal, you cannot make that assumption.
Consider SoC-FPGA machine booting from eMMC, Altera Cyclone V SoC
does support that mode of operation, and MTD support can be disabled.
If I look at Altera SOCFPGAs, I find that 21 defconfigs have MTD
support and 3 do not.
* socfpga_agilex_defconfig
* socfpga_chameleonv3_defconfig
* socfpga_n5x_defconfig
so in most cases there would be a reduction in code size.
All these machines already have FPGA framework enabled, just use it or
extend it.
The argument was that some boards have FPGA framework without MTD
and that gives overhead for the those board.
Since 21 out of 24 have MTD, the FPGA framework gives overhead
for those boards, which could removed if FPGAs were
installed as MTDs.
We certainly need it.
>
>> * Simplified user interface.
>
> If I am to select between 'fpga load' and 'mtd write' for FPGA
bitstream loading , my obvious choice would be 'fpga load' . How is
using 'mtd write' any better or simpler ?
>
fpga load 0 ${loadaddr} ${filesize}
mtd write spy ${loadaddr}
The questions I ask myself.
So is "0" the "spy" FPGA or the "spx" FPGA?
You can use DT /aliases node to enumerate the FPGAs the same way i2c
busses, SPI NORs, SD/MMC devices etc. are enumerated . Also have a
look at e.g. 'net list' command, similar functionality can be added
to the FPGA command to list all registered FPGAs.
Why should I have to remember the size of the FPGA bitstream?
Because it is not always possible to extract that information from
the bitstream blob, remember, some of those blobs may be just raw
binaries of the SRAM/flash content .
The size of the FPGA will be in the devicetree, so there is no need
to supply that in the command itself.
That stops working once you start dealing with partial reconfiguration
and the bitstream size varies. Then it is back to $filesize .
The MTD subsystem supports the concept of partitions.
I suspect that this would allow partial configuration to work quite OK.
>> * The FPGA is an SPI peripheral, so why not add it to the SPI
part of the device tree?
>
> You can add the device into DT and still operate it using the
U-Boot FPGA framework, just add the DT support. Why not do it that
way ?
I don't think you can add the device into DT in U-Boot as it is today.
You can create FPGA contents and add that to the device tree, but not
the configuration itself. At least, I have not seen it.
If I have missed it, where is an example?
Have a look at the Linux FPGA DT bindings in
Documentation/devicetree/bindings/fpga/ . You can implement parsing
of those bindings into the U-Boot FPGA framework and then add your
FPGA device configuration interface into the DT.
The "altr,fpga-passive-serial" driver is very close to what I did.
If there is an existing driver, extend it, rework it, but please don't
introduce new driver that does the same thing.
The current passive serial solutions does not use the existing SPI
drivers. They access the SPI peripheral outside the driver.
A new driver would build on the existing SPI drivers.
It is not possible to test if a modified driver break the function on
those boards, if you do not have access to the
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
On 2/13/23 10:30, Ulf Samuelsson wrote:
Den 2023-02-12 kl. 23:40, skrev Marek Vasut:
On 2/12/23 23:07, Ulf Samuelsson wrote:
Den 2023-02-12 kl. 21:01, skrev Marek Vasut:
> On 2/12/23 20:52, Ulf Samuelsson wrote:
>>
>>
>> Den 2023-02-12 kl. 20:31, skrev Marek Vasut:
>> > On 2/11/23 11:07, u-b...@emagii.com wrote:
>> >
>> > [...]
>> >
>> >> +static int cyc10_write(struct mtd_info *mtd, loff_t to,
size_t len,
>> >> + size_t *retlen, const u_char *buf)
>> >> +{
>> >> + struct udevice *dev = mtd->dev;
>> >> + struct spi_slave *slave = dev_get_parent_priv(dev);
>> >> + struct cyc10_plat *fpga = dev_get_plat(dev);
>> >> + int ret;
>> >
>> > Do I read this right, that the 'write' callback is the only
one doing
>> > meaningful work, all the other callbacks are just empty stubs ?
>> > Yes, you cannot read back the configuration data.
>
> That makes it look like any framework which supports "write"
callback would be suitable, not just MTD framework.
>
>> > Why not update drivers/fpga/cyclon2.c which is Passive Serial
>> > implementation already present in U-Boot for Altera Cyclone II
FPGA ,
>> > with Cyclone 10 FPGA support ? I believe the PS protocol
changed very
>> > little.
>> Since the MTD command set is enough to configure the FPGA, the
FPGA commands can be removed from the build. The FPGA command set
requires you to supply addresses, but the MTD command set uses devices.
>>
>> So:
>> * Smaller U-Boot image
>
> The MTD framework is rather large, compared to the trivial FPGA
framework. Can you back this claim with any numbers ?
I am assuming that the MTD framework is needed anyway.
FPGA and MTD support is orthogonal, you cannot make that assumption.
Consider SoC-FPGA machine booting from eMMC, Altera Cyclone V SoC does
support that mode of operation, and MTD support can be disabled.
If I look at Altera SOCFPGAs, I find that 21 defconfigs have MTD support
and 3 do not.
* socfpga_agilex_defconfig
* socfpga_chameleonv3_defconfig
* socfpga_n5x_defconfig
so in most cases there would be a reduction in code size.
All these machines already have FPGA framework enabled, just use it or
extend it.
We certainly need it.
>
>> * Simplified user interface.
>
> If I am to select between 'fpga load' and 'mtd write' for FPGA
bitstream loading , my obvious choice would be 'fpga load' . How is
using 'mtd write' any better or simpler ?
>
fpga load 0 ${loadaddr} ${filesize}
mtd write spy ${loadaddr}
The questions I ask myself.
So is "0" the "spy" FPGA or the "spx" FPGA?
You can use DT /aliases node to enumerate the FPGAs the same way i2c
busses, SPI NORs, SD/MMC devices etc. are enumerated . Also have a
look at e.g. 'net list' command, similar functionality can be added to
the FPGA command to list all registered FPGAs.
Why should I have to remember the size of the FPGA bitstream?
Because it is not always possible to extract that information from the
bitstream blob, remember, some of those blobs may be just raw binaries
of the SRAM/flash content .
The size of the FPGA will be in the devicetree, so there is no need
to supply that in the command itself.
That stops working once you start dealing with partial reconfiguration
and the bitstream size varies. Then it is back to $filesize .
>> * The FPGA is an SPI peripheral, so why not add it to the SPI
part of the device tree?
>
> You can add the device into DT and still operate it using the
U-Boot FPGA framework, just add the DT support. Why not do it that way ?
I don't think you can add the device into DT in U-Boot as it is today.
You can create FPGA contents and add that to the device tree, but not
the configuration itself. At least, I have not seen it.
If I have missed it, where is an example?
Have a look at the Linux FPGA DT bindings in
Documentation/devicetree/bindings/fpga/ . You can implement parsing of
those bindings into the U-Boot FPGA framework and then add your FPGA
device configuration interface into the DT.
The "altr,fpga-passive-serial" driver is very close to what I did.
If there is an existing driver, extend it, rework it, but please don't
introduce new driver that does the same thing.
My driver supports a superset of a devicetree for that driver.
It does not look like U-Boot is setup to use that.
I feel that the FPGA manager needs a total rework to make this work
and I cannot justify that. If someone adds devicetree support to the
FPGA manager, I will certainly look to adopt to it.
Overloading the MTD subsystem only because it has a .write() callback is
not the way to go either.
The FPGA framework is not really setup to use device-tree to describe
configuration.
Only 5 defconfigs in U-Boot uses the FPGA framework.
* astro_mcf5373l_defconfig
* syzygy_hub_defconfig
* xilinx_zynqmp_virt_defconfig
* xilinx_zynq_virt_defconfig
These two ^ cover very much every zynq 7000 and zynqmp device in
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-12 kl. 23:40, skrev Marek Vasut:
On 2/12/23 23:07, Ulf Samuelsson wrote:
Den 2023-02-12 kl. 21:01, skrev Marek Vasut:
> On 2/12/23 20:52, Ulf Samuelsson wrote:
>>
>>
>> Den 2023-02-12 kl. 20:31, skrev Marek Vasut:
>> > On 2/11/23 11:07, u-b...@emagii.com wrote:
>> >
>> > [...]
>> >
>> >> +static int cyc10_write(struct mtd_info *mtd, loff_t to,
size_t len,
>> >> + size_t *retlen, const u_char *buf)
>> >> +{
>> >> + struct udevice *dev = mtd->dev;
>> >> + struct spi_slave *slave = dev_get_parent_priv(dev);
>> >> + struct cyc10_plat *fpga = dev_get_plat(dev);
>> >> + int ret;
>> >
>> > Do I read this right, that the 'write' callback is the only one
doing
>> > meaningful work, all the other callbacks are just empty stubs ?
>> > Yes, you cannot read back the configuration data.
>
> That makes it look like any framework which supports "write"
callback would be suitable, not just MTD framework.
>
>> > Why not update drivers/fpga/cyclon2.c which is Passive Serial
>> > implementation already present in U-Boot for Altera Cyclone II
FPGA ,
>> > with Cyclone 10 FPGA support ? I believe the PS protocol
changed very
>> > little.
>> Since the MTD command set is enough to configure the FPGA, the
FPGA commands can be removed from the build. The FPGA command set
requires you to supply addresses, but the MTD command set uses devices.
>>
>> So:
>> * Smaller U-Boot image
>
> The MTD framework is rather large, compared to the trivial FPGA
framework. Can you back this claim with any numbers ?
I am assuming that the MTD framework is needed anyway.
FPGA and MTD support is orthogonal, you cannot make that assumption.
Consider SoC-FPGA machine booting from eMMC, Altera Cyclone V SoC does
support that mode of operation, and MTD support can be disabled.
If I look at Altera SOCFPGAs, I find that 21 defconfigs have MTD support
and 3 do not.
* socfpga_agilex_defconfig
* socfpga_chameleonv3_defconfig
* socfpga_n5x_defconfig
so in most cases there would be a reduction in code size.
We certainly need it.
>
>> * Simplified user interface.
>
> If I am to select between 'fpga load' and 'mtd write' for FPGA
bitstream loading , my obvious choice would be 'fpga load' . How is
using 'mtd write' any better or simpler ?
>
fpga load 0 ${loadaddr} ${filesize}
mtd write spy ${loadaddr}
The questions I ask myself.
So is "0" the "spy" FPGA or the "spx" FPGA?
You can use DT /aliases node to enumerate the FPGAs the same way i2c
busses, SPI NORs, SD/MMC devices etc. are enumerated . Also have a look
at e.g. 'net list' command, similar functionality can be added to the
FPGA command to list all registered FPGAs.
Why should I have to remember the size of the FPGA bitstream?
Because it is not always possible to extract that information from the
bitstream blob, remember, some of those blobs may be just raw binaries
of the SRAM/flash content .
The size of the FPGA will be in the devicetree, so there is no need
to supply that in the command itself.
>> * The FPGA is an SPI peripheral, so why not add it to the SPI part
of the device tree?
>
> You can add the device into DT and still operate it using the
U-Boot FPGA framework, just add the DT support. Why not do it that way ?
I don't think you can add the device into DT in U-Boot as it is today.
You can create FPGA contents and add that to the device tree, but not
the configuration itself. At least, I have not seen it.
If I have missed it, where is an example?
Have a look at the Linux FPGA DT bindings in
Documentation/devicetree/bindings/fpga/ . You can implement parsing of
those bindings into the U-Boot FPGA framework and then add your FPGA
device configuration interface into the DT.
The "altr,fpga-passive-serial" driver is very close to what I did.
My driver supports a superset of a devicetree for that driver.
It does not look like U-Boot is setup to use that.
I feel that the FPGA manager needs a total rework to make this work
and I cannot justify that. If someone adds devicetree support to the
FPGA manager, I will certainly look to adopt to it.
The FPGA framework is not really setup to use device-tree to describe
configuration.
Only 5 defconfigs in U-Boot uses the FPGA framework.
* astro_mcf5373l_defconfig
* syzygy_hub_defconfig
* xilinx_zynqmp_virt_defconfig
* xilinx_zynq_virt_defconfig
These two ^ cover very much every zynq 7000 and zynqmp device in
existence, since the way those are used is in combination with provided
custom board DT.
* bitmain_antminer_s9_defconfig
There is also arch/arm/mach-socfpga/Kconfig: imply FPGA_SOCFPGA , which
activates the CONFIG_FPGA on all of Altera Cyclone V/Arria V/Stratix
10/Agilex and whatever new SoCFPGA Intel has.
Their device trees have leafs for configuration:
* compatible = "fpga-region";
* compatible = "xlnx,zynq-devcfg-1.0";
Neither "fpga-region" or "xlnx,zynq-devcfg-1.0" have any
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
On 2/12/23 23:07, Ulf Samuelsson wrote:
Den 2023-02-12 kl. 21:01, skrev Marek Vasut:
> On 2/12/23 20:52, Ulf Samuelsson wrote:
>>
>>
>> Den 2023-02-12 kl. 20:31, skrev Marek Vasut:
>> > On 2/11/23 11:07, u-b...@emagii.com wrote:
>> >
>> > [...]
>> >
>> >> +static int cyc10_write(struct mtd_info *mtd, loff_t to, size_t
len,
>> >> + size_t *retlen, const u_char *buf)
>> >> +{
>> >> + struct udevice *dev = mtd->dev;
>> >> + struct spi_slave *slave = dev_get_parent_priv(dev);
>> >> + struct cyc10_plat *fpga = dev_get_plat(dev);
>> >> + int ret;
>> >
>> > Do I read this right, that the 'write' callback is the only one
doing
>> > meaningful work, all the other callbacks are just empty stubs ?
>> > Yes, you cannot read back the configuration data.
>
> That makes it look like any framework which supports "write" callback
would be suitable, not just MTD framework.
>
>> > Why not update drivers/fpga/cyclon2.c which is Passive Serial
>> > implementation already present in U-Boot for Altera Cyclone II
FPGA ,
>> > with Cyclone 10 FPGA support ? I believe the PS protocol changed
very
>> > little.
>> Since the MTD command set is enough to configure the FPGA, the FPGA
commands can be removed from the build. The FPGA command set requires
you to supply addresses, but the MTD command set uses devices.
>>
>> So:
>> * Smaller U-Boot image
>
> The MTD framework is rather large, compared to the trivial FPGA
framework. Can you back this claim with any numbers ?
I am assuming that the MTD framework is needed anyway.
FPGA and MTD support is orthogonal, you cannot make that assumption.
Consider SoC-FPGA machine booting from eMMC, Altera Cyclone V SoC does
support that mode of operation, and MTD support can be disabled.
We certainly need it.
>
>> * Simplified user interface.
>
> If I am to select between 'fpga load' and 'mtd write' for FPGA
bitstream loading , my obvious choice would be 'fpga load' . How is
using 'mtd write' any better or simpler ?
>
fpga load 0 ${loadaddr} ${filesize}
mtd write spy ${loadaddr}
The questions I ask myself.
So is "0" the "spy" FPGA or the "spx" FPGA?
You can use DT /aliases node to enumerate the FPGAs the same way i2c
busses, SPI NORs, SD/MMC devices etc. are enumerated . Also have a look
at e.g. 'net list' command, similar functionality can be added to the
FPGA command to list all registered FPGAs.
Why should I have to remember the size of the FPGA bitstream?
Because it is not always possible to extract that information from the
bitstream blob, remember, some of those blobs may be just raw binaries
of the SRAM/flash content .
>> * The FPGA is an SPI peripheral, so why not add it to the SPI part
of the device tree?
>
> You can add the device into DT and still operate it using the U-Boot
FPGA framework, just add the DT support. Why not do it that way ?
I don't think you can add the device into DT in U-Boot as it is today.
You can create FPGA contents and add that to the device tree, but not
the configuration itself. At least, I have not seen it.
If I have missed it, where is an example?
Have a look at the Linux FPGA DT bindings in
Documentation/devicetree/bindings/fpga/ . You can implement parsing of
those bindings into the U-Boot FPGA framework and then add your FPGA
device configuration interface into the DT.
The FPGA framework is not really setup to use device-tree to describe
configuration.
Only 5 defconfigs in U-Boot uses the FPGA framework.
* astro_mcf5373l_defconfig
* syzygy_hub_defconfig
* xilinx_zynqmp_virt_defconfig
* xilinx_zynq_virt_defconfig
These two ^ cover very much every zynq 7000 and zynqmp device in
existence, since the way those are used is in combination with provided
custom board DT.
* bitmain_antminer_s9_defconfig
There is also arch/arm/mach-socfpga/Kconfig: imply FPGA_SOCFPGA , which
activates the CONFIG_FPGA on all of Altera Cyclone V/Arria V/Stratix
10/Agilex and whatever new SoCFPGA Intel has.
Their device trees have leafs for configuration:
* compatible = "fpga-region";
* compatible = "xlnx,zynq-devcfg-1.0";
Neither "fpga-region" or "xlnx,zynq-devcfg-1.0" have any compatible
drivers, so there is really no support for configuration based on
device-tree at the moment.
That's correct
If I look at boards using the Altera, no board uses a driver
to configure the FPGA. Instead they implement a number of callbacks
in the board files which manually handles the SPI bus outside the SPI
driver. No trace of device tree.
Which boards do you refer to ?
> Let me be blunt about this, I have this feeling that what is
happening here is just overloading of MTD framework with unrelated
functionality (FPGA bitstream loading). MTD framework simply is not the
right place, esp. if there is dedicated FPGA framework, with existing
Altera PS driver no less.
When you are configuring an FPGA, you are writing a bitstream
to an SRAM
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-12 kl. 21:01, skrev Marek Vasut:
> On 2/12/23 20:52, Ulf Samuelsson wrote:
>>
>>
>> Den 2023-02-12 kl. 20:31, skrev Marek Vasut:
>> > On 2/11/23 11:07, u-b...@emagii.com wrote:
>> >
>> > [...]
>> >
>> >> +static int cyc10_write(struct mtd_info *mtd, loff_t to, size_t len,
>> >> + size_t *retlen, const u_char *buf)
>> >> +{
>> >> +struct udevice *dev = mtd->dev;
>> >> +struct spi_slave *slave = dev_get_parent_priv(dev);
>> >> +struct cyc10_plat *fpga = dev_get_plat(dev);
>> >> +int ret;
>> >
>> > Do I read this right, that the 'write' callback is the only one doing
>> > meaningful work, all the other callbacks are just empty stubs ?
>> > Yes, you cannot read back the configuration data.
>
> That makes it look like any framework which supports "write" callback
would be suitable, not just MTD framework.
>
>> > Why not update drivers/fpga/cyclon2.c which is Passive Serial
>> > implementation already present in U-Boot for Altera Cyclone II FPGA ,
>> > with Cyclone 10 FPGA support ? I believe the PS protocol changed very
>> > little.
>> Since the MTD command set is enough to configure the FPGA, the FPGA
commands can be removed from the build. The FPGA command set requires
you to supply addresses, but the MTD command set uses devices.
>>
>> So:
>> * Smaller U-Boot image
>
> The MTD framework is rather large, compared to the trivial FPGA
framework. Can you back this claim with any numbers ?
I am assuming that the MTD framework is needed anyway.
We certainly need it.
>
>> * Simplified user interface.
>
> If I am to select between 'fpga load' and 'mtd write' for FPGA
bitstream loading , my obvious choice would be 'fpga load' . How is
using 'mtd write' any better or simpler ?
>
fpga load 0 ${loadaddr} ${filesize}
mtd write spy ${loadaddr}
The questions I ask myself.
So is "0" the "spy" FPGA or the "spx" FPGA?
Why should I have to remember the size of the FPGA bitstream?
>> * The FPGA is an SPI peripheral, so why not add it to the SPI part
of the device tree?
>
> You can add the device into DT and still operate it using the U-Boot
FPGA framework, just add the DT support. Why not do it that way ?
I don't think you can add the device into DT in U-Boot as it is today.
You can create FPGA contents and add that to the device tree, but not
the configuration itself. At least, I have not seen it.
If I have missed it, where is an example?
The FPGA framework is not really setup to use device-tree to describe
configuration.
Only 5 defconfigs in U-Boot uses the FPGA framework.
* astro_mcf5373l_defconfig
* syzygy_hub_defconfig
* xilinx_zynqmp_virt_defconfig
* xilinx_zynq_virt_defconfig
* bitmain_antminer_s9_defconfig
Their device trees have leafs for configuration:
* compatible = "fpga-region";
* compatible = "xlnx,zynq-devcfg-1.0";
Neither "fpga-region" or "xlnx,zynq-devcfg-1.0" have any compatible
drivers, so there is really no support for configuration based on
device-tree at the moment.
If I look at boards using the Altera, no board uses a driver
to configure the FPGA. Instead they implement a number of callbacks
in the board files which manually handles the SPI bus outside the SPI
driver. No trace of device tree.
>
> Let me be blunt about this, I have this feeling that what is
happening here is just overloading of MTD framework with unrelated
functionality (FPGA bitstream loading). MTD framework simply is not the
right place, esp. if there is dedicated FPGA framework, with existing
Altera PS driver no less.
When you are configuring an FPGA, you are writing a bitstream
to an SRAM inside the FPGA. SRAM are memories.
The MTD framework does exactly what is needed for passive serial.
I do not see that there is another place to add a device-tree based
FPGA configuration, so you basically have to replicate the MTD
framework, in order to have FPGA configuration in the device tree.
Best Regards
Ulf Samuelsson
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
On 2/12/23 20:52, Ulf Samuelsson wrote:
Den 2023-02-12 kl. 20:31, skrev Marek Vasut:
> On 2/11/23 11:07, u-b...@emagii.com wrote:
>
> [...]
>
>> +static int cyc10_write(struct mtd_info *mtd, loff_t to, size_t len,
>> + size_t *retlen, const u_char *buf)
>> +{
>> + struct udevice *dev = mtd->dev;
>> + struct spi_slave *slave = dev_get_parent_priv(dev);
>> + struct cyc10_plat *fpga = dev_get_plat(dev);
>> + int ret;
>
> Do I read this right, that the 'write' callback is the only one doing
> meaningful work, all the other callbacks are just empty stubs ?
> Yes, you cannot read back the configuration data.
That makes it look like any framework which supports "write" callback
would be suitable, not just MTD framework.
> Why not update drivers/fpga/cyclon2.c which is Passive Serial
> implementation already present in U-Boot for Altera Cyclone II FPGA ,
> with Cyclone 10 FPGA support ? I believe the PS protocol changed very
> little.
Since the MTD command set is enough to configure the FPGA, the FPGA
commands can be removed from the build. The FPGA command set requires
you to supply addresses, but the MTD command set uses devices.
So:
* Smaller U-Boot image
The MTD framework is rather large, compared to the trivial FPGA
framework. Can you back this claim with any numbers ?
* Simplified user interface.
If I am to select between 'fpga load' and 'mtd write' for FPGA bitstream
loading , my obvious choice would be 'fpga load' . How is using 'mtd
write' any better or simpler ?
* The FPGA is an SPI peripheral, so why not add it to the SPI part of
the device tree?
You can add the device into DT and still operate it using the U-Boot
FPGA framework, just add the DT support. Why not do it that way ?
Let me be blunt about this, I have this feeling that what is happening
here is just overloading of MTD framework with unrelated functionality
(FPGA bitstream loading). MTD framework simply is not the right place,
esp. if there is dedicated FPGA framework, with existing Altera PS
driver no less.
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
Den 2023-02-12 kl. 20:31, skrev Marek Vasut:
> On 2/11/23 11:07, u-b...@emagii.com wrote:
>
> [...]
>
>> +static int cyc10_write(struct mtd_info *mtd, loff_t to, size_t len,
>> + size_t *retlen, const u_char *buf)
>> +{
>> +struct udevice *dev = mtd->dev;
>> +struct spi_slave *slave = dev_get_parent_priv(dev);
>> +struct cyc10_plat *fpga = dev_get_plat(dev);
>> +int ret;
>
> Do I read this right, that the 'write' callback is the only one doing
> meaningful work, all the other callbacks are just empty stubs ?
> Yes, you cannot read back the configuration data.
> Why not update drivers/fpga/cyclon2.c which is Passive Serial
> implementation already present in U-Boot for Altera Cyclone II FPGA ,
> with Cyclone 10 FPGA support ? I believe the PS protocol changed very
> little.
Since the MTD command set is enough to configure the FPGA, the FPGA
commands can be removed from the build. The FPGA command set requires
you to supply addresses, but the MTD command set uses devices.
So:
* Smaller U-Boot image
* Simplified user interface.
* The FPGA is an SPI peripheral, so why not add it to the SPI part of
the device tree?
Best Regards
Ulf Samuelsson
>
> [...]
Re: [PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
On 2/11/23 11:07, u-b...@emagii.com wrote:
[...]
+static int cyc10_write(struct mtd_info *mtd, loff_t to, size_t len,
+size_t *retlen, const u_char *buf)
+{
+ struct udevice *dev = mtd->dev;
+ struct spi_slave *slave = dev_get_parent_priv(dev);
+ struct cyc10_plat *fpga = dev_get_plat(dev);
+ int ret;
Do I read this right, that the 'write' callback is the only one doing
meaningful work, all the other callbacks are just empty stubs ?
Why not update drivers/fpga/cyclon2.c which is Passive Serial
implementation already present in U-Boot for Altera Cyclone II FPGA ,
with Cyclone 10 FPGA support ? I believe the PS protocol changed very
little.
[...]
[PATCH 3/4] mtd/fpga: add fpga directory to mtd (with Cyclone 10)
From: Ulf Samuelsson
Signed-off-by: Ulf Samuelsson
---
drivers/mtd/fpga/Kconfig | 47 ++
drivers/mtd/fpga/Makefile | 6 +
drivers/mtd/fpga/cyclone_10.c | 278 ++
3 files changed, 331 insertions(+)
create mode 100644 drivers/mtd/fpga/Kconfig
create mode 100644 drivers/mtd/fpga/Makefile
create mode 100644 drivers/mtd/fpga/cyclone_10.c
diff --git a/drivers/mtd/fpga/Kconfig b/drivers/mtd/fpga/Kconfig
new file mode 100644
index 00..e3aa8c4522
--- /dev/null
+++ b/drivers/mtd/fpga/Kconfig
@@ -0,0 +1,47 @@
+menu "SPI FPGA Support"
+
+config DM_SPI_FPGA
+ bool "Enable Driver Model for FPGA configuration"
+ depends on DM && DM_SPI
+ imply SPI_FPGA
+ help
+ Enable driver model for FPGAs configurable using SPI.
+ This SPI FPGA interface
+ (spi_fpga_probe(), spi_fpga_write(), etc.) is then
+ implemented by the SPI FPGA uclass.
+ There is one standard SPI FPGA driver which knows how to probe
+ chips supported by U-Boot. The uclass interface is defined in
+ include/spi_fpga.h
+ SPI and SPI FPGA must be enabled together
+ (it is not possible to use driver model for one and not the other).
+
+if DM_SPI_FPGA
+
+config SPI_FPGA_MTD
+ bool "SPI FPGA MTD support"
+ depends on MTD
+ help
+ Enable the MTD support for the FPGA SPI Passive Serial,
+ This allows mtd_write commands to load an FPGA using passive serial
+ If unsure, say N
+
+config SPI_FPGA_INTEL
+ bool "Intel/Altera FPGA Passive Serial configuration using SPI"
+ help
+ Add support for various Intel SPI FPGA chips
+
+config SPI_FPGA_XILINX
+ bool "Xilinx FPGA Passive Serial configuration using SPI"
+ help
+ Add support for various Xilinx FPGA chips
+
+config SPI_FPGA_CYCLONE10
+ bool "Cyclone 10 SPI FPGA MTD support"
+ depends on SPI_FPGA_MTD && SPI_FPGA_INTEL
+ help
+ Enable the MTD support for the Cyclone 10 FPGA
+ If unsure, say N
+
+endif
+
+endmenu # menu "SPI FPGA Support"
diff --git a/drivers/mtd/fpga/Makefile b/drivers/mtd/fpga/Makefile
new file mode 100644
index 00..2cf19fc7cf
--- /dev/null
+++ b/drivers/mtd/fpga/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# (C) Copyright 2006
+# Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+
+obj-$(CONFIG_SPI_FPGA_CYCLONE10) += cyclone_10.o
diff --git a/drivers/mtd/fpga/cyclone_10.c b/drivers/mtd/fpga/cyclone_10.c
new file mode 100644
index 00..41e273211e
--- /dev/null
+++ b/drivers/mtd/fpga/cyclone_10.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * MTD Driver for Passive Serial configuration of Cyclone 10
+ *
+ * Copyright (C) 2020 Bombardier Transportation
+ * Ulf Samuelsson
+ * Ulf Samuelsson
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+//#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * How many milliseconds from CONF_DONE high to enter user mode
+ * Datasheet says 650 us, Delay 2 ms to be safe...
+ */
+#defineUSER_MODE_DELAY 2
+
+struct cyc10_plat {
+ struct udevice *dev;
+ struct spi_slave*spi;
+ charname[8];
+ struct gpio_descnconfig;
+ struct gpio_descnstatus;
+ struct gpio_descconf_done;
+ struct gpio_desccrc_error;
+ u32 cs;
+ int flags;
+ int config_size;
+};
+
+static inline void write_nCONFIG(struct cyc10_plat *fpga, int value)
+{
+ dm_gpio_set_value(>nconfig, value);
+}
+
+static inline int read_nSTATUS(struct cyc10_plat *fpga)
+{
+ int val = dm_gpio_get_value(>nstatus);
+ if (val < 0) {
+ printf("%s: Failure reading nSTATUS; error=%d\n", fpga->name,
val);
+ }
+ return val;
+}
+
+static inline int read_CONFIG_DONE(struct cyc10_plat *fpga)
+{
+ int val = dm_gpio_get_value(>conf_done);
+ if (val < 0) {
+ printf("%s: Failure reading CONFIG_DONE; error=%d\n",
fpga->name, val);
+ }
+ return val;
+}
+
+static inline int read_CRC_ERROR(struct cyc10_plat *fpga)
+{
+ int val = dm_gpio_get_value(>crc_error);
+ if (val < 0) {
+ printf("%s: Failure reading CRC_ERROR; error=%d\n", fpga->name,
val);
+ }
+ return val;
+}
+
+/*
+ * Service routine to read status register until ready, or timeout occurs.
+ * Returns non-zero if error.
+ */
+static int cyc10_wait_until_conf_done(struct cyc10_plat *fpga)
+{
+ unsigned long timebase;
+ timebase = get_timer(0);
+
+ while (get_timer(timebase) < USER_MODE_DELAY) {
+ if (read_nSTATUS(fpga) == 0)/* Bad configuration */
+ return -EIO;
+
