On Friday, August 26, 2011 08:36:51 AM Ajay Bhargav wrote:
> This patch adds support for Fast Ethernet Controller driver for
> Armada100 series.
>
> Signed-off-by: Ajay Bhargav <ajay.bhar...@einfochips.com>
Hi, please don't forget to CC me next time ;-)
[...]
> +static int smi_reg_read(const char *devname, u8 phy_addr, u8 phy_reg,
> + u16 *value)
> +{
> + struct eth_device *dev = eth_get_dev_by_name(devname);
> + struct armdfec_device *darmdfec = to_darmdfec(dev);
> + struct armdfec_reg *regs = darmdfec->regs;
> + u32 val, reg_data;
> +
> + if (phy_addr == PHY_ADR_REQ && phy_reg == PHY_ADR_REQ) {
> + reg_data = readl(®s->phyadr);
> + *value = (u16) (reg_data & 0x1f);
Do you need this cast?
> + return 0;
> + }
> +
> + /* check parameters */
> + if (phy_addr > PHY_MASK) {
> + printf("Err..(%s) Invalid phy address: 0x%X\n",
> + __func__, phy_addr);
Maybe a firm "A100 FEC: Invalid phy address (address = 0x%x)\n" would be
better,
fix that "Err.." and "Err " globally please.
> + return -EINVAL;
> + }
> + if (phy_reg > PHY_MASK) {
> + printf("Err..(%s) Invalid register offset: 0x%X\n",
> + __func__, phy_reg);
> + return -EINVAL;
> + }
> +
> + /* wait for the SMI register to become available */
> + if (armdfec_phy_timeout(®s->smi, SMI_BUSY, FALSE)) {
> + printf("Error (%s) PHY busy timeout\n", __func__);
> + return -1;
> + }
> +
> + writel((phy_addr << 16) | (phy_reg << 21) | SMI_OP_R, ®s->smi);
> +
> + /* now wait for the data to be valid */
> + if (armdfec_phy_timeout(®s->smi, SMI_R_VALID, TRUE)) {
> + val = readl(®s->smi);
> + printf("Err (%s) PHY Read timeout, val=0x%x\n", __func__, val);
> + return -1;
> + }
> + val = readl(®s->smi);
> + *value = val & 0xffff;
> +
> + return 0;
> +}
> +
> +static int smi_reg_write(const char *devname,
> + u8 phy_addr, u8 phy_reg, u16 value)
> +{
> + struct eth_device *dev = eth_get_dev_by_name(devname);
> + struct armdfec_device *darmdfec = to_darmdfec(dev);
> + struct armdfec_reg *regs = darmdfec->regs;
> +
> + if (phy_addr == PHY_ADR_REQ && phy_reg == PHY_ADR_REQ) {
> + clrsetbits_le32(®s->phyadr, 0x1f, value & 0x1f);
> + return 0;
> + }
> +
> + /* check parameters */
> + if (phy_addr > PHY_MASK) {
> + printf("Err..(%s) Invalid phy address\n", __func__);
> + return -EINVAL;
> + }
> + if (phy_reg > PHY_MASK) {
> + printf("Err..(%s) Invalid register offset\n", __func__);
> + return -EINVAL;
> + }
> +
> + /* wait for the SMI register to become available */
> + if (armdfec_phy_timeout(®s->smi, SMI_BUSY, FALSE)) {
> + printf("Error (%s) PHY busy timeout\n", __func__);
> + return -1;
> + }
> +
> + writel((phy_addr << 16) | (phy_reg << 21) | SMI_OP_W | (value & 0xffff),
> + ®s->smi);
> + return 0;
> +}
> +
> +/*
> + * Abort any transmit and receive operations and put DMA
> + * in idle state. AT and AR bits are cleared upon entering
> + * in IDLE state. So poll those bits to verify operation.
> + */
> +static void abortdma(struct eth_device *dev)
> +{
> + struct armdfec_device *darmdfec = to_darmdfec(dev);
> + struct armdfec_reg *regs = darmdfec->regs;
> + int delay;
> + int maxretries = 40;
> + u32 tmp;
> +
while (--maxretries) {
That way it won't get negative and you can check below for if (!maxretries)
> + while (maxretries--) {
> + writel(SDMA_CMD_AR | SDMA_CMD_AT, ®s->sdma_cmd);
> + udelay(100);
> +
> + delay = 10;
> + while (delay--) {
This will go negative and the check below will be true for -1.Fix this to
while (--delay) {
> + tmp = readl(®s->sdma_cmd);
> + if (!(tmp & (SDMA_CMD_AR | SDMA_CMD_AT)))
> + break;
> + udelay(10);
> + }
> + if (delay)
> + break;
> + }
> +
> + if (maxretries <= 0)
> + printf("%s : DMA Stuck\n", __func__);
> +}
See two comments above.
> +
> +static inline u32 nibble_swapping_32_bit(u32 x)
> +{
> + return (((x) & 0xf0f0f0f0) >> 4) | (((x) & 0x0f0f0f0f) << 4);
> +}
> +
> +static inline u32 nibble_swapping_16_bit(u32 x)
> +{
> + return (((x) & 0x0000f0f0) >> 4) | (((x) & 0x00000f0f) << 4);
> +}
> +
> +static inline u32 flip_4_bits(u32 x)
> +{
> + return (((x) & 0x01) << 3) | (((x) & 0x002) << 1)
> + | (((x) & 0x04) >> 1) | (((x) & 0x008) >> 3);
> +}
> +
> +/*
> + * This function will calculate the hash function of the address.
> + * depends on the hash mode and hash size.
> + * Inputs
> + * mach - the 2 most significant bytes of the MAC address.
> + * macl - the 4 least significant bytes of the MAC address.
> + * Outputs
> + * return the calculated entry.
> + */
> +static u32 hash_function(u32 mach, u32 macl)
> +{
> + u32 hashresult;
> + u32 addrh;
> + u32 addrl;
> + u32 addr0;
> + u32 addr1;
> + u32 addr2;
> + u32 addr3;
> + u32 addrhswapped;
> + u32 addrlswapped;
> +
> + addrh = nibble_swapping_16_bit(mach);
> + addrl = nibble_swapping_32_bit(macl);
> +
> + addrhswapped = flip_4_bits(addrh & 0xf)
> + + ((flip_4_bits((addrh >> 4) & 0xf)) << 4)
> + + ((flip_4_bits((addrh >> 8) & 0xf)) << 8)
> + + ((flip_4_bits((addrh >> 12) & 0xf)) << 12);
> +
> + addrlswapped = flip_4_bits(addrl & 0xf)
> + + ((flip_4_bits((addrl >> 4) & 0xf)) << 4)
> + + ((flip_4_bits((addrl >> 8) & 0xf)) << 8)
> + + ((flip_4_bits((addrl >> 12) & 0xf)) << 12)
> + + ((flip_4_bits((addrl >> 16) & 0xf)) << 16)
> + + ((flip_4_bits((addrl >> 20) & 0xf)) << 20)
> + + ((flip_4_bits((addrl >> 24) & 0xf)) << 24)
> + + ((flip_4_bits((addrl >> 28) & 0xf)) << 28);
> +
> + addrh = addrhswapped;
> + addrl = addrlswapped;
> +
> + addr0 = (addrl >> 2) & 0x03f;
> + addr1 = (addrl & 0x003) | (((addrl >> 8) & 0x7f) << 2);
> + addr2 = (addrl >> 15) & 0x1ff;
> + addr3 = ((addrl >> 24) & 0x0ff) | ((addrh & 1) << 8);
> +
> + hashresult = (addr0 << 9) | (addr1 ^ addr2 ^ addr3);
> + hashresult = hashresult & 0x07ff;
> + return hashresult;
> +}
> +
> +/*
> + * This function will add an entry to the address table.
> + * depends on the hash mode and hash size that was initialized.
> + * Inputs
> + * mach - the 2 most significant bytes of the MAC address.
> + * macl - the 4 least significant bytes of the MAC address.
> + * skip - if 1, skip this address.
> + * rd - the RD field in the address table.
> + * Outputs
> + * address table entry is added.
> + * 0 if success.
> + * -ENOSPC if table full
> + */
> +static int add_del_hash_entry(struct armdfec_device *darmdfec, u32 mach,
> + u32 macl, u32 rd, u32 skip, int del)
> +{
> + struct addr_table_entry_t *entry, *start;
> + u32 newhi;
> + u32 newlo;
> + u32 i;
> + u8 *last;
> +
> + newlo = (((mach >> 4) & 0xf) << 15)
> + | (((mach >> 0) & 0xf) << 11)
> + | (((mach >> 12) & 0xf) << 7)
> + | (((mach >> 8) & 0xf) << 3)
> + | (((macl >> 20) & 0x1) << 31)
> + | (((macl >> 16) & 0xf) << 27)
> + | (((macl >> 28) & 0xf) << 23)
> + | (((macl >> 24) & 0xf) << 19)
> + | (skip << HTESKIP) | (rd << HTERDBIT)
> + | HTEVALID;
> +
> + newhi = (((macl >> 4) & 0xf) << 15)
> + | (((macl >> 0) & 0xf) << 11)
> + | (((macl >> 12) & 0xf) << 7)
> + | (((macl >> 8) & 0xf) << 3)
> + | (((macl >> 21) & 0x7) << 0);
> +
> + /*
> + * Pick the appropriate table, start scanning for free/reusable
> + * entries at the index obtained by hashing the specified MAC address
> + */
> + start = (struct addr_table_entry_t *) (darmdfec->htpr);
> + entry = start + hash_function(mach, macl);
> + for (i = 0; i < HOP_NUMBER; i++) {
> + if (!(entry->lo & HTEVALID)) {
> + break;
> + } else {
> + /* if same address put in same position */
> + if (((entry->lo & 0xfffffff8) == (newlo & 0xfffffff8))
> + && (entry->hi == newhi))
> + break;
> + }
> + if (entry == start + 0x7ff)
> + entry = start;
> + else
> + entry++;
> + }
> +
> + if (((entry->lo & 0xfffffff8) != (newlo & 0xfffffff8)) &&
> + (entry->hi != newhi) && del)
> + return 0;
> +
> + if (i == HOP_NUMBER) {
> + if (!del) {
> + printf("%s: table section is full\n", __FILE__);
Unify the error reporting please.
It looks good, just a few nits
Cheers!
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
