On 11/28/2018 03:53 PM, Chee, Tien Fong wrote: > On Tue, 2018-11-27 at 13:08 +0100, Marek Vasut wrote: >> 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.inte >>>>>>>>> l.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 ? > I have no strong opinion of this, i can do it.
If it can deduplicate things, that'd be good. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot