On 11/27/2018 09:54 AM, Chee, Tien Fong wrote: > On Mon, 2018-11-26 at 12:18 +0100, Marek Vasut wrote: >> On 11/26/2018 11:09 AM, Chee, Tien Fong wrote: >> [...] >>> >>>> >>>>> >>>>>> >>>>>>> >>>>>>> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig >>>>>>> index 50e9019..06a8204 100644 >>>>>>> --- a/drivers/fpga/Kconfig >>>>>>> +++ b/drivers/fpga/Kconfig >>>>>>> @@ -21,6 +21,15 @@ config FPGA_SOCFPGA >>>>>>> >>>>>>> This provides common functionality for Gen5 and >>>>>>> Arria10 >>>>>>> devices. >>>>>>> >>>>>>> +config CHECK_FPGA_DATA_CRC >>>>>> config FPGA_SOCFPGA_A10_CRC_CHECK >>>>>> >>>>>> What is this for and why shouldn't this be ON by default ? >>>>> Both periph.rbf or full.rbf are wrapped with mkimage. So, this >>>>> CRC >>>>> checking can be used to check the integrity of the loading >>>>> bitstream >>>>> data against checksum from mkimage. It is off for the sake of >>>>> performance. >>>> Is there a measurable performance degradation ? I presume that's >>>> because >>>> caches are disabled at that point, yes? Enable caches and see if >>>> that >>>> helps. >>> Just logical sense, performance sure getting degraded, especially >>> loading full rbf, but may be not that obvious for periph.rbf >>> because of >>> very small size, i can try to measure. If not much difference, i >>> can >>> enable checking in default. >> Hard numbers are the only relevant argument here, please measure. >> And try it with caches enabled as much as possible, you want users to >> boot fast. Arria10 is particularly annoyingly slow at booting. > sure. >> >>> >>>> >>>> >>>>> >>>>> >>>>>> >>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> + bool "Enable CRC cheking on Arria10 FPGA bistream" >>>>>>> + depends on FPGA_SOCFPGA >>>>>>> + help >>>>>>> + Say Y here to enable the CRC checking on Arria 10 >>>>>>> FPGA >>>>>>> bitstream >>>>>>> + >>>>>>> + This provides CRC checking to ensure integrated >>>>>>> of >>>>>>> Arria >>>>>>> 10 FPGA >>>>>>> + bitstream is programmed into FPGA. >>>>>>> + >>>>>>> config FPGA_CYCLON2 >>>>>>> bool "Enable Altera FPGA driver for Cyclone II" >>>>>>> depends on FPGA_ALTERA >>>>>>> diff --git a/drivers/fpga/socfpga_arria10.c >>>>>>> b/drivers/fpga/socfpga_arria10.c >>>>>>> index 114dd91..d9ad237 100644 >>>>>>> --- a/drivers/fpga/socfpga_arria10.c >>>>>>> +++ b/drivers/fpga/socfpga_arria10.c >>>>>>> @@ -1,6 +1,6 @@ >>>>>>> // SPDX-License-Identifier: GPL-2.0 >>>>>>> /* >>>>>>> - * Copyright (C) 2017 Intel Corporation <www.intel.com> >>>>>>> + * Copyright (C) 2017-2018 Intel Corporation <www.intel.co >>>>>>> m> >>>>>>> */ >>>>>>> >>>>>>> #include <asm/io.h> >>>>>>> @@ -10,8 +10,10 @@ >>>>>>> #include <asm/arch/sdram.h> >>>>>>> #include <asm/arch/misc.h> >>>>>>> #include <altera.h> >>>>>>> +#include <asm/arch/pinmux.h> >>>>>>> #include <common.h> >>>>>>> #include <errno.h> >>>>>>> +#include <fs_loader.h> >>>>>>> #include <wait_bit.h> >>>>>>> #include <watchdog.h> >>>>>>> >>>>>>> @@ -21,6 +23,10 @@ >>>>>>> #define COMPRESSION_OFFSET 229 >>>>>>> #define FPGA_TIMEOUT_MSEC 1000 /* timeout in ms */ >>>>>>> #define FPGA_TIMEOUT_CNT 0x1000000 >>>>>>> +#define RBF_UNENCRYPTED 0xa65c >>>>>>> +#define RBF_ENCRYPTED 0xa65d >>>>>>> +#define ARRIA10RBF_PERIPH 0x0001 >>>>>>> +#define ARRIA10RBF_CORE 0x8001 >>>>>> This looks awfully similar to the PERIPH_RBF and CORE_RBF >>>>>> above. >>>>> PERIPH_RBF and CORE_RBF are the flags, so i can change them to >>>>> enum. >>>>> But above #define are magic content used to identify the >>>>> bistream >>>>> type. >>>>> If above #define are not suitable, what can you suggest? >>>> Maybe you can just align those two to avoid duplication ? >>> What's you means with duplication, they are different thing. >>> How about i change the name to ARRIA10RBF_PERIPH_TYPE >>> and ARRIA10RBF_CORE_TYPE. >> ARRIA10RBF_PERIPH = (PERIPH_RBF << 15) | 1 > We can't use the flag PERIPH_RBF(similar TRUE/FALSE) for magic content, > because they are not related each other. Magic content is defined by HW > design.
You can define the flags to match the HW design, which is probably a good idea ? > We identify the type of rbf such as periph, and core through this magic > content within the rbf. After we getting the type, then only we setting > the flag such as PERIPH_RBF to the function. >> >> same for ... _CORE ... is that a coincidence ? >> >> [...] -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
