[PATCH] Add new command fs2bridge for socfpga

[PATCH] Add new command fs2bridge for socfpga

[Mabcded Babcde]

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. 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. I'm a beginner to embedded and
open-source software. This is the reason why I don't know to which
branch I have to push this patch and I don't know which license should
be applied here because I used some code from another source (see
above GPL License / Altera). Nevertheless I would be happy to
contribute to Barebox.
Thanks, Mathieu


---
 commands/Kconfig     |  7 ++++
 commands/Makefile    |  1 +
 commands/f2sbridge.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)
 create mode 100644 commands/f2sbridge.c

diff --git a/commands/Kconfig b/commands/Kconfig
index ae2dc4b..328ab1b 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -2127,6 +2127,13 @@ config CMD_SEED
     help
       Seed the pseudo random number generator (PRNG)

+config CMD_F2SBRIDGE
+    bool
+    depends on ARCH_SOCFPGA
+    prompt "f2sbridge"
+    help
+      Enables or disables the fpga2sdram bridge
+
 # end Miscellaneous commands
 endmenu

diff --git a/commands/Makefile b/commands/Makefile
index 37486dc..2006bca 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -123,3 +123,4 @@ obj-$(CONFIG_CMD_SPD_DECODE)    += spd_decode.o
 obj-$(CONFIG_CMD_MMC_EXTCSD)    += mmc_extcsd.o
 obj-$(CONFIG_CMD_NAND_BITFLIP)    += nand-bitflip.o
 obj-$(CONFIG_CMD_SEED)        += seed.o
+obj-$(CONFIG_CMD_F2SBRIDGE)    += f2sbridge.o
diff --git a/commands/f2sbridge.c b/commands/f2sbridge.c
new file mode 100644
index 0000000..8d3bbcc
--- /dev/null
+++ b/commands/f2sbridge.c
@@ -0,0 +1,92 @@
+/*
+ *  Copyright (C) ???
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+
+#include <common.h>
+#include <io.h>
+#include <command.h>
+
+#define SOCFPGA_SYSMGR_ADDRESS    0xFFD08000
+#define SOCFPGA_SDR_ADDRESS    0xFFC20000
+
+#define SDR_CTRLGRP_FPGAPORTRST_ADDRESS    0x5080
+#define SDR_CTRLGRP_STATICCFG_ADDRESS    0x505C
+#define SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK    0x00000008
+
+#define SYSMGR_FPGAINTF_MODULE    (SOCFPGA_SYSMGR_ADDRESS + 0x28)
+
+
+static void socfpga_sdram_apply_staticcfg(void)
+{
+    const uint32_t staticcfg = SOCFPGA_SDR_ADDRESS +
SDR_CTRLGRP_STATICCFG_ADDRESS;
+    const uint32_t applymask = SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK;
+    uint32_t val = readl(staticcfg) | applymask;
+
+    /*
+     * SDRAM staticcfg register specific:
+     * When applying the register setting, the CPU must not access
+     * SDRAM. Luckily for us, we can abuse i-cache here to help us
+     * circumvent the SDRAM access issue. The idea is to make sure
+     * that the code is in one full i-cache line by branching past
+     * it and back. Once it is in the i-cache, we execute the core
+     * of the code and apply the register settings.
+     *
+     * The code below uses 7 instructions, while the Cortex-A9 has
+     * 32-byte cachelines, thus the limit is 8 instructions total.
+     */
+    asm volatile(
+        ".align    5            \n"
+        "    b    2f        \n"
+        "1:    str    %0,    [%1]    \n"
+        "    dsb            \n"
+        "    isb            \n"
+        "    b    3f        \n"
+        "2:    b    1b        \n"
+        "3:    nop            \n"
+    : : "r"(val), "r"(staticcfg) : "memory", "cc");
+}
+
+
+int do_f2sbridge(int argc, char *argv[])
+{
+    if (argc != 2)
+        return COMMAND_ERROR_USAGE;
+
+    switch (*argv[1]) {
+    case 'e':    // Enable
+        // hps peripheral controller to fpga
+        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
+        socfpga_sdram_apply_staticcfg();
+        // enable all fpga2sdram bridge ports
+        iowrite32(0x3fff, SOCFPGA_SDR_ADDRESS +
SDR_CTRLGRP_FPGAPORTRST_ADDRESS);
+
+        break;
+    case 'd':    // Disable
+        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
+        iowrite32(0, SOCFPGA_SDR_ADDRESS + SDR_CTRLGRP_FPGAPORTRST_ADDRESS);
+        socfpga_sdram_apply_staticcfg();
+
+        break;
+    default:
+        return COMMAND_ERROR_USAGE;
+    }
+
+    return 0;
+}
+
+
+BAREBOX_CMD_HELP_START(f2sbridge)
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-e \t",  "enable f2s bridge")
+BAREBOX_CMD_HELP_OPT ("-d \t",  "disable f2s bridge")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(f2sbridge)
+    .cmd        = do_f2sbridge,
+    BAREBOX_CMD_DESC("Enables or disables the fpga2sdram bridge.")
+    BAREBOX_CMD_GROUP(CMD_GRP_MISC)
+    BAREBOX_CMD_HELP(cmd_f2sbridge_help)
+BAREBOX_CMD_END
-- 
2.7.4

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Trent Piepho]

On Thu, 2017-07-27 at 18:20 +0200, Mabcded Babcde 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

I wonder if this might be better as a settable parameter of the fpga
device?

