On Wed, Feb 08, 2017 at 03:20:27PM +0900, Jiada Wang wrote:
This looks basically fine, a couple of fairly minor things here:
> + for (i = 0; i < transfer->len / 4; i++) {
> + u8 temp;
> +
> + temp = *(buf + i * 4);
> + *(buf + i * 4) = *(buf + i * 4 + 3);
> + *(buf + i * 4 + 3) = temp;
Should this be using one of the cpu_to_ functions? It's a bit unclear
what the goal is here and if it'll work if the kernel is big endian
(which people do do with i.MX systems IIRC).
> + if (spi_imx->bpw_w == 1)
> + spi_imx_buf_rx_u8(spi_imx);
> + else if (spi_imx->bpw_w == 2)
> + spi_imx_buf_rx_u16(spi_imx);
switch statement please.
> + if (spi_imx->dynamic_burst) {
> + spi_imx->count_index =
> + spi_imx->count > MX51_ECSPI_CTRL_MAX_BURST ?
> + spi_imx->count % MX51_ECSPI_CTRL_MAX_BURST :
> + spi_imx->count % sizeof(u32);
Just write a normal if statement please, it's easier to read.
Attachment:
signature.asc
Description: PGP signature
