On Sun, 2007-07-15 at 12:36 +0200, Michael Buesch wrote:
> On Sunday 15 July 2007 11:27:09 Bryan Wu wrote:
> > +#if defined(CONFIG_BFIN_MAC_USE_L1)
> > +# define bfin_mac_alloc(dma_handle, size) l1_data_sram_zalloc(size)
> > +# define bfin_mac_free(dma_handle, ptr) l1_data_sram_free(ptr)
> > +#else
> > +# define bfin_mac_alloc(dma_handle, size) \
> > + dma_alloc_coherent(NULL, size, dma_handle, GFP_NORMAL)
>
> What is GFP_NORMAL? It's not defined in latest linus' tree.
> I think you should use GFP_KERNEL, if you can sleep, or GFP_ATOMIC,
> if you can't.
>
Yes, it should be GFP_KERNEL, i missed up with ZONE_NORMAL and ZONE_DMA.
> The rest looks OK. Except the endianess issues. It might be no
> issue on the hardware this runs on, but in favour of "clean code"
> you might use leXX_to_cpu and friends anyway. :)
> This kind of bugs is done so often, even in places where it _does_
> matter. So, at least for the human reader, the leXX_to_cpu stuff
> says that you really understood what you were doing when writing
> the code. The current code says (to me), that it works by
> accident, somehow, although it seems you knew what you were doing. :)
>
As you know, this driver is original developed by Luke Yang. Now I am
maintaining it.
I just don't understand in this driver where should use leXX_to_cpu()
and where should
use cpu_to_leXX(). please give me some comments about the following
change:
---
@@ -483,9 +487,12 @@
void setup_mac_addr(u8 * mac_addr)
{
+ u32 addr_low = le32_to_cpu(*(u32 *) & mac_addr[0]);
+ u16 addr_hi = le16_to_cpu(*(u16 *) & mac_addr[4]);
+
/* this depends on a little-endian machine */
- bfin_write_EMAC_ADDRLO(*(u32 *) & mac_addr[0]);
- bfin_write_EMAC_ADDRHI(*(u16 *) & mac_addr[4]);
+ bfin_write_EMAC_ADDRLO(addr_low);
+ bfin_write_EMAC_ADDRHI(addr_hi);
}
static void adjust_tx_list(void)
@@ -866,10 +873,10 @@
int retval;
/* Grab the MAC address in the MAC */
- *(u32 *) (&(dev->dev_addr[0])) = bfin_read_EMAC_ADDRLO();
- *(u16 *) (&(dev->dev_addr[4])) = (u16) bfin_read_EMAC_ADDRHI();
+ *(u32 *) (&(dev->dev_addr[0])) = cpu_to_le32(bfin_read_EMAC_ADDRLO());
+ *(u16 *) (&(dev->dev_addr[4])) = cpu_to_le16((u16)
bfin_read_EMAC_ADDRHI());
---
Thanks
- Bryan Wu
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