> +
> +#define SOCFPGA_SYSMGR_ADDRESS    0xFFD08000
> +#define SOCFPGA_SDR_ADDRESS    0xFFC20000
> +
> +#define SDR_CTRLGRP_FPGAPORTRST_ADDRESS    0x5080
> +#define SDR_CTRLGRP_STATICCFG_ADDRESS    0x505C
> +#define SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK    0x00000008
> +
> +#define SYSMGR_FPGAINTF_MODULE    (SOCFPGA_SYSMGR_ADDRESS + 0x28)

You should be able to get these addresses from socfpga-regs.h and
system-manager.h

> +
> +
> +static void socfpga_sdram_apply_staticcfg(void)
> +{
> +    const uint32_t staticcfg = SOCFPGA_SDR_ADDRESS +
> SDR_CTRLGRP_STATICCFG_ADDRESS;
> +    const uint32_t applymask = SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK;
> +    uint32_t val = readl(staticcfg) | applymask;
> +
> +    /*
> +     * SDRAM staticcfg register specific:
> +     * When applying the register setting, the CPU must not access
> +     * SDRAM. Luckily for us, we can abuse i-cache here to help us
> +     * circumvent the SDRAM access issue. The idea is to make sure
> +     * that the code is in one full i-cache line by branching past
> +     * it and back. Once it is in the i-cache, we execute the core
> +     * of the code and apply the register settings.
> +     *
> +     * The code below uses 7 instructions, while the Cortex-A9 has
> +     * 32-byte cachelines, thus the limit is 8 instructions total.
> +     */
> +    asm volatile(
> +        ".align    5            \n"
> +        "    b    2f        \n"
> +        "1:    str    %0,    [%1]    \n"
> +        "    dsb            \n"
> +        "    isb            \n"
> +        "    b    3f        \n"
> +        "2:    b    1b        \n"
> +        "3:    nop            \n"
> +    : : "r"(val), "r"(staticcfg) : "memory", "cc");
> +}
> +
> +
> +int do_f2sbridge(int argc, char *argv[])
> +{
> +    if (argc != 2)
> +        return COMMAND_ERROR_USAGE;
> +
> +    switch (*argv[1]) {
> +    case 'e':    // Enable
> +        // hps peripheral controller to fpga
> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);

Why disable FPGA access to the emacs?  Is that required too?

> +        socfpga_sdram_apply_staticcfg();
> +        // enable all fpga2sdram bridge ports
> +        iowrite32(0x3fff, SOCFPGA_SDR_ADDRESS +
> SDR_CTRLGRP_FPGAPORTRST_ADDRESS);
> +
> +        break;
> +    case 'd':    // Disable
> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
> +        iowrite32(0, SOCFPGA_SDR_ADDRESS + SDR_CTRLGRP_FPGAPORTRST_ADDRESS);
> +        socfpga_sdram_apply_staticcfg();
> +
> +        break;
> +    default:
> +        return COMMAND_ERROR_USAGE;
> +    }
> +
> +    return 0;
> +}
> +
> +
> +BAREBOX_CMD_HELP_START(f2sbridge)
> +BAREBOX_CMD_HELP_TEXT("Options:")
> +BAREBOX_CMD_HELP_OPT ("-e \t",  "enable f2s bridge")
> +BAREBOX_CMD_HELP_OPT ("-d \t",  "disable f2s bridge")
> +BAREBOX_CMD_HELP_END
> +
> +BAREBOX_CMD_START(f2sbridge)
> +    .cmd        = do_f2sbridge,
> +    BAREBOX_CMD_DESC("Enables or disables the fpga2sdram bridge.")
> +    BAREBOX_CMD_GROUP(CMD_GRP_MISC)
> +    BAREBOX_CMD_HELP(cmd_f2sbridge_help)
> +BAREBOX_CMD_END

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Mabcded Babcde]

2017-07-27 20:14 GMT+02:00 Trent Piepho <tpiepho@kymetacorp.com>:
> On Thu, 2017-07-27 at 18:20 +0200, Mabcded Babcde 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
>
> I wonder if this might be better as a settable parameter of the fpga
> device?
>

Ok, but how can I then ensure that the SDRAM is completly idle?

>> +
>> +#define SOCFPGA_SYSMGR_ADDRESS    0xFFD08000
>> +#define SOCFPGA_SDR_ADDRESS    0xFFC20000
>> +
>> +#define SDR_CTRLGRP_FPGAPORTRST_ADDRESS    0x5080
>> +#define SDR_CTRLGRP_STATICCFG_ADDRESS    0x505C
>> +#define SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK    0x00000008
>> +
>> +#define SYSMGR_FPGAINTF_MODULE    (SOCFPGA_SYSMGR_ADDRESS + 0x28)
>
> You should be able to get these addresses from socfpga-regs.h and
> system-manager.h
>

Thanks for the hint, I included mach/cyclone5-regs,
mach/cyclone5-sdram.h and cyclone5-system-manager.
But I did't find socfpga-regs.h and system-manager.h.

