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

Wolfgang Denk
2010-10-24T12:21:08Z

Dear Can Aydin,

In message <1287920728-6458-1-git-send-email-can.aydin@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!