[U-Boot] [PATCH v6] socfpga: Adding Scan Manager driver

[Pavel Machek]

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 at altera.com>
> Cc: Dinh Nguyen <dinguyen at altera.com>
> Cc: Wolfgang Denk <wd at denx.de>
> CC: Pavel Machek <pavel at denx.de>
> Cc: Tom Rini <trini at ti.com>
> Cc: Albert Aribaud <albert.u.boot at 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);
> +		else
> +			return 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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html