>> +
>> +
>> +static void socfpga_sdram_apply_staticcfg(void)
>> +{
>> +    const uint32_t staticcfg = SOCFPGA_SDR_ADDRESS +
>> SDR_CTRLGRP_STATICCFG_ADDRESS;
>> +    const uint32_t applymask = SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK;
>> +    uint32_t val = readl(staticcfg) | applymask;
>> +
>> +    /*
>> +     * SDRAM staticcfg register specific:
>> +     * When applying the register setting, the CPU must not access
>> +     * SDRAM. Luckily for us, we can abuse i-cache here to help us
>> +     * circumvent the SDRAM access issue. The idea is to make sure
>> +     * that the code is in one full i-cache line by branching past
>> +     * it and back. Once it is in the i-cache, we execute the core
>> +     * of the code and apply the register settings.
>> +     *
>> +     * The code below uses 7 instructions, while the Cortex-A9 has
>> +     * 32-byte cachelines, thus the limit is 8 instructions total.
>> +     */
>> +    asm volatile(
>> +        ".align    5            \n"
>> +        "    b    2f        \n"
>> +        "1:    str    %0,    [%1]    \n"
>> +        "    dsb            \n"
>> +        "    isb            \n"
>> +        "    b    3f        \n"
>> +        "2:    b    1b        \n"
>> +        "3:    nop            \n"
>> +    : : "r"(val), "r"(staticcfg) : "memory", "cc");
>> +}
>> +
>> +
>> +int do_f2sbridge(int argc, char *argv[])
>> +{
>> +    if (argc != 2)
>> +        return COMMAND_ERROR_USAGE;
>> +
>> +    switch (*argv[1]) {
>> +    case 'e':    // Enable
>> +        // hps peripheral controller to fpga
>> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
>
> Why disable FPGA access to the emacs?  Is that required too?
>

In the altera code it is also disabled. But I tried it without the
line and the bridges seem to be enabled.

>> +        socfpga_sdram_apply_staticcfg();
>> +        // enable all fpga2sdram bridge ports
>> +        iowrite32(0x3fff, SOCFPGA_SDR_ADDRESS +
>> SDR_CTRLGRP_FPGAPORTRST_ADDRESS);
>> +
>> +        break;
>> +    case 'd':    // Disable
>> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
>> +        iowrite32(0, SOCFPGA_SDR_ADDRESS + SDR_CTRLGRP_FPGAPORTRST_ADDRESS);
>> +        socfpga_sdram_apply_staticcfg();
>> +
>> +        break;
>> +    default:
>> +        return COMMAND_ERROR_USAGE;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
>> +BAREBOX_CMD_HELP_START(f2sbridge)
>> +BAREBOX_CMD_HELP_TEXT("Options:")
>> +BAREBOX_CMD_HELP_OPT ("-e \t",  "enable f2s bridge")
>> +BAREBOX_CMD_HELP_OPT ("-d \t",  "disable f2s bridge")
>> +BAREBOX_CMD_HELP_END
>> +
>> +BAREBOX_CMD_START(f2sbridge)
>> +    .cmd        = do_f2sbridge,
>> +    BAREBOX_CMD_DESC("Enables or disables the fpga2sdram bridge.")
>> +    BAREBOX_CMD_GROUP(CMD_GRP_MISC)
>> +    BAREBOX_CMD_HELP(cmd_f2sbridge_help)
>> +BAREBOX_CMD_END
>

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Andrey Smirnov]

On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
<thepusherpushes@gmail.com> 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.

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

> I'm a beginner to embedded and
> open-source software. This is the reason why I don't know to which
> branch I have to push this patch and I don't know which license should
> be applied here because I used some code from another source (see
> above GPL License / Altera). Nevertheless I would be happy to
> contribute to Barebox.
> Thanks, Mathieu
>
>
> ---

Just in case you didn't know this already, text below this line will
be ignored when the patch is applied, so this is a great place to put
some technical or personal notes that might not necessarily belong to
the commit message.


>  commands/Kconfig     |  7 ++++
>  commands/Makefile    |  1 +
>  commands/f2sbridge.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 100 insertions(+)
>  create mode 100644 commands/f2sbridge.c
>
> diff --git a/commands/Kconfig b/commands/Kconfig
> index ae2dc4b..328ab1b 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -2127,6 +2127,13 @@ config CMD_SEED
>      help
>        Seed the pseudo random number generator (PRNG)
>
> +config CMD_F2SBRIDGE
> +    bool
> +    depends on ARCH_SOCFPGA
> +    prompt "f2sbridge"
> +    help
> +      Enables or disables the fpga2sdram bridge
> +
>  # end Miscellaneous commands
>  endmenu
>
> diff --git a/commands/Makefile b/commands/Makefile
> index 37486dc..2006bca 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -123,3 +123,4 @@ obj-$(CONFIG_CMD_SPD_DECODE)    += spd_decode.o
>  obj-$(CONFIG_CMD_MMC_EXTCSD)    += mmc_extcsd.o
>  obj-$(CONFIG_CMD_NAND_BITFLIP)    += nand-bitflip.o
>  obj-$(CONFIG_CMD_SEED)        += seed.o
> +obj-$(CONFIG_CMD_F2SBRIDGE)    += f2sbridge.o
> diff --git a/commands/f2sbridge.c b/commands/f2sbridge.c
> new file mode 100644
> index 0000000..8d3bbcc
> --- /dev/null
> +++ b/commands/f2sbridge.c
> @@ -0,0 +1,92 @@
> +/*
> + *  Copyright (C) ???

Maybe:

Copyright (C) 2017 Your Name

Based on the code form U-Boot

 Copyright (C) Year, Altera

?

> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +
> +#include <common.h>
> +#include <io.h>
> +#include <command.h>
> +
> +#define SOCFPGA_SYSMGR_ADDRESS    0xFFD08000
> +#define SOCFPGA_SDR_ADDRESS    0xFFC20000
> +
> +#define SDR_CTRLGRP_FPGAPORTRST_ADDRESS    0x5080
> +#define SDR_CTRLGRP_STATICCFG_ADDRESS    0x505C
> +#define SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK    0x00000008
> +
> +#define SYSMGR_FPGAINTF_MODULE    (SOCFPGA_SYSMGR_ADDRESS + 0x28)
> +
> +
> +static void socfpga_sdram_apply_staticcfg(void)
> +{
> +    const uint32_t staticcfg = SOCFPGA_SDR_ADDRESS +
> SDR_CTRLGRP_STATICCFG_ADDRESS;
> +    const uint32_t applymask = SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK;
> +    uint32_t val = readl(staticcfg) | applymask;
> +
> +    /*
> +     * SDRAM staticcfg register specific:
> +     * When applying the register setting, the CPU must not access
> +     * SDRAM. Luckily for us, we can abuse i-cache here to help us
> +     * circumvent the SDRAM access issue. The idea is to make sure
> +     * that the code is in one full i-cache line by branching past
> +     * it and back. Once it is in the i-cache, we execute the core
> +     * of the code and apply the register settings.
> +     *
> +     * The code below uses 7 instructions, while the Cortex-A9 has
> +     * 32-byte cachelines, thus the limit is 8 instructions total.
> +     */
> +    asm volatile(
> +        ".align    5            \n"
> +        "    b    2f        \n"
> +        "1:    str    %0,    [%1]    \n"
> +        "    dsb            \n"
> +        "    isb            \n"
> +        "    b    3f        \n"
> +        "2:    b    1b        \n"
> +        "3:    nop            \n"
> +    : : "r"(val), "r"(staticcfg) : "memory", "cc");
> +}
> +
> +
> +int do_f2sbridge(int argc, char *argv[])
> +{
> +    if (argc != 2)
> +        return COMMAND_ERROR_USAGE;
> +
> +    switch (*argv[1]) {
> +    case 'e':    // Enable
> +        // hps peripheral controller to fpga
> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);

You use readl() above but iowrite32() (as opposed to writel()) here,
is that on purpose?

Thank you for the contribution!

Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Mabcded Babcde]

