On Sun, Jul 30, 2017 at 2:17 PM, Mabcded Babcde <thepusherpushes@xxxxxxxxx> wrote: > 2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@xxxxxxxxx>: >> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde >> <thepusherpushes@xxxxxxxxx> wrote: >>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@xxxxxxxxx>: >>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde >>>> <thepusherpushes@xxxxxxxxx> wrote: >>>>> Hi, >>>>> this patch adds a new command to barebox. It is used to enable or >>>>> disable the fpga-to-sdram bridges on socfpgas. The patch is based on a >>>>> manual from altera >>>>> (https://www.altera.com/support/support-resources/knowledge-base/embedded/2016/how-and-when-can-i-enable-the-fpga2sdram-bridge-on-cyclone-v-soc.html) >>>>> and a implementation for u-boot >>>>> (https://github.com/rogerq/u-boot/blob/master/arch/arm/mach-socfpga/misc.c). >>>>> The fpga2sdram fpga configuration can only be set when the SDRAM >>>>> interface is idle. So it is necessary to use the on-chip ram. >>>> >>>> And by "on-chip ram" you mean i-cache and not SRAM block used by first >>>> stage bootloader, correct? I am just a bit confused by the wording. >>>> >>> >>> Sorry, my fault. I meant i-cache. >>> >>>>> To bring all fpga2sdram bridges out of reset it is necessary to write 0x3FFF to >>>>> the register. Only the fpga2sdram bridges are enabled or disabled but >>>>> no other bridges like the axi bridges. >>>> >>>> Linux kernel already has: >>>> >>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt?h=v4.13-rc2 >>>> >>>> so, IMHO, it would be better to implement a compatible driver and, if >>>> ability to change the value at run-time is needed, add "enable" >>>> property to it rather than introducing a separate command. >>>> >>> >>> But how can I ensure that the SDRAM is idle? I quote Altera: >>> "First time enabling of the FPGA2SDRAM bridge within Linux is not >>> supported as the SDRAM subsystem cannot be easily put into a >>> guaranteed idle state." >>> >> >> I feel like we are talking about different things. What I am trying to >> say that instead of adding this functionality to Barebox as a >> standalone command, it would be better to add it _to Barebox_ as a >> driver that claims compatibility with "altr,socfpga-fpga2sdram-bridge" >> and implements all of the necessary logic there. It would be the same >> code running in the boot-loader but triggered to executed via >> different means, so it should literally be the same in terms of SDRAM >> access quiescence. A somewhat relative example of that approach would >> be "drivers/firmware/socfpga.c". > > Ok, I hardcoded my code from the command to drivers/firmware/socfpga.c > for testing purposes. That didn't seem to work. I'd double check that CONFIG_FIRMWARE and CONFIG_FIRMWARE_ALTERA_SOCFPGA are set to "y". > However how would I > enable or disable the bridge? With a parameter? Where can I add it and > change it? > >> The driver in firmware/socfpga.c should create a "fpga0" device, after which, you cat set that device's variables/attributes by simply doing: fpga0.variable_name = value and get them via echo ${fpga0.variable_name} Commit https://git.pengutronix.de/cgit/barebox/commit/?id=48aecb409dcbff1d13008de72a2c42af8069aec6 where "programmed" attribute for that driver was added should be a pretty good example. >>>> You use readl() above but iowrite32() (as opposed to writel()) here, >>>> is that on purpose? >>> >>> That was a mistake. I changed it to ioread32. >>> >> >> I'd maybe convert it the other around since there are very few users >> of "ioread32" in the codebase and readl()/writel() seem to be more >> idiomatic. > > Ok, but I thought readl()/writel() are less safe? > I might be missing something, but, to the best of my knowledge, they are not. Ioread32/iowrite32 are defined to expand into readl/writel in include/asm-generic/io.h, so I'd say they should be the same in all aspects. Thanks, Andrey Smirnov _______________________________________________ barebox mailing list barebox@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/barebox
