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

[Chin Liang See]

Hi Pavel,

Nice to hear from you again :)

On Fri, 2014-02-21 at 16:07 +0100, ZY - pavel wrote:
> Hi!
> 
> > +	/*
> > +	 * 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;
> > +	}
> 
> Hmm.. function named "_is_idle" maybe should just return 0 or 1?

Yup we can simplify that. Probably just remain for readability. Hope its
ok for you.

> 
> > +		/*
> > +		 * 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)) {
> > +			/* Disable IO Scan chain when error detected */
> > +			clrbits_le32(&scan_manager_base->en,
> > +				1 << io_scan_chain_id);
> > +			return 1;
> > +		}
> > +	}
> 
> "goto error" would help avoid code duplication.

Good suggestion here. Will change that.

> 
> > +struct socfpga_scan_manager {
> > +	u32	stat;
> > +	u32	en;
> > +	u32	padding[2];
> > +	u32	fifosinglebyte;
> > +	u32	fifodoublebyte;
> > +	u32	fifoquadbyte;
> > +};
> 
> some underscores should be added here.

Sure, for better readability.

Thanks!

Chin Liang

> 
> Thanks,
> 									Pavel