(Resend to netdev; already sent to relevant individuals.)

Here's a possible fix for the p[] array issues akpm noticed.
This replaces them with calls to a new mdio_write_bits function.

Boot-tested, passes net traffic, and mii-tool and mii-diag produce sensible
output (including noticing link status changes).

Also, regarding
>> +    for (i = 0; i < IPG_TFDLIST_LENGTH; i++) {
>> +            offset = (u32) &sp->txd[i].next_desc - (u32) sp->txd;
>> +            printk(KERN_INFO "%2.2x %4.4x TFDNextPtr = %16.16lx\n", i,
>> +                   offset, (unsigned long) sp->txd[i].next_desc);
>> +
>> +            offset = (u32) &sp->txd[i].tfc - (u32) sp->txd;
>
> Is the u32 cast safe here on all architectures?

IPG_TFDLIST_LENGTH is 256, and sp->txd is an array of struct ipg_tx,
which are 24 bytes each, so the most it can be is 6K.  The result fits
into 32 bits, so the inputs can be safely truncated.

A more awkward way to write it would be
        offset = i * sizeof(struct ipg_tx) + offsetof(struct ipg_tx, tfc);


This patch is placed in the public domain; copyright abandoned.

(The final hunk is a space-TAB whitespace repair that git complained about
when I imported the patch.)

diff --git a/drivers/net/ipg.c b/drivers/net/ipg.c
index 87a674c..6267a34 100644
--- a/drivers/net/ipg.c
+++ b/drivers/net/ipg.c
@@ -180,12 +180,31 @@ static u16 read_phy_bit(void __iomem * ioaddr, u8 
phyctrlpolarity)
 }
 
 /*
+ * Transmit the given bits, MSB-first, through the MgmtData bit (bit 1)
+ * of the PhyCtrl register. 1 <= len <= 32.  "ioaddr" is the register
+ * address, and "otherbits" are the values of the other bits.
+ */
+static void mdio_write_bits(void __iomem *ioaddr, u8 otherbits, u32 data, 
unsigned len)
+{
+       otherbits |= IPG_PC_MGMTDIR;
+       do {
+               /* IPG_PC_MGMTDATA is a power of 2; compiler knows to shift */
+               u8 d = ((data >> --len) & 1) * IPG_PC_MGMTDATA;
+               /* + rather than | lets compiler microoptimize better */
+               ipg_drive_phy_ctl_low_high(ioaddr, d + otherbits);
+       } while (len);
+}
+
+/*
  * Read a register from the Physical Layer device located
  * on the IPG NIC, using the IPG PHYCTRL register.
  */
 static int mdio_read(struct net_device * dev, int phy_id, int phy_reg)
 {
        void __iomem *ioaddr = ipg_ioaddr(dev);
+       u8 const polarity = ipg_r8(PHY_CTRL) &
+                       (IPG_PC_DUPLEX_POLARITY | IPG_PC_LINK_POLARITY);
+       unsigned i, data = 0;
        /*
         * The GMII mangement frame structure for a read is as follows:
         *
@@ -199,75 +218,30 @@ static int mdio_read(struct net_device * dev, int phy_id, 
int phy_reg)
         * D = bit of read data (MSB first)
         *
         * Transmission order is 'Preamble' field first, bits transmitted
-        * left to right (first to last).
+        * left to right (msbit-first).
         */
-       struct {
-               u32 field;
-               unsigned int len;
-       } p[] = {
-               { GMII_PREAMBLE,        32 },   /* Preamble */
-               { GMII_ST,              2  },   /* ST */
-               { GMII_READ,            2  },   /* OP */
-               { phy_id,               5  },   /* PHYAD */
-               { phy_reg,              5  },   /* REGAD */
-               { 0x0000,               2  },   /* TA */
-               { 0x0000,               16 },   /* DATA */
-               { 0x0000,               1  }    /* IDLE */
-       };
-       unsigned int i, j;
-       u8 polarity, data;
-
-       polarity  = ipg_r8(PHY_CTRL);
-       polarity &= (IPG_PC_DUPLEX_POLARITY | IPG_PC_LINK_POLARITY);
-
-       /* Create the Preamble, ST, OP, PHYAD, and REGAD field. */
-       for (j = 0; j < 5; j++) {
-               for (i = 0; i < p[j].len; i++) {
-                       /* For each variable length field, the MSB must be
-                        * transmitted first. Rotate through the field bits,
-                        * starting with the MSB, and move each bit into the
-                        * the 1st (2^1) bit position (this is the bit position
-                        * corresponding to the MgmtData bit of the PhyCtrl
-                        * register for the IPG).
-                        *
-                        * Example: ST = 01;
-                        *
-                        *          First write a '0' to bit 1 of the PhyCtrl
-                        *          register, then write a '1' to bit 1 of the
-                        *          PhyCtrl register.
-                        *
-                        * To do this, right shift the MSB of ST by the value:
-                        * [field length - 1 - #ST bits already written]
-                        * then left shift this result by 1.
-                        */
-                       data  = (p[j].field >> (p[j].len - 1 - i)) << 1;
-                       data &= IPG_PC_MGMTDATA;
-                       data |= polarity | IPG_PC_MGMTDIR;
-
-                       ipg_drive_phy_ctl_low_high(ioaddr, data);
-               }
-       }
-
-       send_three_state(ioaddr, polarity);
-
-       read_phy_bit(ioaddr, polarity);
+       mdio_write_bits(ioaddr, polarity, GMII_PREAMBLE, 32);
+       mdio_write_bits(ioaddr, polarity, GMII_ST<<12 | GMII_READ << 10 |
+                                         phy_id << 5 | phy_reg, 14);
 
        /*
         * For a read cycle, the bits for the next two fields (TA and
         * DATA) are driven by the PHY (the IPG reads these bits).
         */
-       for (i = 0; i < p[6].len; i++) {
-               p[6].field |=
-                   (read_phy_bit(ioaddr, polarity) << (p[6].len - 1 - i));
-       }
+       send_three_state(ioaddr, polarity);     /* TA first bit */
+       (void)read_phy_bit(ioaddr, polarity);           /* TA second bit */
 
+       for (i = 0; i < 16; i++)
+               data += data + read_phy_bit(ioaddr, polarity);
+
+       /* Trailing idle */
        send_three_state(ioaddr, polarity);
        send_three_state(ioaddr, polarity);
        send_three_state(ioaddr, polarity);
        send_end(ioaddr, polarity);
 
        /* Return the value of the DATA field. */
-       return p[6].field;
+       return data;
 }
 
 /*
@@ -277,6 +251,8 @@ static int mdio_read(struct net_device * dev, int phy_id, 
int phy_reg)
 static void mdio_write(struct net_device *dev, int phy_id, int phy_reg, int 
val)
 {
        void __iomem *ioaddr = ipg_ioaddr(dev);
+       u8 const polarity = ipg_r8(PHY_CTRL) &
+                       (IPG_PC_DUPLEX_POLARITY | IPG_PC_LINK_POLARITY);
        /*
         * The GMII mangement frame structure for a read is as follows:
         *
@@ -290,66 +266,18 @@ static void mdio_write(struct net_device *dev, int 
phy_id, int phy_reg, int val)
         * D = bit of write data (MSB first)
         *
         * Transmission order is 'Preamble' field first, bits transmitted
-        * left to right (first to last).
+        * left to right (msbit-first).
         */
