* [U-Boot] [PATCH v5] socfpga: Adding Scan Manager driver
@ 2014-02-21 22:46 Chin Liang See
2014-02-22 8:42 ` Wolfgang Denk
0 siblings, 1 reply; 3+ messages in thread
From: Chin Liang See @ 2014-02-21 22:46 UTC (permalink / raw)
To: u-boot
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 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 | 214 +++++++
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, 991 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..6e6f372
--- /dev/null
+++ b/arch/arm/cpu/armv7/socfpga/scan_manager.c
@@ -0,0 +1,214 @@
+/*
+ * 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_mgr_io_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_IO_SCAN_ENGINE_STATUS_ACTIVE;
+ }
+ return SCAN_MGR_IO_SCAN_ENGINE_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 (3 == io_scan_chain_id)
+ 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_MGR_IO_SCAN_ENGINE_STATUS_IDLE
+ != scan_mgr_io_scan_chain_engine_is_idle(
+ MAX_WAITING_DELAY_IO_SCAN_ENGINE))
+ 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_IO_SCAN_ENGINE_STATUS_IDLE
+ != scan_mgr_io_scan_chain_engine_is_idle(
+ MAX_WAITING_DELAY_IO_SCAN_ENGINE))
+ goto error;
+ }
+
+ /* Calculate array index for final TDI_TDO packet */
+ index = io_program_iter * 4;
+
+ /* Final TDI_TDO packet if any */
+ if (0 != 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_IO_SCAN_ENGINE_STATUS_IDLE
+ != scan_mgr_io_scan_chain_engine_is_idle(
+ MAX_WAITING_DELAY_IO_SCAN_ENGINE))
+ 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..1166dbb
--- /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_IO_SCAN_ENGINE_STATUS_IDLE (0)
+
+/* IO scan chain engine is active */
+#define SCAN_MGR_IO_SCAN_ENGINE_STATUS_ACTIVE (1)
+
+/*
+ * Maximum polling loop to wait for IO scan chain engine
+ * becomes idle to prevent infinite loop
+ */
+#define MAX_WAITING_DELAY_IO_SCAN_ENGINE (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
--
1.7.9.5
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH v5] socfpga: Adding Scan Manager driver
2014-02-21 22:46 [U-Boot] [PATCH v5] socfpga: Adding Scan Manager driver Chin Liang See
@ 2014-02-22 8:42 ` Wolfgang Denk
2014-02-27 16:03 ` Chin Liang See
0 siblings, 1 reply; 3+ messages in thread
From: Wolfgang Denk @ 2014-02-22 8:42 UTC (permalink / raw)
To: u-boot
Dear Chin Liang See,
In message <1393022790-9296-1-git-send-email-clsee@altera.com> you 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 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 | 214 +++++++
> 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, 991 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..6e6f372
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/socfpga/scan_manager.c
> @@ -0,0 +1,214 @@
> +/*
> + * 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_mgr_io_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--;
This is difficult to read due to unlucky indentation. I suggest to
rewrite like this:
while (SCANMGR_STAT_ACTIVE_GET(scanmgr_status) ||
(SCANMGR_STAT_WFIFOCNT_GET(scanmgr_status) > 0)) {
or even
while (SCANMGR_STAT_ACTIVE_GET(scanmgr_status) ||
(SCANMGR_STAT_WFIFOCNT_GET(scanmgr_status) > 0)
) {
> + scanmgr_status = readl(&scan_manager_base->stat);
> + else
> + return SCAN_MGR_IO_SCAN_ENGINE_STATUS_ACTIVE;
> + }
> + return SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE;
> +}
You decrement the parameter "max_iter" in the loop - I guess this was
intended to make sure this is not an endless loop. But there is no
test anywhere for it becoming 0 or so - which makes the parameter
unused. I guess this needs fixing?
> + if (SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE
> + != scan_mgr_io_scan_chain_engine_is_idle(
> + MAX_WAITING_DELAY_IO_SCAN_ENGINE))
> + return 1;
See above. This is not readable.
Blame yourself that you have such problems - why do you cose so long
identifiers as "SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE" or
"scan_mgr_io_scan_chain_engine_is_idle" - when you cannot put two
names on a single line without breaking the 80 character limit, you
are doing something wrong.
> + if (SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE
> + != scan_mgr_io_scan_chain_engine_is_idle(
> + MAX_WAITING_DELAY_IO_SCAN_ENGINE))
> + goto error;
Same here. Fix the indentation.
Even better, fix these unwieldy names.
> + /* Final TDI_TDO packet if any */
> + if (0 != io_scan_chain_data_residual) {
Actually I would prefer if you wrote all this in the natural order,
i. e. as
if (io_scan_chain_data_residual != 0)
With != it's just a nuisance, but with < or > it requires the reader
to think backwards which makes the code hard to read and increases the
error rate.
> + tdi_tdo_header = TDI_TDO_HEADER_FIRST_BYTE |
> + ((io_scan_chain_data_residual - 1)
> + << TDI_TDO_HEADER_SECOND_BYTE_SHIFT);
Again, this is hard to read. Please fix globally. Do not start a new
line with an operation - put this in the previous line where it serves
as a mark that the line will need a continuation.
> + 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);
Incorrect multiline comment style, fix the last line.
Second, even if checkpatch lats this pass, we don't. This is a
multi-line statement (even without the comment), and as such it
requires braces.
> + 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 {
Braces needed - first because it is multiline, second because the esle
branch has braces.
> + for (i = 0; i < residual; i += 8)
> + writel(((iocsr_scan_chain[index] >> i)
> + & IO_SCAN_CHAIN_BYTE_MASK),
> + &scan_manager_base->fifo_single_byte);
Braces needed.
> + if (SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE
> + != scan_mgr_io_scan_chain_engine_is_idle(
> + MAX_WAITING_DELAY_IO_SCAN_ENGINE))
> + goto error;
fix indentation.,
> +#define IO_SCAN_CHAIN_128BIT_SHIFT (7)
Remove parens around simple values - please fix globally.
> +++ 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
Why is this not under GPL? New code that gets added to U-Boot shall
be GPL-2.0+
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Every solution breeds new problems.
^ permalink raw reply [flat|nested] 3+ messages in thread
* [U-Boot] [PATCH v5] socfpga: Adding Scan Manager driver
2014-02-22 8:42 ` Wolfgang Denk
@ 2014-02-27 16:03 ` Chin Liang See
0 siblings, 0 replies; 3+ messages in thread
From: Chin Liang See @ 2014-02-27 16:03 UTC (permalink / raw)
To: u-boot
Dear Wolfgang,
On Sat, 2014-02-22 at 09:42 +0100, ZY - wd wrote:
> Dear Chin Liang See,
>
> In message <1393022790-9296-1-git-send-email-clsee@altera.com> you 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 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 | 214 +++++++
> > 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, 991 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..6e6f372
> > --- /dev/null
> > +++ b/arch/arm/cpu/armv7/socfpga/scan_manager.c
> > @@ -0,0 +1,214 @@
> > +/*
> > + * 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_mgr_io_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--;
>
> This is difficult to read due to unlucky indentation. I suggest to
> rewrite like this:
>
>
> while (SCANMGR_STAT_ACTIVE_GET(scanmgr_status) ||
> (SCANMGR_STAT_WFIFOCNT_GET(scanmgr_status) > 0)) {
>
> or even
>
> while (SCANMGR_STAT_ACTIVE_GET(scanmgr_status) ||
> (SCANMGR_STAT_WFIFOCNT_GET(scanmgr_status) > 0)
> ) {
>
Fixed
> > + scanmgr_status = readl(&scan_manager_base->stat);
> > + else
> > + return SCAN_MGR_IO_SCAN_ENGINE_STATUS_ACTIVE;
> > + }
> > + return SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE;
> > +}
>
> You decrement the parameter "max_iter" in the loop - I guess this was
> intended to make sure this is not an endless loop. But there is no
> test anywhere for it becoming 0 or so - which makes the parameter
> unused. I guess this needs fixing?
Actually this is checked at later line of code where the function will
exit if max_iter equal to zero or less.
>
> > + if (SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE
> > + != scan_mgr_io_scan_chain_engine_is_idle(
> > + MAX_WAITING_DELAY_IO_SCAN_ENGINE))
> > + return 1;
>
> See above. This is not readable.
>
> Blame yourself that you have such problems - why do you cose so long
> identifiers as "SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE" or
> "scan_mgr_io_scan_chain_engine_is_idle" - when you cannot put two
> names on a single line without breaking the 80 character limit, you
> are doing something wrong.
Actually the variables are generated by tools. But I agree that it
create unnecessary length which make the code hard to read. With that, I
fixed all these variables.
>
> > + if (SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE
> > + != scan_mgr_io_scan_chain_engine_is_idle(
> > + MAX_WAITING_DELAY_IO_SCAN_ENGINE))
> > + goto error;
>
> Same here. Fix the indentation.
>
> Even better, fix these unwieldy names.
Fixed
>
> > + /* Final TDI_TDO packet if any */
> > + if (0 != io_scan_chain_data_residual) {
>
> Actually I would prefer if you wrote all this in the natural order,
> i. e. as
>
> if (io_scan_chain_data_residual != 0)
>
> With != it's just a nuisance, but with < or > it requires the reader
> to think backwards which makes the code hard to read and increases the
> error rate.
Fixed by simplifying it to if (io_scan_chain_data_residual)
>
> > + tdi_tdo_header = TDI_TDO_HEADER_FIRST_BYTE |
> > + ((io_scan_chain_data_residual - 1)
> > + << TDI_TDO_HEADER_SECOND_BYTE_SHIFT);
>
> Again, this is hard to read. Please fix globally. Do not start a new
> line with an operation - put this in the previous line where it serves
> as a mark that the line will need a continuation.
Fixed
>
> > + 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);
>
> Incorrect multiline comment style, fix the last line.
Fixed
>
> Second, even if checkpatch lats this pass, we don't. This is a
> multi-line statement (even without the comment), and as such it
> requires braces.
Yup, good to note this. Fixed
>
> > + 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 {
>
> Braces needed - first because it is multiline, second because the esle
> branch has braces.
Fixed
>
> > + for (i = 0; i < residual; i += 8)
> > + writel(((iocsr_scan_chain[index] >> i)
> > + & IO_SCAN_CHAIN_BYTE_MASK),
> > + &scan_manager_base->fifo_single_byte);
>
> Braces needed.
Fixed
>
> > + if (SCAN_MGR_IO_SCAN_ENGINE_STATUS_IDLE
> > + != scan_mgr_io_scan_chain_engine_is_idle(
> > + MAX_WAITING_DELAY_IO_SCAN_ENGINE))
> > + goto error;
>
> fix indentation.,
Fixed
>
>
> > +#define IO_SCAN_CHAIN_128BIT_SHIFT (7)
>
> Remove parens around simple values - please fix globally.
Good to know this. Fixed
>
> > +++ 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
>
> Why is this not under GPL? New code that gets added to U-Boot shall
> be GPL-2.0+
Actually this is tools generated which will be consumed by non GPL
software too. This file doesn't contain any source but just array with
values. Hope this is ok.
>
>
> Best regards,
>
> Wolfgang Denk
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-02-27 16:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-21 22:46 [U-Boot] [PATCH v5] socfpga: Adding Scan Manager driver Chin Liang See
2014-02-22 8:42 ` Wolfgang Denk
2014-02-27 16:03 ` Chin Liang See
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.