On 21-12-26 14:25:02, Matthias-Christian Ott wrote: > Commit 1a8deec09d12 ("pegasus: fixes reported packet length") tried to > configure the hardware to not include the FCS/CRC of Ethernet frames. > Unfortunately, this does not work with the D-Link DSB-650TX (USB IDs > 2001:4002 and 2001:400b): the transferred "packets" (in the terminology > of the hardware) still contain 4 additional octets. For IP packets in > Ethernet this is not a problem as IP packets contain their own lengths > fields but other protocols also see Ethernet frames that include the FCS > in the payload which might be a problem for some protocols. > > I was not able to open the D-Link DSB-650TX as the case is a very tight > press fit and opening it would likely destroy it. However, according to > the source code the earlier revision of the D-Link DSB-650TX (USB ID > 2001:4002) is a Pegasus (possibly AN986) and not Pegasus II (AN8511) > device. I also tried it with the later revision of the D-Link DSB-650TX > (USB ID 2001:400b) which is a Pegasus II device according to the source > code and had the same results. Therefore, I'm not sure whether the RXCS > (rx_crc_sent) field of the EC0 (Ethernet control_0) register has any > effect or in which revision of the hardware it is usable and has an > effect. As a result, it seems best to me to revert commit > 1a8deec09d12 ("pegasus: fixes reported packet length") and to set the > RXCS (rx_crc_sent) field of the EC0 (Ethernet control_0) register so > that the FCS/CRC is always included. > > Fixes: 1a8deec09d12 ("pegasus: fixes reported packet length") > Signed-off-by: Matthias-Christian Ott <ott@xxxxxxxxx> > --- > drivers/net/usb/pegasus.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c > index c4cd40b090fd..140d11ae6688 100644 > --- a/drivers/net/usb/pegasus.c > +++ b/drivers/net/usb/pegasus.c > @@ -422,7 +422,13 @@ static int enable_net_traffic(struct net_device *dev, struct usb_device *usb) > ret = read_mii_word(pegasus, pegasus->phy, MII_LPA, &linkpart); > if (ret < 0) > goto fail; > - data[0] = 0xc8; /* TX & RX enable, append status, no CRC */ > + /* At least two hardware revisions of the D-Link DSB-650TX (USB IDs > + * 2001:4002 and 2001:400b) include the Ethernet FCS in the packets, > + * even if RXCS is set to 0 in the EC0 register and the hardware is > + * instructed to not include the Ethernet FCS in the packet.Therefore, > + * it seems best to set RXCS to 1 and later ignore the Ethernet FCS. > + */ > + data[0] = 0xc9; /* TX & RX enable, append status, CRC */ > data[1] = 0; > if (linkpart & (ADVERTISE_100FULL | ADVERTISE_10FULL)) > data[1] |= 0x20; /* set full duplex */ > @@ -513,6 +519,13 @@ static void read_bulk_callback(struct urb *urb) > pkt_len = buf[count - 3] << 8; > pkt_len += buf[count - 4]; > pkt_len &= 0xfff; > + /* The FCS at the end of the packet is ignored. So subtract > + * its length to ignore it. > + */ > + pkt_len -= ETH_FCS_LEN; > + /* Subtract the length of the received status at the end of the > + * packet as it is not part of the Ethernet frame. > + */ > pkt_len -= 4; > } Nice catch. However, changing these constants for all devices isn't such a good idea. I'd rather use vendor and device IDs to distinguish these two cases in the above code. cheers, Petko