-       struct {
-               u32 field;
-               unsigned int len;
-       } p[] = {
-               { GMII_PREAMBLE,        32 },   /* Preamble */
-               { GMII_ST,              2  },   /* ST */
-               { GMII_WRITE,           2  },   /* OP */
-               { phy_id,               5  },   /* PHYAD */
-               { phy_reg,              5  },   /* REGAD */
-               { 0x0002,               2  },   /* TA */
-               { val & 0xffff,         16 },   /* DATA */
-               { 0x0000,               1  }    /* IDLE */
-       };
-       unsigned int i, j;
-       u8 polarity, data;
-
-       polarity  = ipg_r8(PHY_CTRL);
-       polarity &= (IPG_PC_DUPLEX_POLARITY | IPG_PC_LINK_POLARITY);
-
-       /* Create the Preamble, ST, OP, PHYAD, and REGAD field. */
-       for (j = 0; j < 7; j++) {
-               for (i = 0; i < p[j].len; i++) {
-                       /* For each variable length field, the MSB must be
-                        * transmitted first. Rotate through the field bits,
-                        * starting with the MSB, and move each bit into the
-                        * the 1st (2^1) bit position (this is the bit position
-                        * corresponding to the MgmtData bit of the PhyCtrl
-                        * register for the IPG).
-                        *
-                        * Example: ST = 01;
-                        *
-                        *          First write a '0' to bit 1 of the PhyCtrl
-                        *          register, then write a '1' to bit 1 of the
-                        *          PhyCtrl register.
-                        *
-                        * To do this, right shift the MSB of ST by the value:
-                        * [field length - 1 - #ST bits already written]
-                        * then left shift this result by 1.
-                        */
-                       data  = (p[j].field >> (p[j].len - 1 - i)) << 1;
-                       data &= IPG_PC_MGMTDATA;
-                       data |= polarity | IPG_PC_MGMTDIR;
-
-                       ipg_drive_phy_ctl_low_high(ioaddr, data);
-               }
-       }
+       mdio_write_bits(ioaddr, polarity, GMII_PREAMBLE, 32);
+       mdio_write_bits(ioaddr, polarity, GMII_ST << 14 | GMII_WRITE << 12 |
+                                         phy_id << 7 | phy_reg << 2 |
+                                         0x2, 16);
+       mdio_write_bits(ioaddr, polarity, val, 16);     /* DATA */
 
        /* The last cycle is a tri-state, so read from the PHY. */
-       for (j = 7; j < 8; j++) {
-               for (i = 0; i < p[j].len; i++) {
-                       ipg_write_phy_ctl(ioaddr, IPG_PC_MGMTCLK_LO | polarity);
-
-                       p[j].field |= ((ipg_r8(PHY_CTRL) &
-                               IPG_PC_MGMTDATA) >> 1) << (p[j].len - 1 - i);
-
-                       ipg_write_phy_ctl(ioaddr, IPG_PC_MGMTCLK_HI | polarity);
-               }
-       }
+       ipg_write_phy_ctl(ioaddr, IPG_PC_MGMTCLK_LO | polarity);
+       (void)ipg_r8(PHY_CTRL);
+       ipg_write_phy_ctl(ioaddr, IPG_PC_MGMTCLK_HI | polarity);
 }
 
 /* Set LED_Mode JES20040127EEPROM */
@@ -1484,7 +1412,7 @@ static int ipg_nic_rx(struct net_device *dev)
                                 * Indicate IP checksums were performed
                                 * by the IPG.
                                 *
-                               skb->ip_summed = CHECKSUM_UNNECESSARY;
+                               skb->ip_summed = CHECKSUM_UNNECESSARY;
                         } else
                         */
                         {
-
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

Reply via email to