Liu Hui-R64343 wrote: > Hi, Hi,
>> >> static unsigned long spi_bases[] = { >> 0x43fa4000, >> 0x50010000, >> 0x53f84000, >> }; > Here hardcode the value in mx31, while in mx51 it use the macro. Which makes > Code style not consistent. yes, agree, the driver already contains a lot of hard-coded values for mx.31 and I changed only according to the mx.51. As I see, in the imx-regs.h for MX.31 several defines are missing, and the drivers define theirselves the values. I will add the defines, at least for spi, to the mx31-regs.h in a separate patch. >> +static unsigned long spi_bases[] = { >> + CSPI1_BASE_ADDR, >> + CSPI2_BASE_ADDR, >> + CSPI3_BASE_ADDR, >> +}; > See above comments. Ok, but this is correct. Only the MX31 part should be changed. >> struct mxc_spi_slave { >> struct spi_slave slave; >> unsigned long base; >> u32 ctrl_reg; >> + u32 cfg_reg; >> int gpio; >> }; > Only MX51 use it, MX31 will not use it. However, I need a general structure to support both processors. Agree this register is available only on the MX.51 processor, I can surround the definition with an #ifdef CONFIG_MX51 statement. > The function spi_cfg only used in MX51, will have compile warnings for MX31. > Use CONFIG_MX51 to cover it. Agree, and as reported by Tom, there are other issues regarding the MX.31. I will check all of them globally and I will try to test on a MX.31 board, too. >> + * a single byte first, >> followed by 4 words. > > Comments is wrong, should be "followed by 4 bytes" Agree, it must be changed. > >> + */ >> + if ((cnt_blk == 0) && (bitlen % 32) && >> + (j >= ((bitlen % 32) / 8))) { >> + continue; >> + } >> + if (pbuf) >> + tmpdata = *pbuf++ | >> (tmpdata << 8); >> + n_bytes--; >> + } >> + } >> + debug("writing TX FIFO 0x%x\n", tmpdata); >> + reg_write(mxcs->base + MXC_CSPITXDATA, tmpdata); >> } > Can use word copy for the left part(cnt_blk !=0) to get high performance. Not sure, but it could be I have not get the real problem here. >> + if (din) >> + *pbuf++ = (tmpdata >> >> + ((3 - >> spare_bytes - j) * 8)) > > The RX path should be logic wrong, spare_bytes not reset to zero and the data > got not correct when data is not 4B alignement. Thanks, I will recheck the code, surely spare_bytes must be reset to zero. Regards, Stefano Babic -- ===================================================================== DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de ===================================================================== _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot