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