Re: [U-Boot] [PATCH v3 03/11] net: add Faraday FTMAC110 10/100Mbps ethernet support
2013/5/3 Tom Rini : > On Fri, Apr 26, 2013 at 04:02:32PM +0800, Kuo-Jung Su wrote: > >> From: Kuo-Jung Su > [snip] >> + | (phyaddr << PHYCR_ADDR_SHIFT) >> + | (phyreg << PHYCR_REG_SHIFT) >> + | 0x3000; > > Magic number. > It's the HW debug function, it would be removed at next version. >> + >> + writel(tmp, ®s->phycr); >> + >> + for (ts = get_timer(0); get_timer(ts) < 1000; ) { > > Please define a TIMEOUT and use that insteadof 1000 all the time. > Got it, thanks > [snip] >> + /* interrupt at every packet transmit/receive */ >> + writel(0x1010, ®s->itc); >> + /* tx/rx poll interval=5.12us; rx_poll_cnt=1 */ >> + writel(0x0001, ®s->aptc); >> + /* rx fifo: high=1536, low=512 */ >> + writel(0x0390, ®s->dblac); >> + /* clear all interrupt status */ >> + writel(0x03FF, ®s->isr); > > More magic numbers. Please fix globally. Thanks! > Got it, thanks > -- > Tom -- Best wishes, Kuo-Jung Su ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3 03/11] net: add Faraday FTMAC110 10/100Mbps ethernet support
On Fri, Apr 26, 2013 at 04:02:32PM +0800, Kuo-Jung Su wrote: > From: Kuo-Jung Su [snip] > + | (phyaddr << PHYCR_ADDR_SHIFT) > + | (phyreg << PHYCR_REG_SHIFT) > + | 0x3000; Magic number. > + > + writel(tmp, ®s->phycr); > + > + for (ts = get_timer(0); get_timer(ts) < 1000; ) { Please define a TIMEOUT and use that insteadof 1000 all the time. [snip] > + /* interrupt at every packet transmit/receive */ > + writel(0x1010, ®s->itc); > + /* tx/rx poll interval=5.12us; rx_poll_cnt=1 */ > + writel(0x0001, ®s->aptc); > + /* rx fifo: high=1536, low=512 */ > + writel(0x0390, ®s->dblac); > + /* clear all interrupt status */ > + writel(0x03FF, ®s->isr); More magic numbers. Please fix globally. Thanks! -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot