| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20070926094632.19889.qmail@science.horizon.com>
Date: 26 Sep 2007 05:46:32 -0400
From: linux@...izon.com
To: netdev@...r.kernel.org
Subject: Re: [PATCH] ipg: add IP1000A driver to kernel tree
(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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists