Dear Michal Simek,
sorry for the long delay. We're still lacking a (new) network
custodian...
In message <1280753377-2894-4-git-send-email-mon...@monstr.eu> you wrote:
> Add Xilinx LL Temac driver to u-boot.
>
> Signed-off-by: Michal Simek <mon...@monstr.eu>
> +++ b/drivers/net/xilinx_ll_temac.c
> @@ -0,0 +1,561 @@
> +/*
> + *
Drop blank line.
> + * Copyright (C) 2008 - 2009 Michal Simek <mon...@monstr.eu>
> + * June 2008 Microblaze optimalization, FIFO mode support
2009? Not 2010 ?
> +/* XPS_LL_TEMAC direct registers definition */
> +#define TEMAC_RAF0 (dev->iobase + 0x00)
> +#define TEMAC_TPF0 (dev->iobase + 0x04)
> +#define TEMAC_IFGP0 (dev->iobase + 0x08)
> +#define TEMAC_IS0 (dev->iobase + 0x0c)
> +#define TEMAC_IP0 (dev->iobase + 0x10)
> +#define TEMAC_IE0 (dev->iobase + 0x14)
> +
> +#define TEMAC_MSW0 (dev->iobase + 0x20)
> +#define TEMAC_LSW0 (dev->iobase + 0x24)
> +#define TEMAC_CTL0 (dev->iobase + 0x28)
> +#define TEMAC_RDY0 (dev->iobase + 0x2c)
Please use a C struct to describe the register layout.
> +/* XPS_LL_TEMAC indirect registers offset definition */
> +
> +#define RCW0 0x200
> +#define RCW1 0x240
> +#define TC 0x280
> +#define FCC 0x2c0
> +#define EMMC 0x300
> +#define PHYC 0x320
> +#define MC 0x340
> +#define UAW0 0x380
> +#define UAW1 0x384
> +#define MAW0 0x388
> +#define MAW1 0x38c
> +#define AFM 0x390
> +#define TIS 0x3a0
> +#define TIE 0x3a4
> +#define MIIMWD 0x3b0
> +#define MIIMAI 0x3b4
Ditto.
> + out_be32((u32 *)TEMAC_LSW0, phy_data);
> + out_be32((u32 *)TEMAC_CTL0, CNTLREG_WRITE_ENABLE_MASK | MIIMWD);
> + out_be32((u32 *)TEMAC_LSW0, (phy_addr << 5) | (reg_addr));
> + out_be32((u32 *)TEMAC_CTL0, \
> + CNTLREG_WRITE_ENABLE_MASK | MIIMAI | (emac << 10));
> + while (!(in_be32((u32 *)TEMAC_RDY0) & XTE_RSE_MIIM_WR_MASK))
The need to have all these casts should ring some alarm bell to you.
Please use a proper C struct instead.
> + out_be32((u32 *)TEMAC_LSW0, reg_data);
> + out_be32((u32 *)TEMAC_CTL0, \
> + CNTLREG_WRITE_ENABLE_MASK | (emac << 10) | reg_offset);
Drop the backslash.
> + debug ("%d: 0x%x ", j, result);
> + }
> + debug ("\n");
No spaces between function name and '('. Please fix globally.
Consider running your patch through checkpatch...
> + while (retries-- &&
> + ((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) == 0x24))
> + ;
Bad indentation.
> + if (i == 0x7c0a3) {
...
> + if (i == 0x1410cc2) {
Please use self-explaining symbolic names for these magic numbers,
and/or add sufficient comments what these mean.
> +static void debugll(int count)
> +{
> + printf ("%d fifo isr 0x%08x, fifo_ier 0x%08x, fifo_rdfr 0x%08x, "
> + "fifo_rdfo 0x%08x fifo_rlr 0x%08x\n", count, ll_fifo->isr, \
> + ll_fifo->ier, ll_fifo->rdfr, ll_fifo->rdfo, ll_fifo->rlf);
Drop the backslash. Please check & fix globally.
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: w...@denx.de
What can it profit a man to gain the whole world and to come to his
property with a gastric ulcer, a blown prostate, and bifocals?
-- John Steinbeck, _Cannery Row_
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
