From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kuo-Jung Su Date: Mon, 22 Apr 2013 10:56:13 +0800 Subject: [U-Boot] [PATCH v2 03/12] net: add Faraday FTMAC110 10/100Mbps ethernet support In-Reply-To: <20130418105201.7508820019A@gemini.denx.de> References: <1366277139-29728-1-git-send-email-dantesu@gmail.com> <1364540788-13943-2-git-send-email-dantesu@gmail.com> <1366277139-29728-4-git-send-email-dantesu@gmail.com> <20130418105201.7508820019A@gemini.denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 2013/4/18 Wolfgang Denk : > Dear Kuo-Jung Su, > > In message <1366277139-29728-4-git-send-email-dantesu@gmail.com> you wrote: >> >> +/*******************************************************************/ >> +/* FTMAC110 DMA design issue */ >> +/* Dante Su 2010.02.03 */ >> +/* */ >> +/* The DMA engine has a weird restriction that its Rx DMA engine */ >> +/* accepts only 16-bits aligned address, 32-bits aligned is not */ >> +/* acceptable. However this restriction does not apply to Tx DMA. */ >> +/* Conclusion: */ >> +/* (1) Tx DMA Buffer Address: */ >> +/* 1 bytes aligned: Invalid */ >> +/* 2 bytes aligned: O.K */ >> +/* 4 bytes aligned: O.K (-> u-boot ZeroCopy is possible) */ >> +/* (2) Rx DMA Buffer Address: */ >> +/* 1 bytes aligned: Invalid */ >> +/* 2 bytes aligned: O.K */ >> +/* 4 bytes aligned: Invalid */ >> +/*******************************************************************/ > > Incorrect multi-line comment style. Please fix globally. > Got it, thanks >> +/* Register access macros */ >> +#define MAC_READ(r) le32_to_cpu(readl(r)) >> +#define MAC_WRITE(v, r) writel(cpu_to_le32(v), r) > > Please drop these and use the real functions instead. > Got it, thanks. >> +static char ftmac110_mac_addr[] = { 0x00, 0x41, 0x71, 0x00, 0x00, 0x52 }; > > NAK. We do not allow static configuration of MAC addresses. > Got it, thanks. >> + ulong ts; >> + uint32_t maccr; >> + uint16_t pa, tmp; > > Are you sure using uint16_t gives more efficient code than a plain > int? > No, it won't. It' would be fixed latter. >> + if (pa >= 32) { >> + puts("ftmac110: phy device not found!\n"); > > make this a debug() ? > > ... Got it, thanks >> + MAC_WRITE(0x00001010, ®s->itc); >> + MAC_WRITE(0x00000001, ®s->aptc); >> + MAC_WRITE(0x00000390, ®s->dblac); >> + MAC_WRITE(0x000003FF, ®s->isr); > > What do these magic numbers mean? > > Got it, thanks. I'll add comments to these timing control registers >> +struct ftmac110_rxd { >> + /* RXDES0 */ >> +#ifdef __ARMEB__ >> + uint32_t owner:1; /* BIT: 31 - 1:Hardware, 0: Software */ >> + uint32_t rsvd3:1; >> + uint32_t frs:1; >> + uint32_t lrs:1; >> + uint32_t rsvd2:5; >> + uint32_t error:5; >> + uint32_t bcast:1; >> + uint32_t mcast:1; >> + uint32_t rsvd1:5; >> + uint32_t len:11; >> +#else >> + uint32_t len:11; >> + uint32_t rsvd1:5; >> + uint32_t mcast:1; >> + uint32_t bcast:1; >> + uint32_t error:5; >> + uint32_t rsvd2:5; >> + uint32_t lrs:1; >> + uint32_t frs:1; >> + uint32_t rsvd3:1; >> + uint32_t owner:1; /* BIT: 31 - 1:Hardware, 0: Software */ >> +#endif > > NAK. Please do NOT use bit fields. They are non-portable and > dangerous. > > Got it, thanks > 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 > A year spent in artificial intelligence is enough to make one believe > in God. -- Best wishes, Kuo-Jung Su