Re: [PATCH v2] ethernet driver for the WIZnet W5300 chip

On Wed, 2012-03-21 at 12:20 +0600, Mike Sinkovsky wrote:

21.03.2012 0:08, Ben Hutchings написал:

quoted

+static void w5300_get_drvinfo(struct net_device *ndev,
+			      struct ethtool_drvinfo *info)
+{
+	strlcpy(info->driver, DRV_NAME, sizeof(info->driver));
+	strlcpy(info->version, DRV_VERSION, sizeof(info->version));
+	strlcpy(info->fw_version, "N/A", sizeof(info->fw_version));
[...]

Nitpick: if you don't have firmware, don't set fw_version at all.

Ben.

Then ethtool prints empty string as firmware version, I think "N/A" is 
more intuitive.

Well, we can replace it with "N/A" in ethtool if that's what people want
to see.  But it should not be necessary for every driver that can't
report a firmware version to write this special string.

Can will remove, if it is preferred way.

It is, and the various other drivers that used this string have recently
been changed.

But I have a question to networking guru's:

This chip have FRAME_SIZE limited to 1514, including eth header.
So, for 802.1Q vlan packets mtu must be set to 1496.

For now we handle this from userspace, but I think this is wrong in general
- handling hardware bugs and limitations is kernels job.

Can driver somehow limit mtu for slave vlan devices to 1496, but still 
use 1500
for plain ethernet?
->ndo_change_mtu is called for master device only, not for it's slaves.

I don't think it's possible to do this at the moment.  You can only set
NETIF_F_VLAN_CHALLENGED, which unfortunately disables VLAN sub-devices
entirely.

(and sorry for my russian english, just in case)

Not to worry - your writing is quite clear.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.