[U-Boot] [PATCH v6] socfpga: Adding Scan Manager driver
Scan Manager driver will be called to configure the IOCSR scan chain. This configuration will setup the IO buffer settings
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@altera.com Cc: Wolfgang Denk wd@denx.de CC: Pavel Machek pavel@denx.de Cc: Tom Rini trini@ti.com Cc: Albert Aribaud albert.u.boot@aribaud.net --- Changes for v6 - Fixed various coding style issue Changes for v5 - Removal of additional blank line - Added comment for magic number Changes for v4 - avoid code duplication by add goto error - include underscore to variables name Changes for v3 - merge the handoff file and driver into single patch Changes for v2 - rebase with latest v2014.01-rc1 --- arch/arm/cpu/armv7/socfpga/Makefile | 2 +- arch/arm/cpu/armv7/socfpga/scan_manager.c | 211 +++++++ arch/arm/cpu/armv7/socfpga/spl.c | 4 + arch/arm/include/asm/arch-socfpga/scan_manager.h | 96 +++ .../include/asm/arch-socfpga/socfpga_base_addrs.h | 1 + board/altera/socfpga/iocsr_config.c | 657 ++++++++++++++++++++ board/altera/socfpga/iocsr_config.h | 17 + include/configs/socfpga_cyclone5.h | 1 + 8 files changed, 988 insertions(+), 1 deletion(-) create mode 100644 arch/arm/cpu/armv7/socfpga/scan_manager.c create mode 100644 arch/arm/include/asm/arch-socfpga/scan_manager.h create mode 100644 board/altera/socfpga/iocsr_config.c create mode 100644 board/altera/socfpga/iocsr_config.h
diff --git a/arch/arm/cpu/armv7/socfpga/Makefile b/arch/arm/cpu/armv7/socfpga/Makefile index 3e84a0c..4edc5d4 100644 --- a/arch/arm/cpu/armv7/socfpga/Makefile +++ b/arch/arm/cpu/armv7/socfpga/Makefile @@ -9,4 +9,4 @@
obj-y := lowlevel_init.o obj-y += misc.o timer.o reset_manager.o system_manager.o -obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o +obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o scan_manager.o diff --git a/arch/arm/cpu/armv7/socfpga/scan_manager.c b/arch/arm/cpu/armv7/socfpga/scan_manager.c new file mode 100644 index 0000000..a79b57b --- /dev/null +++ b/arch/arm/cpu/armv7/socfpga/scan_manager.c @@ -0,0 +1,211 @@ +/* + * Copyright (C) 2013 Altera Corporation <www.altera.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <asm/io.h> +#include <asm/arch/freeze_controller.h> +#include <asm/arch/scan_manager.h> + +DECLARE_GLOBAL_DATA_PTR; + +static const struct socfpga_scan_manager *scan_manager_base = + (void *)(SOCFPGA_SCANMGR_ADDRESS); +static const struct socfpga_freeze_controller *freeze_controller_base = + (void *)(SOCFPGA_SYSMGR_ADDRESS + SYSMGR_FRZCTRL_ADDRESS); + +/* + * Function to check IO scan chain engine status and wait if the engine is + * is active. Poll the IO scan chain engine till maximum iteration reached. + */ +static inline uint32_t scan_chain_engine_is_idle(uint32_t max_iter) +{ + uint32_t scanmgr_status; + + scanmgr_status = readl(&scan_manager_base->stat); + + /* Poll the engine until the scan engine is inactive */ + while (SCANMGR_STAT_ACTIVE_GET(scanmgr_status) || + (SCANMGR_STAT_WFIFOCNT_GET(scanmgr_status) > 0)) { + max_iter--; + if (max_iter > 0) + scanmgr_status = readl(&scan_manager_base->stat); + else + return SCAN_MGR_STATUS_ACTIVE; + } + return SCAN_MGR_STATUS_IDLE; +} + +/* Program HPS IO Scan Chain */ +uint32_t scan_mgr_io_scan_chain_prg( + uint32_t io_scan_chain_id, + uint32_t io_scan_chain_len_in_bits, + const uint32_t *iocsr_scan_chain) +{ + uint16_t tdi_tdo_header; + uint32_t io_program_iter; + uint32_t io_scan_chain_data_residual; + uint32_t residual; + uint32_t i; + uint32_t index = 0; + + /* + * De-assert reinit if the IO scan chain is intended for HIO. In + * this, its the chain 3. + */ + if (io_scan_chain_id == 3) + clrbits_le32(&freeze_controller_base->hioctrl, + SYSMGR_FRZCTRL_HIOCTRL_DLLRST_MASK); + + /* + * Check if the scan chain engine is inactive and the + * WFIFO is empty before enabling the IO scan chain + */ + if (scan_chain_engine_is_idle(SCAN_MAX_DELAY) != SCAN_MGR_STATUS_IDLE) + return 1; + + /* + * Enable IO Scan chain based on scan chain id + * Note: only one chain can be enabled at a time + */ + setbits_le32(&scan_manager_base->en, 1 << io_scan_chain_id); + + /* + * Calculate number of iteration needed for full 128-bit (4 x32-bits) + * bits shifting. Each TDI_TDO packet can shift in maximum 128-bits + */ + io_program_iter = io_scan_chain_len_in_bits >> + IO_SCAN_CHAIN_128BIT_SHIFT; + io_scan_chain_data_residual = io_scan_chain_len_in_bits & + IO_SCAN_CHAIN_128BIT_MASK; + + /* Construct TDI_TDO packet for 128-bit IO scan chain (2 bytes) */ + tdi_tdo_header = TDI_TDO_HEADER_FIRST_BYTE | + (TDI_TDO_MAX_PAYLOAD << TDI_TDO_HEADER_SECOND_BYTE_SHIFT); + + /* Program IO scan chain in 128-bit iteration */ + for (i = 0; i < io_program_iter; i++) { + /* write TDI_TDO packet header to scan manager */ + writel(tdi_tdo_header, &scan_manager_base->fifo_double_byte); + + /* calculate array index. Multiply by 4 as write 4 x 32bits */ + index = i * 4; + + /* write 4 successive 32-bit IO scan chain data into WFIFO */ + writel(iocsr_scan_chain[index], + &scan_manager_base->fifo_quad_byte); + writel(iocsr_scan_chain[index + 1], + &scan_manager_base->fifo_quad_byte); + writel(iocsr_scan_chain[index + 2], + &scan_manager_base->fifo_quad_byte); + writel(iocsr_scan_chain[index + 3], + &scan_manager_base->fifo_quad_byte); + + /* + * Check if the scan chain engine has completed the + * IO scan chain data shifting + */ + if (SCAN_MGR_STATUS_IDLE != scan_chain_engine_is_idle( + SCAN_MAX_DELAY)) + goto error; + } + + /* Calculate array index for final TDI_TDO packet */ + index = io_program_iter * 4; + + /* Final TDI_TDO packet if any */ + if (io_scan_chain_data_residual) { + /* + * Calculate number of quad bytes FIFO write + * needed for the final TDI_TDO packet + */ + io_program_iter = io_scan_chain_data_residual >> + IO_SCAN_CHAIN_32BIT_SHIFT; + + /* + * Construct TDI_TDO packet for remaining IO + * scan chain (2 bytes) + */ + tdi_tdo_header = TDI_TDO_HEADER_FIRST_BYTE | + ((io_scan_chain_data_residual - 1) << + TDI_TDO_HEADER_SECOND_BYTE_SHIFT); + + /* + * Program the last part of IO scan chain write TDI_TDO packet + * header (2 bytes) to scan manager + */ + writel(tdi_tdo_header, &scan_manager_base->fifo_double_byte); + + for (i = 0; i < io_program_iter; i++) { + /* + * write remaining scan chain data into scan + * manager WFIFO with 4 bytes write + */ + writel(iocsr_scan_chain[index + i], + &scan_manager_base->fifo_quad_byte); + } + + index += io_program_iter; + residual = io_scan_chain_data_residual & + IO_SCAN_CHAIN_32BIT_MASK; + + if (IO_SCAN_CHAIN_PAYLOAD_24BIT < residual) { + /* + * write the last 4B scan chain data + * into scan manager WFIFO + */ + writel(iocsr_scan_chain[index], + &scan_manager_base->fifo_quad_byte); + } else { + /* + * write the remaining 1 - 3 bytes scan chain + * data into scan manager WFIFO byte by byte + * to prevent JTAG engine shifting unused data + * from the FIFO and mistaken the data as a + * valid command (even though unused bits are + * set to 0, but just to prevent hardware + * glitch) + */ + for (i = 0; i < residual; i += 8) { + writel(((iocsr_scan_chain[index] >> i) + & IO_SCAN_CHAIN_BYTE_MASK), + &scan_manager_base->fifo_single_byte); + } + } + + /* + * Check if the scan chain engine has completed the + * IO scan chain data shifting + */ + if (SCAN_MGR_STATUS_IDLE != scan_chain_engine_is_idle( + SCAN_MAX_DELAY)) + goto error; + } + + /* Disable IO Scan chain when configuration done*/ + clrbits_le32(&scan_manager_base->en, 1 << io_scan_chain_id); + return 0; + +error: + /* Disable IO Scan chain when error detected */ + clrbits_le32(&scan_manager_base->en, 1 << io_scan_chain_id); + return 1; +} + +int scan_mgr_configure_iocsr(void) +{ + int status = 0; + + /* configure the IOCSR through scan chain */ + status |= scan_mgr_io_scan_chain_prg(0, + CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH, iocsr_scan_chain0_table); + status |= scan_mgr_io_scan_chain_prg(1, + CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH, iocsr_scan_chain1_table); + status |= scan_mgr_io_scan_chain_prg(2, + CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH, iocsr_scan_chain2_table); + status |= scan_mgr_io_scan_chain_prg(3, + CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH, iocsr_scan_chain3_table); + return status; +} diff --git a/arch/arm/cpu/armv7/socfpga/spl.c b/arch/arm/cpu/armv7/socfpga/spl.c index 36a00c3..8a49a1a 100644 --- a/arch/arm/cpu/armv7/socfpga/spl.c +++ b/arch/arm/cpu/armv7/socfpga/spl.c @@ -32,6 +32,10 @@ void spl_board_init(void) /* freeze all IO banks */ sys_mgr_frzctrl_freeze_req();
+ /* configure the IOCSR / IO buffer settings */ + if (scan_mgr_configure_iocsr()) + hang(); + /* configure the pin muxing through system manager */ sysmgr_pinmux_init(); #endif /* CONFIG_SOCFPGA_VIRTUAL_TARGET */ diff --git a/arch/arm/include/asm/arch-socfpga/scan_manager.h b/arch/arm/include/asm/arch-socfpga/scan_manager.h new file mode 100644 index 0000000..101355e --- /dev/null +++ b/arch/arm/include/asm/arch-socfpga/scan_manager.h @@ -0,0 +1,96 @@ +/* + * Copyright (C) 2013 Altera Corporation <www.altera.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#ifndef _SCAN_MANAGER_H_ +#define _SCAN_MANAGER_H_ + +struct socfpga_scan_manager { + u32 stat; + u32 en; + u32 padding[2]; + u32 fifo_single_byte; + u32 fifo_double_byte; + u32 fifo_quad_byte; +}; + +/* + * Shift count to get number of IO scan chain data in granularity + * of 128-bit ( N / 128 ) + */ +#define IO_SCAN_CHAIN_128BIT_SHIFT 7 + +/* + * Mask to get residual IO scan chain data in + * granularity of 128-bit ( N mod 128 ) + */ +#define IO_SCAN_CHAIN_128BIT_MASK 0x7F + +/* + * Shift count to get number of IO scan chain + * data in granularity of 32-bit ( N / 32 ) + */ +#define IO_SCAN_CHAIN_32BIT_SHIFT 5 + +/* + * Mask to get residual IO scan chain data in + * granularity of 32-bit ( N mod 32 ) + */ +#define IO_SCAN_CHAIN_32BIT_MASK 0x1F + +/* Byte mask */ +#define IO_SCAN_CHAIN_BYTE_MASK 0xFF + +/* 24-bits (3 bytes) IO scan chain payload definition */ +#define IO_SCAN_CHAIN_PAYLOAD_24BIT 24 + +/* + * Maximum length of TDI_TDO packet payload is 128 bits, + * represented by (length - 1) in TDI_TDO header + */ +#define TDI_TDO_MAX_PAYLOAD 127 + +/* TDI_TDO packet header for IO scan chain program */ +#define TDI_TDO_HEADER_FIRST_BYTE 0x80 + +/* Position of second command byte for TDI_TDO packet */ +#define TDI_TDO_HEADER_SECOND_BYTE_SHIFT 8 + +/* IO scan chain engine is idle */ +#define SCAN_MGR_STATUS_IDLE 0 + +/* IO scan chain engine is active */ +#define SCAN_MGR_STATUS_ACTIVE 1 + +/* + * Maximum polling loop to wait for IO scan chain engine + * becomes idle to prevent infinite loop + */ +#define SCAN_MAX_DELAY 100 + +#define SCANMGR_STAT_ACTIVE_GET(x) (((x) & 0x80000000) >> 31) +#define SCANMGR_STAT_WFIFOCNT_GET(x) (((x) & 0x70000000) >> 28) + +/* + * Program HPS IO Scan Chain + * io_scan_chain_id - IO scan chain ID + * io_scan_chain_len_in_bits - IO scan chain length in bits + * iocsr_scan_chain - IO scan chain table + */ +uint32_t scan_mgr_io_scan_chain_prg( + uint32_t io_scan_chain_id, + uint32_t io_scan_chain_len_in_bits, + const uint32_t *iocsr_scan_chain); + +extern const uint32_t iocsr_scan_chain0_table[ + ((CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH / 32) + 1)]; +extern const uint32_t iocsr_scan_chain1_table[ + ((CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH / 32) + 1)]; +extern const uint32_t iocsr_scan_chain2_table[ + ((CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH / 32) + 1)]; +extern const uint32_t iocsr_scan_chain3_table[ + ((CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH / 32) + 1)]; + +#endif /* _SCAN_MANAGER_H_ */ diff --git a/arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h b/arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h index 50c4ebd..8d329cf 100644 --- a/arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h +++ b/arch/arm/include/asm/arch-socfpga/socfpga_base_addrs.h @@ -13,5 +13,6 @@ #define SOCFPGA_OSC1TIMER0_ADDRESS 0xffd00000 #define SOCFPGA_RSTMGR_ADDRESS 0xffd05000 #define SOCFPGA_SYSMGR_ADDRESS 0xffd08000 +#define SOCFPGA_SCANMGR_ADDRESS 0xfff02000
#endif /* _SOCFPGA_BASE_ADDRS_H_ */ diff --git a/board/altera/socfpga/iocsr_config.c b/board/altera/socfpga/iocsr_config.c new file mode 100644 index 0000000..b4b5ff8 --- /dev/null +++ b/board/altera/socfpga/iocsr_config.c @@ -0,0 +1,657 @@ +/* + * Copyright Altera Corporation (C) 2012-2014. All rights reserved + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/* This file is generated by Preloader Generator */ + +#include <iocsr_config.h> + +const unsigned long iocsr_scan_chain0_table[(( + CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH / 32) + 1)] = { + 0x00000000, + 0x00000000, + 0x0FF00000, + 0xC0000000, + 0x0000003F, + 0x00008000, + 0x00020080, + 0x08020000, + 0x08000000, + 0x00018020, + 0x00000000, + 0x00004000, + 0x00010040, + 0x04010000, + 0x04000000, + 0x00000010, + 0x00004010, + 0x00002000, + 0x00020000, + 0x02008000, + 0x02000000, + 0x00000008, + 0x00002008, + 0x00001000, +}; + +const unsigned long iocsr_scan_chain1_table[(( + CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH / 32) + 1)] = { + 0x000C0300, + 0x10040000, + 0x100000C0, + 0x00000040, + 0x00010040, + 0x00008000, + 0x00080000, + 0x18060000, + 0x18000000, + 0x00000060, + 0x00018060, + 0x00004000, + 0x00010040, + 0x10000000, + 0x04000000, + 0x00000010, + 0x00004010, + 0x00002000, + 0x06008020, + 0x02008000, + 0x01FE0000, + 0xF8000000, + 0x00000007, + 0x00001000, + 0x00004010, + 0x01004000, + 0x01000000, + 0x00003004, + 0x00001004, + 0x00000800, + 0x00000000, + 0x00000000, + 0x00800000, + 0x00000002, + 0x00002000, + 0x00000400, + 0x00000000, + 0x00401000, + 0x00000003, + 0x00000000, + 0x00000000, + 0x00000200, + 0x00600802, + 0x00000000, + 0x80200000, + 0x80000600, + 0x00000200, + 0x00000100, + 0x00300401, + 0xC0100400, + 0x40100000, + 0x40000300, + 0x000C0100, + 0x00000080, +}; + +const unsigned long iocsr_scan_chain2_table[(( + CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH / 32) + 1)] = { + 0x80040100, + 0x00000000, + 0x0FF00000, + 0x00000000, + 0x0C010040, + 0x00008000, + 0x18020080, + 0x00000000, + 0x08000000, + 0x00040020, + 0x06018060, + 0x00004000, + 0x0C010040, + 0x04010000, + 0x00000030, + 0x00000000, + 0x03004010, + 0x00002000, + 0x06008020, + 0x02008000, + 0x02000018, + 0x00006008, + 0x01802008, + 0x00001000, + 0x03004010, + 0x01004000, + 0x0100000C, + 0x00003004, + 0x00C01004, + 0x00000800, +}; + +const unsigned long iocsr_scan_chain3_table[(( + CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH / 32) + 1)] = { + 0x2C420D80, + 0x082000FF, + 0x0A804001, + 0x07900000, + 0x08020000, + 0x00100000, + 0x0A800000, + 0x07900000, + 0x08020000, + 0x00100000, + 0xC8800000, + 0x00003001, + 0x00C00722, + 0x00000000, + 0x00000021, + 0x82000004, + 0x05400000, + 0x03C80000, + 0x04010000, + 0x00080000, + 0x05400000, + 0x03C80000, + 0x05400000, + 0x03C80000, + 0xE4400000, + 0x00001800, + 0x00600391, + 0x800E4400, + 0x00000001, + 0x40000002, + 0x02A00000, + 0x01E40000, + 0x02A00000, + 0x01E40000, + 0x02A00000, + 0x01E40000, + 0x02A00000, + 0x01E40000, + 0x72200000, + 0x80000C00, + 0x003001C8, + 0xC0072200, + 0x1C880000, + 0x20000300, + 0x00040000, + 0x50670000, + 0x00000070, + 0x24590000, + 0x00001000, + 0xA0000034, + 0x0D000001, + 0x906808A2, + 0xA2834024, + 0x05141A00, + 0x808A20D0, + 0x34024906, + 0x01A00A28, + 0xA20D0000, + 0x24906808, + 0x00A28340, + 0xD000001A, + 0x06808A20, + 0x10040000, + 0x00200000, + 0x10040000, + 0x00200000, + 0x15000000, + 0x0F200000, + 0x15000000, + 0x0F200000, + 0x01FE0000, + 0x00000000, + 0x01800E44, + 0x00391000, + 0x007F8006, + 0x00000000, + 0x0A800001, + 0x07900000, + 0x0A800000, + 0x07900000, + 0x0A800000, + 0x07900000, + 0x08020000, + 0x00100000, + 0xC8800000, + 0x00003001, + 0x00C00722, + 0x00000FF0, + 0x72200000, + 0x80000C00, + 0x05400000, + 0x02480000, + 0x04000000, + 0x00080000, + 0x05400000, + 0x03C80000, + 0x05400000, + 0x03C80000, + 0x6A1C0000, + 0x00001800, + 0x00600391, + 0x800E4400, + 0x1A870001, + 0x40000600, + 0x02A00040, + 0x01E40000, + 0x02A00000, + 0x01E40000, + 0x02A00000, + 0x01E40000, + 0x02A00000, + 0x01E40000, + 0x72200000, + 0x80000C00, + 0x003001C8, + 0xC0072200, + 0x1C880000, + 0x20000300, + 0x00040000, + 0x50670000, + 0x00000070, + 0x24590000, + 0x00001000, + 0xA0000034, + 0x0D000001, + 0x906808A2, + 0xA2834024, + 0x05141A00, + 0x808A20D0, + 0x34024906, + 0x01A00040, + 0xA20D0002, + 0x24906808, + 0x00A28340, + 0xD005141A, + 0x06808A20, + 0x10040000, + 0x00200000, + 0x10040000, + 0x00200000, + 0x15000000, + 0x0F200000, + 0x15000000, + 0x0F200000, + 0x01FE0000, + 0x00000000, + 0x01800E44, + 0x00391000, + 0x007F8006, + 0x00000000, + 0x99300001, + 0x34343400, + 0xAA0D4000, + 0x01C3A810, + 0xAA0D4000, + 0x01C3A808, + 0xAA0D4000, + 0x01C3A810, + 0x00040100, + 0x00000800, + 0x00000000, + 0x00001208, + 0x00482000, + 0x000001C1, + 0x00000000, + 0x00410482, + 0x0006A000, + 0x0001B400, + 0x00020000, + 0x00000400, + 0x0002A000, + 0x0001E400, + 0x5506A000, + 0x00E1D404, + 0x00000000, + 0xC880090C, + 0x00003001, + 0x90400000, + 0x00000000, + 0x2020C243, + 0x2A835000, + 0x0070EA04, + 0x2A835000, + 0x0070EA02, + 0x2A835000, + 0x0070EA04, + 0x00010040, + 0x00000200, + 0x00000000, + 0x00000482, + 0x00120800, + 0x00002000, + 0x80000000, + 0x00104120, + 0x00000200, + 0xAC255F80, + 0xF1C71C71, + 0x14F3690D, + 0x1A041414, + 0x00D00000, + 0x14864000, + 0x59647A05, + 0xBA28A3D8, + 0xF511451E, + 0x0341D348, + 0x821A0000, + 0x0000D000, + 0x04510680, + 0xD859647A, + 0x1EBA28A3, + 0x48F51145, + 0x000341D3, + 0x00080200, + 0x00001000, + 0x00080200, + 0x00001000, + 0x000A8000, + 0x00075000, + 0x541A8000, + 0x03875011, + 0x10000000, + 0x00000000, + 0x0080C000, + 0x41000000, + 0x00003FC2, + 0x00820000, + 0xAA0D4000, + 0x01C3A810, + 0xAA0D4000, + 0x01C3A808, + 0xAA0D4000, + 0x01C3A810, + 0x00040100, + 0x00000800, + 0x00000000, + 0x00001208, + 0x00482000, + 0x00008000, + 0x00000000, + 0x00410482, + 0x0006A000, + 0x0001B400, + 0x00020000, + 0x00000400, + 0x00020080, + 0x00000400, + 0x5506A000, + 0x00E1D404, + 0x00000000, + 0x0000090C, + 0x00000010, + 0x90400000, + 0x00000000, + 0x2020C243, + 0x2A835000, + 0x0070EA04, + 0x2A835000, + 0x0070EA02, + 0x2A835000, + 0x0070EA04, + 0x00015000, + 0x0000F200, + 0x00000000, + 0x00000482, + 0x00120800, + 0x00600391, + 0x80000000, + 0x00104120, + 0x00000200, + 0xAC255F80, + 0xF1C71C71, + 0x14F3690D, + 0x1A041414, + 0x00D00000, + 0x14864000, + 0x59647A05, + 0xBA28A3D8, + 0xF511451E, + 0x8341D348, + 0x821A0124, + 0x0000D000, + 0x00000680, + 0xD859647A, + 0x1EBA28A3, + 0x48F51145, + 0x000341D3, + 0x00080200, + 0x00001000, + 0x00080200, + 0x00001000, + 0x000A8000, + 0x00075000, + 0x541A8000, + 0x03875011, + 0x10000000, + 0x00000000, + 0x0080C000, + 0x41000000, + 0x04000002, + 0x00820000, + 0xAA0D4000, + 0x01C3A810, + 0xAA0D4000, + 0x01C3A808, + 0xAA0D4000, + 0x01C3A810, + 0x00040100, + 0x00000800, + 0x00000000, + 0x00001208, + 0x00482000, + 0x00008000, + 0x00000000, + 0x00410482, + 0x0006A000, + 0x0001B400, + 0x00020000, + 0x00000400, + 0x0002A000, + 0x0001E400, + 0x5506A000, + 0x00E1D404, + 0x00000000, + 0xC880090C, + 0x00003001, + 0x90400000, + 0x00000000, + 0x2020C243, + 0x2A835000, + 0x0070EA04, + 0x2A835000, + 0x0070EA02, + 0x2A835000, + 0x0070EA04, + 0x00010040, + 0x00000200, + 0x00000000, + 0x00000482, + 0x00120800, + 0x00002000, + 0x80000000, + 0x00104120, + 0x00000200, + 0xAC255F80, + 0xF1C71C71, + 0x14F3690D, + 0x1A041414, + 0x00D00000, + 0x14864000, + 0x59647A05, + 0xBA28A3D8, + 0xF511451E, + 0x0341D348, + 0x821A0000, + 0x0000D000, + 0x00000680, + 0xD859647A, + 0x1EBA28A3, + 0x48F51145, + 0x000341D3, + 0x00080200, + 0x00001000, + 0x00080200, + 0x00001000, + 0x000A8000, + 0x00075000, + 0x541A8000, + 0x03875011, + 0x10000000, + 0x00000000, + 0x0080C000, + 0x41000000, + 0x04000002, + 0x00820000, + 0xAA0D4000, + 0x01C3A810, + 0xAA0D4000, + 0x01C3A808, + 0xAA0D4000, + 0x01C3A810, + 0x00040100, + 0x00000800, + 0x00000000, + 0x00001208, + 0x00482000, + 0x00008000, + 0x00000000, + 0x00410482, + 0x0006A000, + 0x0001B400, + 0x00020000, + 0x00000400, + 0x00020080, + 0x00000400, + 0x5506A000, + 0x00E1D404, + 0x00000000, + 0x0000090C, + 0x00000010, + 0x90400000, + 0x00000000, + 0x2020C243, + 0x2A835000, + 0x0070EA04, + 0x2A835000, + 0x0070EA02, + 0x2A835000, + 0x0070EA04, + 0x00010040, + 0x00000200, + 0x00000000, + 0x00000482, + 0x40120800, + 0x00000070, + 0x80000000, + 0x00104120, + 0x00000200, + 0xAC255F80, + 0xF1C71C71, + 0x14F1690D, + 0x1A041414, + 0x00D00000, + 0x14864000, + 0x59647A05, + 0xBA28A3D8, + 0xF511451E, + 0x0341D348, + 0x821A0000, + 0x0000D000, + 0x00000680, + 0xD859647A, + 0x1EBA28A3, + 0x48F51145, + 0x000341D3, + 0x00080200, + 0x00001000, + 0x00080200, + 0x00001000, + 0x000A8000, + 0x00075000, + 0x541A8000, + 0x03875011, + 0x10000000, + 0x00000000, + 0x0080C000, + 0x41000000, + 0x04000002, + 0x00820000, + 0x00489800, + 0x001A1A1A, + 0x085506A0, + 0x0000E1D4, + 0x045506A0, + 0x0000E1D4, + 0x085506A0, + 0x8000E1D4, + 0x00000200, + 0x00000004, + 0x04000000, + 0x00000009, + 0x00002410, + 0x00000040, + 0x41000000, + 0x00002082, + 0x00000350, + 0x000000DA, + 0x00000100, + 0x40000002, + 0x00000100, + 0x00000002, + 0x022A8350, + 0x000070EA, + 0x86000000, + 0x08000004, + 0x00000000, + 0x00482000, + 0x21800000, + 0x00101061, + 0x021541A8, + 0x00003875, + 0x011541A8, + 0x00003875, + 0x021541A8, + 0x20003875, + 0x00000080, + 0x00000001, + 0x41000000, + 0x00000002, + 0x00FF0904, + 0x00000000, + 0x90400000, + 0x00000820, + 0xC0000001, + 0x38D612AF, + 0x86F8E38E, + 0x0A0A78B4, + 0x000D020A, + 0x00006800, + 0x028A4320, + 0xEC2CB23D, + 0x8F5D1451, + 0xA47A88A2, + 0x0001A0E9, + 0x00410D00, + 0x40000068, + 0x3D000003, + 0x51EC2CB2, + 0xA28F5D14, + 0xE9A47A88, + 0x000001A0, + 0x00000401, + 0x00000008, + 0x00000401, + 0x00000008, + 0x00000540, + 0x000003A8, + 0x08AA0D40, + 0x8001C3A8, + 0x0000007F, + 0x00000000, + 0x00004060, + 0xE1208000, + 0x0000001F, + 0x00004100, +}; diff --git a/board/altera/socfpga/iocsr_config.h b/board/altera/socfpga/iocsr_config.h new file mode 100644 index 0000000..490f109 --- /dev/null +++ b/board/altera/socfpga/iocsr_config.h @@ -0,0 +1,17 @@ +/* + * Copyright Altera Corporation (C) 2012-2014. All rights reserved + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/* This file is generated by Preloader Generator */ + +#ifndef _PRELOADER_IOCSR_CONFIG_H_ +#define _PRELOADER_IOCSR_CONFIG_H_ + +#define CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH (764) +#define CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH (1719) +#define CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH (955) +#define CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH (16766) + +#endif /*_PRELOADER_IOCSR_CONFIG_H_*/ diff --git a/include/configs/socfpga_cyclone5.h b/include/configs/socfpga_cyclone5.h index 608578a..3d553f7 100644 --- a/include/configs/socfpga_cyclone5.h +++ b/include/configs/socfpga_cyclone5.h @@ -8,6 +8,7 @@
#include <asm/arch/socfpga_base_addrs.h> #include "../../board/altera/socfpga/pinmux_config.h" +#include "../../board/altera/socfpga/iocsr_config.h"
/* * High level configuration
On 02/27/2014 05:03 PM, Chin Liang See wrote:
Scan Manager driver will be called to configure the IOCSR scan chain. This configuration will setup the IO buffer settings
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@altera.com Cc: Wolfgang Denk wd@denx.de CC: Pavel Machek pavel@denx.de Cc: Tom Rini trini@ti.com Cc: Albert Aribaud albert.u.boot@aribaud.net
Changes for v6
- Fixed various coding style issue
Changes for v5
- Removal of additional blank line
- Added comment for magic number
Changes for v4
- avoid code duplication by add goto error
- include underscore to variables name
Changes for v3
- merge the handoff file and driver into single patch
Changes for v2
- rebase with latest v2014.01-rc1
arch/arm/cpu/armv7/socfpga/Makefile | 2 +- arch/arm/cpu/armv7/socfpga/scan_manager.c | 211 +++++++ arch/arm/cpu/armv7/socfpga/spl.c | 4 + arch/arm/include/asm/arch-socfpga/scan_manager.h | 96 +++ .../include/asm/arch-socfpga/socfpga_base_addrs.h | 1 + board/altera/socfpga/iocsr_config.c | 657 ++++++++++++++++++++ board/altera/socfpga/iocsr_config.h | 17 +
I still have problem with content of these two files. In iocsr_config.c is ~600 lines which targets just one specific hardware design configuration. I can't see any reason why this should go to mainline and stay there. Because it brings no value.
I would recommend you just to define that arrays like this
const unsigned long iocsr_scan_chain0_table[]; const unsigned long iocsr_scan_chain0_table[]; ...
+ in header #define CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH 0
and write these 2 files by hand. Then your users will just replace them by hand for specific board/design.
Thanks, Michal
Hi Michal,
On Fri, 2014-02-28 at 11:17 +0100, Michal Simek wrote:
On 02/27/2014 05:03 PM, Chin Liang See wrote:
Scan Manager driver will be called to configure the IOCSR scan chain. This configuration will setup the IO buffer settings
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@altera.com Cc: Wolfgang Denk wd@denx.de CC: Pavel Machek pavel@denx.de Cc: Tom Rini trini@ti.com Cc: Albert Aribaud albert.u.boot@aribaud.net
Changes for v6
- Fixed various coding style issue
Changes for v5
- Removal of additional blank line
- Added comment for magic number
Changes for v4
- avoid code duplication by add goto error
- include underscore to variables name
Changes for v3
- merge the handoff file and driver into single patch
Changes for v2
- rebase with latest v2014.01-rc1
arch/arm/cpu/armv7/socfpga/Makefile | 2 +- arch/arm/cpu/armv7/socfpga/scan_manager.c | 211 +++++++ arch/arm/cpu/armv7/socfpga/spl.c | 4 + arch/arm/include/asm/arch-socfpga/scan_manager.h | 96 +++ .../include/asm/arch-socfpga/socfpga_base_addrs.h | 1 + board/altera/socfpga/iocsr_config.c | 657 ++++++++++++++++++++ board/altera/socfpga/iocsr_config.h | 17 +
I still have problem with content of these two files. In iocsr_config.c is ~600 lines which targets just one specific hardware design configuration. I can't see any reason why this should go to mainline and stay there. Because it brings no value.
I would recommend you just to define that arrays like this
const unsigned long iocsr_scan_chain0_table[]; const unsigned long iocsr_scan_chain0_table[]; ...
- in header
#define CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH 0
and write these 2 files by hand. Then your users will just replace them by hand for specific board/design.
Actually the intention is that user can pull the code from git and build it. We want to avoid any tools dependency here.
At same time, these files are located inside board folders. If user have different boards, they will have new set of folders here their own handoff files. From there, there won't the need to regenerate everytime.
Thanks Chin Liang
Thanks, Michal
On 03/04/2014 04:01 PM, Chin Liang See wrote:
Hi Michal,
On Fri, 2014-02-28 at 11:17 +0100, Michal Simek wrote:
On 02/27/2014 05:03 PM, Chin Liang See wrote:
Scan Manager driver will be called to configure the IOCSR scan chain. This configuration will setup the IO buffer settings
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@altera.com Cc: Wolfgang Denk wd@denx.de CC: Pavel Machek pavel@denx.de Cc: Tom Rini trini@ti.com Cc: Albert Aribaud albert.u.boot@aribaud.net
Changes for v6
- Fixed various coding style issue
Changes for v5
- Removal of additional blank line
- Added comment for magic number
Changes for v4
- avoid code duplication by add goto error
- include underscore to variables name
Changes for v3
- merge the handoff file and driver into single patch
Changes for v2
- rebase with latest v2014.01-rc1
arch/arm/cpu/armv7/socfpga/Makefile | 2 +- arch/arm/cpu/armv7/socfpga/scan_manager.c | 211 +++++++ arch/arm/cpu/armv7/socfpga/spl.c | 4 + arch/arm/include/asm/arch-socfpga/scan_manager.h | 96 +++ .../include/asm/arch-socfpga/socfpga_base_addrs.h | 1 + board/altera/socfpga/iocsr_config.c | 657 ++++++++++++++++++++ board/altera/socfpga/iocsr_config.h | 17 +
I still have problem with content of these two files. In iocsr_config.c is ~600 lines which targets just one specific hardware design configuration. I can't see any reason why this should go to mainline and stay there. Because it brings no value.
I would recommend you just to define that arrays like this
const unsigned long iocsr_scan_chain0_table[]; const unsigned long iocsr_scan_chain0_table[]; ...
- in header
#define CONFIG_HPS_IOCSR_SCANCHAIN0_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN1_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN2_LENGTH 0 #define CONFIG_HPS_IOCSR_SCANCHAIN3_LENGTH 0
and write these 2 files by hand. Then your users will just replace them by hand for specific board/design.
Actually the intention is that user can pull the code from git and build it. We want to avoid any tools dependency here.
There is tool dependency even you don't want to have it. These files are autogenerated by tools it means you already have dependency.
Also I expect that you can change all pins for uarts/ethernets/spi/i2c/etc that's why there is no golden configuration for socfpga that's why it is better to keep it empty just to compile it.
At same time, these files are located inside board folders. If user have different boards, they will have new set of folders here their own handoff files. From there, there won't the need to regenerate everytime.
Please explain me one thing how many users will use this configuration? Especially these ~600 lines? I also expect that you can generate unlimited number of configuration for the same board. It means this is just one configuration for one board which you will use till the time when you find out a bug in your tools and you will regenerate this.
Also I expect that socfpga_cyclone is family that's why you are also missing connection to the real board that's why every socfpga user will have to open tool and generate this configuration for the board (Or can download it from web or get with the board).
It is the same situation which we have with zynq that's why I didn't add our ps7_init file because it is just useless for others.
Thanks, Michal
Hi!
Also I expect that you can change all pins for uarts/ethernets/spi/i2c/etc that's why there is no golden configuration for socfpga that's why it is better to keep it empty just to compile it.
Well, there are some development boards around... and having u-boot able to boot on the devel board by default would be very very useful.
At same time, these files are located inside board folders. If user have different boards, they will have new set of folders here their own handoff files. From there, there won't the need to regenerate everytime.
Please explain me one thing how many users will use this configuration? Especially these ~600 lines?
Pretty much everyone when they test-boot their system...? Pavel
Hi Pavel,
2014-03-05 17:13 GMT+01:00 Pavel Machek pavel@denx.de:
Hi!
Also I expect that you can change all pins for
uarts/ethernets/spi/i2c/etc
that's why there is no golden configuration for socfpga that's why it is better to keep it empty just to compile it.
Well, there are some development boards around... and having u-boot able to boot on the devel board by default would be very very useful.
I believe you but the point is somewhere else if you can reach it by universal target.
At same time, these files are located inside board folders. If user
have
different boards, they will have new set of folders here their own handoff files. From there, there won't the need to regenerate
everytime.
Please explain me one thing how many users will use this configuration? Especially these ~600 lines?
Pretty much everyone when they test-boot their system...?
Really? It means that all development boards/custom boards with socfpga use the same configuration which seems to me weird. If yes, then why these files are autogenerated if the same config can be use for all boards?
Thanks, Michal
On Wed, 2014-03-05 at 19:47 +0100, Michal Simek wrote:
Hi Pavel,
2014-03-05 17:13 GMT+01:00 Pavel Machek pavel@denx.de: Hi!
> Also I expect that you can change all pins for uarts/ethernets/spi/i2c/etc > that's why there is no golden configuration for socfpga that's why > it is better to keep it empty just to compile it. Well, there are some development boards around... and having u-boot able to boot on the devel board by default would be very very useful.I believe you but the point is somewhere else if you can reach it by universal
target.
> > At same time, these files are located inside board folders. If user have > > different boards, they will have new set of folders here their own > > handoff files. From there, there won't the need to regenerate everytime. > > Please explain me one thing how many users will use this configuration? > Especially these ~600 lines? Pretty much everyone when they test-boot their system...?Really? It means that all development boards/custom boards with socfpga
I believe you confuse with arch and board. These files are located inside board. For board/altera/socfpga, its referring to Altera dev kit. When another vendor, example like Xilinx, fabricated a board with SOCFPGA chip, then it will have another folder called board/xilinx/socfpga.
use the same configuration which seems to me weird.
If yes, then why these files are autogenerated if the same config can be use for all boards?
Probably an example will clear your doubt. When the board is fabricated, the pin mux used will be fixed. You won't need to change it until you fabricated a new board.
Hope this info helps. Thanks
Chin Liang
Thanks,
Michal
On 03/05/2014 08:00 PM, Chin Liang See wrote:
On Wed, 2014-03-05 at 19:47 +0100, Michal Simek wrote:
Hi Pavel,
2014-03-05 17:13 GMT+01:00 Pavel Machek pavel@denx.de: Hi!
> Also I expect that you can change all pins for uarts/ethernets/spi/i2c/etc > that's why there is no golden configuration for socfpga that's why > it is better to keep it empty just to compile it. Well, there are some development boards around... and having u-boot able to boot on the devel board by default would be very very useful.I believe you but the point is somewhere else if you can reach it by universal
target.
> > At same time, these files are located inside board folders. If user have > > different boards, they will have new set of folders here their own > > handoff files. From there, there won't the need to regenerate everytime. > > Please explain me one thing how many users will use this configuration? > Especially these ~600 lines? Pretty much everyone when they test-boot their system...?Really? It means that all development boards/custom boards with socfpga
I believe you confuse with arch and board. These files are located inside board. For board/altera/socfpga, its referring to Altera dev kit. When another vendor, example like Xilinx, fabricated a board with SOCFPGA chip, then it will have another folder called board/xilinx/socfpga.
That explains a lot. Why isn't call devkit then?
use the same configuration which seems to me weird.
If yes, then why these files are autogenerated if the same config can be use for all boards?
Probably an example will clear your doubt. When the board is fabricated, the pin mux used will be fixed. You won't need to change it until you fabricated a new board.
I understand that you want to have something which you can just compile and boot. But only pins are fixed not internal connection inside chip. Also I expect you have connectors on the board which you can use for add-on cards. My doubts are just based on my experience with zynq that not everything what it is generated by tools is really needed to get board boot. Also boards have just fixed pins but not internal connections to these pins. I believe that this is just the same for your dev kit. Also I hardly believe that you need all 600lines just to have something to boot.
Thanks, Michal
Hi Michal,
On Fri, 2014-03-07 at 09:12 +0100, Michal Simek wrote:
On 03/05/2014 08:00 PM, Chin Liang See wrote:
On Wed, 2014-03-05 at 19:47 +0100, Michal Simek wrote:
Hi Pavel,
2014-03-05 17:13 GMT+01:00 Pavel Machek pavel@denx.de: Hi!
> Also I expect that you can change all pins for uarts/ethernets/spi/i2c/etc > that's why there is no golden configuration for socfpga that's why > it is better to keep it empty just to compile it. Well, there are some development boards around... and having u-boot able to boot on the devel board by default would be very very useful.I believe you but the point is somewhere else if you can reach it by universal
target.
> > At same time, these files are located inside board folders. If user have > > different boards, they will have new set of folders here their own > > handoff files. From there, there won't the need to regenerate everytime. > > Please explain me one thing how many users will use this configuration? > Especially these ~600 lines? Pretty much everyone when they test-boot their system...?Really? It means that all development boards/custom boards with socfpga
I believe you confuse with arch and board. These files are located inside board. For board/altera/socfpga, its referring to Altera dev kit. When another vendor, example like Xilinx, fabricated a board with SOCFPGA chip, then it will have another folder called board/xilinx/socfpga.
That explains a lot. Why isn't call devkit then?
Examples always nice when I explaining and I love examples. While the naming, it would be interesting topic which same as giving baby's name.
use the same configuration which seems to me weird.
If yes, then why these files are autogenerated if the same config can be use for all boards?
Probably an example will clear your doubt. When the board is fabricated, the pin mux used will be fixed. You won't need to change it until you fabricated a new board.
I understand that you want to have something which you can just compile and boot. But only pins are fixed not internal connection inside chip. Also I expect you have connectors on the board which you can use for add-on cards. My doubts are just based on my experience with zynq that not everything what it is generated by tools is really needed to get board boot. Also boards have just fixed pins but not internal connections to these pins. I believe that this is just the same for your dev kit. Also I hardly believe that you need all 600lines just to have something to boot.
Both SOCFPGA and Zynq are unique due to its programmability. But in this case, all our configurations here would only support golden system reference design on Altera dev kit. This would make our lives much easier.
Thanks Chin Liang
Thanks, Michal
Hi!
Scan Manager driver will be called to configure the IOCSR scan chain. This configuration will setup the IO buffer settings
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@altera.com Cc: Wolfgang Denk wd@denx.de CC: Pavel Machek pavel@denx.de Cc: Tom Rini trini@ti.com Cc: Albert Aribaud albert.u.boot@aribaud.net
+static inline uint32_t scan_chain_engine_is_idle(uint32_t max_iter) +{
- uint32_t scanmgr_status;
- scanmgr_status = readl(&scan_manager_base->stat);
- /* Poll the engine until the scan engine is inactive */
- while (SCANMGR_STAT_ACTIVE_GET(scanmgr_status) ||
(SCANMGR_STAT_WFIFOCNT_GET(scanmgr_status) > 0)) {max_iter--;if (max_iter > 0)scanmgr_status = readl(&scan_manager_base->stat);elsereturn SCAN_MGR_STATUS_ACTIVE;- }
- return SCAN_MGR_STATUS_IDLE;
+}
The function is named _is_idle, but returns 1 if it is _active_. I'd get rid of SCAN_MGR_STATUS_* defines (they make it harder to read) and reverse the logic...
+/* Program HPS IO Scan Chain */ +uint32_t scan_mgr_io_scan_chain_prg(
- uint32_t io_scan_chain_id,
- uint32_t io_scan_chain_len_in_bits,
- const uint32_t *iocsr_scan_chain)
+{
...
- /*
* Check if the scan chain engine is inactive and the* WFIFO is empty before enabling the IO scan chain*/- if (scan_chain_engine_is_idle(SCAN_MAX_DELAY) != SCAN_MGR_STATUS_IDLE)
return 1;
Maybe it would be better to return <0 numbers on error?
Should the function be static? Then it could have shorter name...
- /* Disable IO Scan chain when configuration done*/
I'd put space before end of comment.
+/*
- Program HPS IO Scan Chain
- io_scan_chain_id - IO scan chain ID
- io_scan_chain_len_in_bits - IO scan chain length in bits
- iocsr_scan_chain - IO scan chain table
- */
+uint32_t scan_mgr_io_scan_chain_prg(
- uint32_t io_scan_chain_id,
- uint32_t io_scan_chain_len_in_bits,
- const uint32_t *iocsr_scan_chain);
...and you can probably drop the prototype...
Thanks, Pavel
Hi Pavel,
On Mon, 2014-03-03 at 21:41 +0100, ZY - pavel wrote:
Hi!
Scan Manager driver will be called to configure the IOCSR scan chain. This configuration will setup the IO buffer settings
Signed-off-by: Chin Liang See clsee@altera.com Cc: Dinh Nguyen dinguyen@altera.com Cc: Wolfgang Denk wd@denx.de CC: Pavel Machek pavel@denx.de Cc: Tom Rini trini@ti.com Cc: Albert Aribaud albert.u.boot@aribaud.net
+static inline uint32_t scan_chain_engine_is_idle(uint32_t max_iter) +{
- uint32_t scanmgr_status;
- scanmgr_status = readl(&scan_manager_base->stat);
- /* Poll the engine until the scan engine is inactive */
- while (SCANMGR_STAT_ACTIVE_GET(scanmgr_status) ||
(SCANMGR_STAT_WFIFOCNT_GET(scanmgr_status) > 0)) {max_iter--;if (max_iter > 0)scanmgr_status = readl(&scan_manager_base->stat);elsereturn SCAN_MGR_STATUS_ACTIVE;- }
- return SCAN_MGR_STATUS_IDLE;
+}
The function is named _is_idle, but returns 1 if it is _active_. I'd get rid of SCAN_MGR_STATUS_* defines (they make it harder to read) and reverse the logic...
This seems is controversial function :) Yup, I already fixed that by removed the status variable.
+/* Program HPS IO Scan Chain */ +uint32_t scan_mgr_io_scan_chain_prg(
- uint32_t io_scan_chain_id,
- uint32_t io_scan_chain_len_in_bits,
- const uint32_t *iocsr_scan_chain)
+{
...
- /*
* Check if the scan chain engine is inactive and the* WFIFO is empty before enabling the IO scan chain*/- if (scan_chain_engine_is_idle(SCAN_MAX_DELAY) != SCAN_MGR_STATUS_IDLE)
return 1;Maybe it would be better to return <0 numbers on error?
I changed that to if (!scan_chain_engine_is_idle(SCAN_MAX_DELAY)) It should much better now
Should the function be static? Then it could have shorter name...
Its already static and fit into single line now.
- /* Disable IO Scan chain when configuration done*/
I'd put space before end of comment.
+/*
- Program HPS IO Scan Chain
- io_scan_chain_id - IO scan chain ID
- io_scan_chain_len_in_bits - IO scan chain length in bits
- iocsr_scan_chain - IO scan chain table
- */
+uint32_t scan_mgr_io_scan_chain_prg(
- uint32_t io_scan_chain_id,
- uint32_t io_scan_chain_len_in_bits,
- const uint32_t *iocsr_scan_chain);
...and you can probably drop the prototype...
Removed. Thanks
Chin Liang
Thanks, Pavel
Hi!
I changed that to if (!scan_chain_engine_is_idle(SCAN_MAX_DELAY)) It should much better now
Yes, thanks.
Removed. Thanks
Thanks for all the changes, it looks better now, so you have my Acked-by in another thread. Pavel
On Wed, 2014-03-05 at 17:15 +0100, ZY - pavel wrote:
Hi!
I changed that to if (!scan_chain_engine_is_idle(SCAN_MAX_DELAY)) It should much better now
Yes, thanks.
Removed. Thanks
Thanks for all the changes, it looks better now, so you have my Acked-by in another thread.
Thanks for reviewing too!
Chin Liang
Pavel
participants (3)
-
Chin Liang See -
Michal Simek -
Pavel Machek