2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
> <thepusherpushes@gmail.com> 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'm a beginner to embedded and
>> open-source software. This is the reason why I don't know to which
>> branch I have to push this patch and I don't know which license should
>> be applied here because I used some code from another source (see
>> above GPL License / Altera). Nevertheless I would be happy to
>> contribute to Barebox.
>> Thanks, Mathieu
>>
>>
>> ---
>
> Just in case you didn't know this already, text below this line will
> be ignored when the patch is applied, so this is a great place to put
> some technical or personal notes that might not necessarily belong to
> the commit message.
>

Thanks!

>
>>  commands/Kconfig     |  7 ++++
>>  commands/Makefile    |  1 +
>>  commands/f2sbridge.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 100 insertions(+)
>>  create mode 100644 commands/f2sbridge.c
>>
>> diff --git a/commands/Kconfig b/commands/Kconfig
>> index ae2dc4b..328ab1b 100644
>> --- a/commands/Kconfig
>> +++ b/commands/Kconfig
>> @@ -2127,6 +2127,13 @@ config CMD_SEED
>>      help
>>        Seed the pseudo random number generator (PRNG)
>>
>> +config CMD_F2SBRIDGE
>> +    bool
>> +    depends on ARCH_SOCFPGA
>> +    prompt "f2sbridge"
>> +    help
>> +      Enables or disables the fpga2sdram bridge
>> +
>>  # end Miscellaneous commands
>>  endmenu
>>
>> diff --git a/commands/Makefile b/commands/Makefile
>> index 37486dc..2006bca 100644
>> --- a/commands/Makefile
>> +++ b/commands/Makefile
>> @@ -123,3 +123,4 @@ obj-$(CONFIG_CMD_SPD_DECODE)    += spd_decode.o
>>  obj-$(CONFIG_CMD_MMC_EXTCSD)    += mmc_extcsd.o
>>  obj-$(CONFIG_CMD_NAND_BITFLIP)    += nand-bitflip.o
>>  obj-$(CONFIG_CMD_SEED)        += seed.o
>> +obj-$(CONFIG_CMD_F2SBRIDGE)    += f2sbridge.o
>> diff --git a/commands/f2sbridge.c b/commands/f2sbridge.c
>> new file mode 100644
>> index 0000000..8d3bbcc
>> --- /dev/null
>> +++ b/commands/f2sbridge.c
>> @@ -0,0 +1,92 @@
>> +/*
>> + *  Copyright (C) ???
>
> Maybe:
>
> Copyright (C) 2017 Your Name
>
> Based on the code form U-Boot
>
>  Copyright (C) Year, Altera
>
> ?

Thanks!

>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +
>> +#include <common.h>
>> +#include <io.h>
>> +#include <command.h>
>> +
>> +#define SOCFPGA_SYSMGR_ADDRESS    0xFFD08000
>> +#define SOCFPGA_SDR_ADDRESS    0xFFC20000
>> +
>> +#define SDR_CTRLGRP_FPGAPORTRST_ADDRESS    0x5080
>> +#define SDR_CTRLGRP_STATICCFG_ADDRESS    0x505C
>> +#define SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK    0x00000008
>> +
>> +#define SYSMGR_FPGAINTF_MODULE    (SOCFPGA_SYSMGR_ADDRESS + 0x28)
>> +
>> +
>> +static void socfpga_sdram_apply_staticcfg(void)
>> +{
>> +    const uint32_t staticcfg = SOCFPGA_SDR_ADDRESS +
>> SDR_CTRLGRP_STATICCFG_ADDRESS;
>> +    const uint32_t applymask = SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK;
>> +    uint32_t val = readl(staticcfg) | applymask;
>> +
>> +    /*
>> +     * SDRAM staticcfg register specific:
>> +     * When applying the register setting, the CPU must not access
>> +     * SDRAM. Luckily for us, we can abuse i-cache here to help us
>> +     * circumvent the SDRAM access issue. The idea is to make sure
>> +     * that the code is in one full i-cache line by branching past
>> +     * it and back. Once it is in the i-cache, we execute the core
>> +     * of the code and apply the register settings.
>> +     *
>> +     * The code below uses 7 instructions, while the Cortex-A9 has
>> +     * 32-byte cachelines, thus the limit is 8 instructions total.
>> +     */
>> +    asm volatile(
>> +        ".align    5            \n"
>> +        "    b    2f        \n"
>> +        "1:    str    %0,    [%1]    \n"
>> +        "    dsb            \n"
>> +        "    isb            \n"
>> +        "    b    3f        \n"
>> +        "2:    b    1b        \n"
>> +        "3:    nop            \n"
>> +    : : "r"(val), "r"(staticcfg) : "memory", "cc");
>> +}
>> +
>> +
>> +int do_f2sbridge(int argc, char *argv[])
>> +{
>> +    if (argc != 2)
>> +        return COMMAND_ERROR_USAGE;
>> +
>> +    switch (*argv[1]) {
>> +    case 'e':    // Enable
>> +        // hps peripheral controller to fpga
>> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
>
> You use readl() above but iowrite32() (as opposed to writel()) here,
> is that on purpose?

That was a mistake. I changed it to ioread32.

>
> Thank you for the contribution!
>
> Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Andrey Smirnov]

On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
<thepusherpushes@gmail.com> wrote:
> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>> <thepusherpushes@gmail.com> 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".

>
>>> I'm a beginner to embedded and
>>> open-source software. This is the reason why I don't know to which
>>> branch I have to push this patch and I don't know which license should
>>> be applied here because I used some code from another source (see
>>> above GPL License / Altera). Nevertheless I would be happy to
>>> contribute to Barebox.
>>> Thanks, Mathieu
>>>
>>>
>>> ---
>>
>> Just in case you didn't know this already, text below this line will
>> be ignored when the patch is applied, so this is a great place to put
>> some technical or personal notes that might not necessarily belong to
>> the commit message.
>>
>
> Thanks!
>
>>
>>>  commands/Kconfig     |  7 ++++
>>>  commands/Makefile    |  1 +
>>>  commands/f2sbridge.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 100 insertions(+)
>>>  create mode 100644 commands/f2sbridge.c
>>>
>>> diff --git a/commands/Kconfig b/commands/Kconfig
>>> index ae2dc4b..328ab1b 100644
>>> --- a/commands/Kconfig
>>> +++ b/commands/Kconfig
>>> @@ -2127,6 +2127,13 @@ config CMD_SEED
>>>      help
>>>        Seed the pseudo random number generator (PRNG)
>>>
>>> +config CMD_F2SBRIDGE
>>> +    bool
>>> +    depends on ARCH_SOCFPGA
>>> +    prompt "f2sbridge"
>>> +    help
>>> +      Enables or disables the fpga2sdram bridge
>>> +
>>>  # end Miscellaneous commands
>>>  endmenu
>>>
>>> diff --git a/commands/Makefile b/commands/Makefile
>>> index 37486dc..2006bca 100644
>>> --- a/commands/Makefile
>>> +++ b/commands/Makefile
>>> @@ -123,3 +123,4 @@ obj-$(CONFIG_CMD_SPD_DECODE)    += spd_decode.o
>>>  obj-$(CONFIG_CMD_MMC_EXTCSD)    += mmc_extcsd.o
>>>  obj-$(CONFIG_CMD_NAND_BITFLIP)    += nand-bitflip.o
>>>  obj-$(CONFIG_CMD_SEED)        += seed.o
>>> +obj-$(CONFIG_CMD_F2SBRIDGE)    += f2sbridge.o
>>> diff --git a/commands/f2sbridge.c b/commands/f2sbridge.c
>>> new file mode 100644
>>> index 0000000..8d3bbcc
>>> --- /dev/null
>>> +++ b/commands/f2sbridge.c
>>> @@ -0,0 +1,92 @@
>>> +/*
>>> + *  Copyright (C) ???
>>
>> Maybe:
>>
>> Copyright (C) 2017 Your Name
>>
>> Based on the code form U-Boot
>>
>>  Copyright (C) Year, Altera
>>
>> ?
>
> Thanks!
>
>>
>>> + *
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>> + */
>>> +
>>> +
>>> +#include <common.h>
>>> +#include <io.h>
>>> +#include <command.h>
>>> +
>>> +#define SOCFPGA_SYSMGR_ADDRESS    0xFFD08000
>>> +#define SOCFPGA_SDR_ADDRESS    0xFFC20000
>>> +
>>> +#define SDR_CTRLGRP_FPGAPORTRST_ADDRESS    0x5080
>>> +#define SDR_CTRLGRP_STATICCFG_ADDRESS    0x505C
>>> +#define SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK    0x00000008
>>> +
>>> +#define SYSMGR_FPGAINTF_MODULE    (SOCFPGA_SYSMGR_ADDRESS + 0x28)
>>> +
>>> +
>>> +static void socfpga_sdram_apply_staticcfg(void)
>>> +{
>>> +    const uint32_t staticcfg = SOCFPGA_SDR_ADDRESS +
>>> SDR_CTRLGRP_STATICCFG_ADDRESS;
>>> +    const uint32_t applymask = SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK;
>>> +    uint32_t val = readl(staticcfg) | applymask;
>>> +
>>> +    /*
>>> +     * SDRAM staticcfg register specific:
>>> +     * When applying the register setting, the CPU must not access
>>> +     * SDRAM. Luckily for us, we can abuse i-cache here to help us
>>> +     * circumvent the SDRAM access issue. The idea is to make sure
>>> +     * that the code is in one full i-cache line by branching past
>>> +     * it and back. Once it is in the i-cache, we execute the core
>>> +     * of the code and apply the register settings.
>>> +     *
>>> +     * The code below uses 7 instructions, while the Cortex-A9 has
>>> +     * 32-byte cachelines, thus the limit is 8 instructions total.
>>> +     */
>>> +    asm volatile(
>>> +        ".align    5            \n"
>>> +        "    b    2f        \n"
>>> +        "1:    str    %0,    [%1]    \n"
>>> +        "    dsb            \n"
>>> +        "    isb            \n"
>>> +        "    b    3f        \n"
>>> +        "2:    b    1b        \n"
>>> +        "3:    nop            \n"
>>> +    : : "r"(val), "r"(staticcfg) : "memory", "cc");
>>> +}
>>> +
>>> +
>>> +int do_f2sbridge(int argc, char *argv[])
>>> +{
>>> +    if (argc != 2)
>>> +        return COMMAND_ERROR_USAGE;
>>> +
>>> +    switch (*argv[1]) {
>>> +    case 'e':    // Enable
>>> +        // hps peripheral controller to fpga
>>> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
>>
>> 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.

Thanks,
Andrey Smirnov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Mabcded Babcde]

2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
> <thepusherpushes@gmail.com> wrote:
>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>>> <thepusherpushes@gmail.com> 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. However how would I
enable or disable the bridge? With a parameter? Where can I add it and
change it?

>
>>
>>>> I'm a beginner to embedded and
>>>> open-source software. This is the reason why I don't know to which
>>>> branch I have to push this patch and I don't know which license should
>>>> be applied here because I used some code from another source (see
>>>> above GPL License / Altera). Nevertheless I would be happy to
>>>> contribute to Barebox.
>>>> Thanks, Mathieu
>>>>
>>>>
>>>> ---
>>>
>>> Just in case you didn't know this already, text below this line will
>>> be ignored when the patch is applied, so this is a great place to put
>>> some technical or personal notes that might not necessarily belong to
>>> the commit message.
>>>
>>
>> Thanks!
>>
>>>
>>>>  commands/Kconfig     |  7 ++++
>>>>  commands/Makefile    |  1 +
>>>>  commands/f2sbridge.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 100 insertions(+)
>>>>  create mode 100644 commands/f2sbridge.c
>>>>
>>>> diff --git a/commands/Kconfig b/commands/Kconfig
>>>> index ae2dc4b..328ab1b 100644
>>>> --- a/commands/Kconfig
>>>> +++ b/commands/Kconfig
>>>> @@ -2127,6 +2127,13 @@ config CMD_SEED
>>>>      help
>>>>        Seed the pseudo random number generator (PRNG)
>>>>
>>>> +config CMD_F2SBRIDGE
>>>> +    bool
>>>> +    depends on ARCH_SOCFPGA
>>>> +    prompt "f2sbridge"
>>>> +    help
>>>> +      Enables or disables the fpga2sdram bridge
>>>> +
>>>>  # end Miscellaneous commands
>>>>  endmenu
>>>>
>>>> diff --git a/commands/Makefile b/commands/Makefile
>>>> index 37486dc..2006bca 100644
>>>> --- a/commands/Makefile
>>>> +++ b/commands/Makefile
>>>> @@ -123,3 +123,4 @@ obj-$(CONFIG_CMD_SPD_DECODE)    += spd_decode.o
>>>>  obj-$(CONFIG_CMD_MMC_EXTCSD)    += mmc_extcsd.o
>>>>  obj-$(CONFIG_CMD_NAND_BITFLIP)    += nand-bitflip.o
>>>>  obj-$(CONFIG_CMD_SEED)        += seed.o
>>>> +obj-$(CONFIG_CMD_F2SBRIDGE)    += f2sbridge.o
>>>> diff --git a/commands/f2sbridge.c b/commands/f2sbridge.c
>>>> new file mode 100644
>>>> index 0000000..8d3bbcc
>>>> --- /dev/null
>>>> +++ b/commands/f2sbridge.c
>>>> @@ -0,0 +1,92 @@
>>>> +/*
>>>> + *  Copyright (C) ???
>>>
>>> Maybe:
>>>
>>> Copyright (C) 2017 Your Name
>>>
>>> Based on the code form U-Boot
>>>
>>>  Copyright (C) Year, Altera
>>>
>>> ?
>>
>> Thanks!
>>
>>>
>>>> + *
>>>> + * SPDX-License-Identifier:    GPL-2.0+
>>>> + */
>>>> +
>>>> +
>>>> +#include <common.h>
>>>> +#include <io.h>
>>>> +#include <command.h>
>>>> +
>>>> +#define SOCFPGA_SYSMGR_ADDRESS    0xFFD08000
>>>> +#define SOCFPGA_SDR_ADDRESS    0xFFC20000
>>>> +
>>>> +#define SDR_CTRLGRP_FPGAPORTRST_ADDRESS    0x5080
>>>> +#define SDR_CTRLGRP_STATICCFG_ADDRESS    0x505C
>>>> +#define SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK    0x00000008
>>>> +
>>>> +#define SYSMGR_FPGAINTF_MODULE    (SOCFPGA_SYSMGR_ADDRESS + 0x28)
>>>> +
>>>> +
>>>> +static void socfpga_sdram_apply_staticcfg(void)
>>>> +{
>>>> +    const uint32_t staticcfg = SOCFPGA_SDR_ADDRESS +
>>>> SDR_CTRLGRP_STATICCFG_ADDRESS;
>>>> +    const uint32_t applymask = SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK;
>>>> +    uint32_t val = readl(staticcfg) | applymask;
>>>> +
>>>> +    /*
>>>> +     * SDRAM staticcfg register specific:
>>>> +     * When applying the register setting, the CPU must not access
>>>> +     * SDRAM. Luckily for us, we can abuse i-cache here to help us
>>>> +     * circumvent the SDRAM access issue. The idea is to make sure
>>>> +     * that the code is in one full i-cache line by branching past
>>>> +     * it and back. Once it is in the i-cache, we execute the core
>>>> +     * of the code and apply the register settings.
>>>> +     *
>>>> +     * The code below uses 7 instructions, while the Cortex-A9 has
>>>> +     * 32-byte cachelines, thus the limit is 8 instructions total.
>>>> +     */
>>>> +    asm volatile(
>>>> +        ".align    5            \n"
>>>> +        "    b    2f        \n"
>>>> +        "1:    str    %0,    [%1]    \n"
>>>> +        "    dsb            \n"
>>>> +        "    isb            \n"
>>>> +        "    b    3f        \n"
>>>> +        "2:    b    1b        \n"
>>>> +        "3:    nop            \n"
>>>> +    : : "r"(val), "r"(staticcfg) : "memory", "cc");
>>>> +}
>>>> +
>>>> +
>>>> +int do_f2sbridge(int argc, char *argv[])
>>>> +{
>>>> +    if (argc != 2)
>>>> +        return COMMAND_ERROR_USAGE;
>>>> +
>>>> +    switch (*argv[1]) {
>>>> +    case 'e':    // Enable
>>>> +        // hps peripheral controller to fpga
>>>> +        iowrite32(0, SYSMGR_FPGAINTF_MODULE);
>>>
>>> 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?

>
> Thanks,
> Andrey Smirnov

Thanks,
Mathieu

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Andrey Smirnov]

On Sun, Jul 30, 2017 at 2:17 PM, Mabcded Babcde
<thepusherpushes@gmail.com> wrote:
> 2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
>> <thepusherpushes@gmail.com> wrote:
>>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>>>> <thepusherpushes@gmail.com> 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@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Steffen Trumtrar]

Hi!

Andrey Smirnov <andrew.smirnov@gmail.com> writes:

> On Sun, Jul 30, 2017 at 2:17 PM, Mabcded Babcde
> <thepusherpushes@gmail.com> wrote:
>> 2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
>>> <thepusherpushes@gmail.com> wrote:
>>>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>>>>> <thepusherpushes@gmail.com> 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}
>

The right approach should be adding a proper bridge driver like in the
kernel. Then in the fpgamgr_program_finish enable all registered
bridges. And set the APPFLYCFG bit in the bridge driver.
Or do I miss something?

The cacheline magic seems rather fragile and therefore just running a
function (with almost the same assembler code) from OCRAM seems to be the
more robust option.

As a matter of fact, I already have the bridge drivers ported to
barebox. I can post the series as an RFC as I haven't tested it since I
rebased it to v2017.07.0.


Regards,
Steffen

-- 
Pengutronix e.K.                           | Steffen Trumtrar            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Mabcded Babcde]

2017-08-01 11:13 GMT+02:00 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>
> Hi!
>
> Andrey Smirnov <andrew.smirnov@gmail.com> writes:
>
>> On Sun, Jul 30, 2017 at 2:17 PM, Mabcded Babcde
>> <thepusherpushes@gmail.com> wrote:
>>> 2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
>>>> <thepusherpushes@gmail.com> wrote:
>>>>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>>>>>> <thepusherpushes@gmail.com> 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}
>>
>
> The right approach should be adding a proper bridge driver like in the
> kernel. Then in the fpgamgr_program_finish enable all registered
> bridges. And set the APPFLYCFG bit in the bridge driver.
> Or do I miss something?

I retried it and it works with my application.

>
> The cacheline magic seems rather fragile and therefore just running a
> function (with almost the same assembler code) from OCRAM seems to be the
> more robust option.
>
> As a matter of fact, I already have the bridge drivers ported to
> barebox. I can post the series as an RFC as I haven't tested it since I
> rebased it to v2017.07.0.

Perfect, thank you!

>
>
> Regards,
> Steffen
>
> --
> Pengutronix e.K.                           | Steffen Trumtrar            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Mabcded Babcde]

2017-08-01 11:13 GMT+02:00 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>
> Hi!
>
> Andrey Smirnov <andrew.smirnov@gmail.com> writes:
>
>> On Sun, Jul 30, 2017 at 2:17 PM, Mabcded Babcde
>> <thepusherpushes@gmail.com> wrote:
>>> 2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
>>>> <thepusherpushes@gmail.com> wrote:
>>>>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>>>>>> <thepusherpushes@gmail.com> 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}
>>
>
> The right approach should be adding a proper bridge driver like in the
> kernel. Then in the fpgamgr_program_finish enable all registered
> bridges. And set the APPFLYCFG bit in the bridge driver.
> Or do I miss something?
>
> The cacheline magic seems rather fragile and therefore just running a
> function (with almost the same assembler code) from OCRAM seems to be the
> more robust option.
>
> As a matter of fact, I already have the bridge drivers ported to
> barebox. I can post the series as an RFC as I haven't tested it since I
> rebased it to v2017.07.0.

