[U-Boot] [PATCH v2] Freescale 85xx/P1/P2 eSPI controller driver

[Wolfgang Denk]

Dear Can Aydin,

In message <1287920728-6458-1-git-send-email-can.aydin at locatacorp.com> you wrote:
> Driver for the Freescale eSPI controller found in 85xx, P1/P2 and P4xx SoCs.
...
> +static inline void write_u32_part (u32 * in, u32 * out, u8 size)
> +{
> +	int i;
> +	u8 *ibyte = (u8 *) in;
> +	u8 *obyte = (u8 *) out;
> +
> +	for (i = 0; i < size; i++) {
> +		obyte[i] = ibyte[i];
> +	}
> +}

What is this cunction needed for?  Can you not use a plain memcpy()
instead?

If not, is it really sufficient you use plain memory read/write
operations here, without any MBs or similar (i. e. are you sure that
no I/O accessors are needed here) ?
+
> +	if (len > ESPI_MAX_TRANLEN) {
> +		debug
> +		    ("spi_xfer: Transfer length (%u) too long, max is %u(0x%X)\n",
> +		     len, ESPI_MAX_TRANLEN, ESPI_MAX_TRANLEN);

Indentation by TAB, please. And join the "debug" and "(..." lines
splitting the string, if it cannot be made shorter).

> +	debug ("spi_xfer: slave %u:%u dout %08X(%08x) din %08X(%08x) len %u\n",
> +	       slave->bus, slave->cs,
> +	       *(uint *) dout, (uint) dout, *(uint *) din, (uint) din, len);

Please globally fix your paren style. There is no space between
function (or macro) name and paren.

> +			} else if (ESPI_EV_GET_TXCNT (event) >= txcount) {
> +				/* Less than a word left and room to put it */
> +				tmpdout = 0;
> +				/* Avoid dout buffer read overflow */
> +				write_u32_part ((u32 *) dout, &tmpdout,
> +						txcount);
> +				//tmpdout = *(u32 *)dout;

Please do not add dead code.  C++ comments ar4 not allowed either.
Fix globally, please.

> diff --git a/include/configs/P1_P2_RDB.h b/include/configs/P1_P2_RDB.h
> index cff0ed3..33f1b1e 100644
> --- a/include/configs/P1_P2_RDB.h
> +++ b/include/configs/P1_P2_RDB.h
> @@ -172,6 +172,13 @@ extern unsigned long get_board_sys_clk(unsigned long dummy);
>   */
>  
>  /*
> + * eSPI - Enhanced SPI
> + */
> +#define CONFIG_FSL_ESPI
> +#define CONFIG_HARD_SPI
> +#define CONFIG_CMD_SPI

Please split into several commits: first adding the driver, the next
one(s) changing board configuration(s) to use that driver.


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
Boykottiert Microsoft - Kauft Eure Fenster bei OBI!