On 02/08/2013 04:02 AM, Ben Hutchings wrote:
On Thu, 2013-02-07 at 21:39 +0800, Freddy Xin wrote:
From: Freddy Xin <fre...@asix.com.tw>

This is a resubmission.
Added "const" to ethtool_ops structure and fixed the coding style of 
AX88179_BULKIN_SIZE array.
Fixed the issue that the default MTU is not 1500.
Added ax88179_change_mtu function and enabled the hardware jumbo frame function 
to support an
MTU higher than 1500.
Fixed indentation and empty line coding style errors.
The _nopm version usb functions were added to access register in suspend and 
resume functions.
Serveral variables allocted dynamically were removed and replaced by stack 
variables.
ax88179_get_eeprom were modified from asix_get_eeprom in asix_common.

This patch adds a driver for ASIX's AX88179 family of USB 3.0/2.0
to gigabit ethernet adapters. It's based on the AX88xxx driver but
the usb commands used to access registers for AX88179 are completely different.
This driver had been verified on x86 system with AX88179/AX88178A and
Sitcomm LN-032 USB dongles.

Signed-off-by: Freddy Xin <fre...@asix.com.tw>
[...]
--- /dev/null
+++ b/drivers/net/usb/ax88179_178a.c
[...]
+struct ax88179_data {
+       u16 rxctl;
+};
Should also be declared __packed, as on some architectures it may be
padded to 4 bytes.

rxctl is presumably required to be little-endian (__le16)?

+struct ax88179_int_data {
+       u16 res1;
+       u8 link;
+       u16 res2;
+       u8 status;
+       u16 res3;
+} __packed;
Same here, the u16 fields are presumably little-endian?

[...]
+struct ax88179_rx_pkt_header {
+       u8      l4_csum_err:1,
+               l3_csum_err:1,
+               l4_type:3,
+               l3_type:2,
+               ce:1;
+
+       u8      vlan_ind:3,
+               rx_ok:1,
+               pri:3,
+               bmc:1;
+
+       u16     len:13,
+               crc:1,
+               mii:1,
+               drop:1;
+} __packed;
This won't work on both big-endian systems (assuming this works on x86).
You apparently try to convert the structure in-place in
ax88179_rx_fixup() by calling le32_to_cpus(); that may work if you
define all the bitfields to be part of a u32 but it won't work with the
current definition.

[...]
+static int
+ax88179_set_features(struct net_device *net, netdev_features_t features)
+{
+       u8 tmp;
+       struct usbnet *dev = netdev_priv(net);
+       netdev_features_t changed = net->features ^ features;
+
+       if (changed & NETIF_F_TSO)
+               net->features ^= NETIF_F_TSO;
+
+       if (changed & NETIF_F_SG)
+               net->features ^= NETIF_F_SG;
Don't change net->features; the caller will do that for you.

+       if (changed & NETIF_F_IP_CSUM) {
+               ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_TXCOE_CTL, 1, 1, &tmp);
+               tmp ^= AX_TXCOE_TCP | AX_TXCOE_UDP;
+               ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_TXCOE_CTL, 1, 1, &tmp);
+
+               net->features ^= NETIF_F_IP_CSUM;
+       }
[...]

Isn't tmp going to be in little-endian byte order, so this doesn't work
correctly on a big-endian system?

There are a lot of reads and writes of 16-bit registers using
ax88179_{read,write}_cmd(); maybe you should add
ax88179_{read,write}_le16() to handle this specific case.

Ben.

Thank you, Ben. I will fix bugs and test it on a big-endian system.

Freddy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to