I built Barebox successfully with your drivers, but the fpga-bridge
wasn't enabled. And where is the OCRAM part in your driver?

>
>
> Regards,
> Steffen
>
> --
> Pengutronix e.K.                           | Steffen Trumtrar            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Mabcded Babcde]

2017-08-04 15:13 GMT+02:00 Mabcded Babcde <thepusherpushes@gmail.com>:
> 2017-08-01 11:13 GMT+02:00 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>>
>> Hi!
>>
>> Andrey Smirnov <andrew.smirnov@gmail.com> writes:
>>
>>> On Sun, Jul 30, 2017 at 2:17 PM, Mabcded Babcde
>>> <thepusherpushes@gmail.com> wrote:
>>>> 2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>>> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
>>>>> <thepusherpushes@gmail.com> wrote:
>>>>>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>>>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>>>>>>> <thepusherpushes@gmail.com> 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}
>>>
>>
>> The right approach should be adding a proper bridge driver like in the
>> kernel. Then in the fpgamgr_program_finish enable all registered
>> bridges. And set the APPFLYCFG bit in the bridge driver.
>> Or do I miss something?
>>
>> The cacheline magic seems rather fragile and therefore just running a
>> function (with almost the same assembler code) from OCRAM seems to be the
>> more robust option.
>>
>> As a matter of fact, I already have the bridge drivers ported to
>> barebox. I can post the series as an RFC as I haven't tested it since I
>> rebased it to v2017.07.0.
>
> I built Barebox successfully with your drivers, but the fpga-bridge
> wasn't enabled. And where is the OCRAM part in your driver?

In socfpga-fpgasdram-bridge.c ALT_SDR_CTL_FPGAPORTRST_PORTRSTN_MSK is
defined, but not used.
In the same file the socfpga_sdram_apply_staticcfg part is missing in
static inline int _alt_fpga2sdram_enable_set(struct
alt_fpga2sdram_data *priv, bool enable)

>
>>
>>
>> Regards,
>> Steffen
>>
>> --
>> Pengutronix e.K.                           | Steffen Trumtrar            |
>> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Best regards,
Mathieu

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] Add new command fs2bridge for socfpga

[Steffen Trumtrar]

Mabcded Babcde <thepusherpushes@gmail.com> writes:

> 2017-08-04 15:13 GMT+02:00 Mabcded Babcde <thepusherpushes@gmail.com>:
>> 2017-08-01 11:13 GMT+02:00 Steffen Trumtrar <s.trumtrar@pengutronix.de>:
>>>
>>> Hi!
>>>
>>> Andrey Smirnov <andrew.smirnov@gmail.com> writes:
>>>
>>>> On Sun, Jul 30, 2017 at 2:17 PM, Mabcded Babcde
>>>> <thepusherpushes@gmail.com> wrote:
>>>>> 2017-07-28 15:42 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>>>> On Fri, Jul 28, 2017 at 6:24 AM, Mabcded Babcde
>>>>>> <thepusherpushes@gmail.com> wrote:
>>>>>>> 2017-07-27 20:49 GMT+02:00 Andrey Smirnov <andrew.smirnov@gmail.com>:
>>>>>>>> On Thu, Jul 27, 2017 at 9:20 AM, Mabcded Babcde
>>>>>>>> <thepusherpushes@gmail.com> 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}
>>>>
>>>
>>> The right approach should be adding a proper bridge driver like in the
>>> kernel. Then in the fpgamgr_program_finish enable all registered
>>> bridges. And set the APPFLYCFG bit in the bridge driver.
>>> Or do I miss something?
>>>
>>> The cacheline magic seems rather fragile and therefore just running a
>>> function (with almost the same assembler code) from OCRAM seems to be the
>>> more robust option.
>>>
>>> As a matter of fact, I already have the bridge drivers ported to
>>> barebox. I can post the series as an RFC as I haven't tested it since I
>>> rebased it to v2017.07.0.
>>
>> I built Barebox successfully with your drivers, but the fpga-bridge
>> wasn't enabled. And where is the OCRAM part in your driver?

I didn't say the driver is finished ;-)

>
> In socfpga-fpgasdram-bridge.c ALT_SDR_CTL_FPGAPORTRST_PORTRSTN_MSK is
> defined, but not used.

Yepp, it's hardcoded. Not good. Seems like the devicetree binding does
not include this mask value. This should also be added than to the
binding, as it depends on the bitstream.

> In the same file the socfpga_sdram_apply_staticcfg part is missing in
> static inline int _alt_fpga2sdram_enable_set(struct
> alt_fpga2sdram_data *priv, bool enable)
>

Correct.


Regards,
Steffen

-- 
Pengutronix e.K.                           | Steffen Trumtrar            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox