Patchwork [net-next-2.6] net: Changes to support the "-f" option of ethtool.

login
register
mail settings
Submitter Ajit Khaparde
Date Aug. 5, 2009, 12:32 p.m.
Message ID <20090805123211.GB6592@serverengines.com>
Download mbox | patch
Permalink /patch/30787/
State Rejected
Delegated to: David Miller
Headers show

Comments

Ajit Khaparde - Aug. 5, 2009, 12:32 p.m.
This patch adds kernel support for the new "-f" option in ethtool,
This will allow a firmware image to be flashed to a network device.

Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
---
 include/linux/ethtool.h |    3 +++
 net/core/ethtool.c      |   13 +++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)
Ajit Khaparde - Aug. 5, 2009, 12:37 p.m.
The patch intends to use the request_firmware() interface to pick the
firmware image file. 
I had missed this in the original post. Thanks.

On 05/08/09 18:02 +0530, Ajit Khaparde wrote:
> This patch adds kernel support for the new "-f" option in ethtool,
> This will allow a firmware image to be flashed to a network device.
> 
> Signed-off-by: Ajit Khaparde <ajitk@serverengines.com>
> ---
>  include/linux/ethtool.h |    3 +++
>  net/core/ethtool.c      |   13 +++++++++++++
>  2 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 90c4a36..9b92f44 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -489,6 +489,7 @@ struct ethtool_ops {
>  	int	(*get_stats_count)(struct net_device *);/* use get_sset_count */
>  	int	(*get_rxnfc)(struct net_device *, struct ethtool_rxnfc *, void *);
>  	int	(*set_rxnfc)(struct net_device *, struct ethtool_rxnfc *);
> +	int     (*flash_device)(struct net_device *);
>  };
>  #endif /* __KERNEL__ */
>  
> @@ -545,6 +546,8 @@ struct ethtool_ops {
>  #define	ETHTOOL_GRXCLSRLALL	0x00000030 /* Get all RX classification rule */
>  #define	ETHTOOL_SRXCLSRLDEL	0x00000031 /* Delete RX classification rule */
>  #define	ETHTOOL_SRXCLSRLINS	0x00000032 /* Insert RX classification rule */
> +#define	ETHTOOL_FLASHDEV	0x00000033 /* Flash firmware to device
> +					    * (ethtool_value). */
>  
>  /* compatibility with older code */
>  #define SPARC_ETH_GSET		ETHTOOL_GSET
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 44e5711..0524201 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -898,6 +898,16 @@ static int ethtool_set_value(struct net_device *dev, char __user *useraddr,
>  	return actor(dev, edata.data);
>  }
>  
> +static int ethtool_flash_device(struct net_device *dev)
> +{
> +	int err;
> +
> +	if (!dev->ethtool_ops->flash_device)
> +		return -EOPNOTSUPP;
> +
> +	return dev->ethtool_ops->flash_device(dev);
> +}
> +
>  /* The main entry point in this file.  Called from net/core/dev.c */
>  
>  int dev_ethtool(struct net *net, struct ifreq *ifr)
> @@ -1111,6 +1121,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
>  	case ETHTOOL_SGRO:
>  		rc = ethtool_set_gro(dev, useraddr);
>  		break;
> +	case ETHTOOL_FLASHDEV:
> +		rc = ethtool_flash_device(dev);
> +		break;
>  	default:
>  		rc = -EOPNOTSUPP;
>  	}
> -- 
> 1.6.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings - Aug. 5, 2009, 1:06 p.m.
On Wed, 2009-08-05 at 18:07 +0530, Ajit Khaparde wrote:
> The patch intends to use the request_firmware() interface to pick the
> firmware image file. 
> I had missed this in the original post. Thanks.
[...]

Thanks for the clarification.

request_firmware() is meant for loading firmware that is stored in
volatile memory (RAM) on the device and therefore needs to be installed
on the host.

When the firmware is stored in flash on the device, updates only need to
be used once, and there should be no need to install them on the host.
So request_firmware() does not seem suitable.

I believe the ethtool EEPROM commands were meant for updating firmware
on NICs.  Although they assume random access and so are unsuitable for
flash-based firmware, they might be a better model for adding flash
update commands.  However, this command set is already available through
the MTD device class, which is what we use for firmware update as far as
possible.  Not only does this allow for an arbitrary number of separate
firmware partitions per network device, but it can be used in an
out-of-tree driver for older kernel versions.

Ben.
David Miller - Aug. 5, 2009, 8:17 p.m.
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 05 Aug 2009 14:06:07 +0100

> request_firmware() is meant for loading firmware that is stored in
> volatile memory (RAM) on the device and therefore needs to be installed
> on the host.
> 
> When the firmware is stored in flash on the device, updates only need to
> be used once, and there should be no need to install them on the host.
> So request_firmware() does not seem suitable.
> 
> I believe the ethtool EEPROM commands were meant for updating firmware
> on NICs.  Although they assume random access and so are unsuitable for
> flash-based firmware, they might be a better model for adding flash
> update commands.  However, this command set is already available through
> the MTD device class, which is what we use for firmware update as far as
> possible.  Not only does this allow for an arbitrary number of separate
> firmware partitions per network device, but it can be used in an
> out-of-tree driver for older kernel versions.

I mostly agree with this.  Either use MTD or create an ethtool
operation that explicitly specifies the file to flash onto the
chip.  If you want, I suppose you can take no explicit file
specification to mean "whatever firmware was loaded into the
chip by the driver and is running right now"

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ajit Khaparde - Aug. 10, 2009, 9:04 a.m.
On 05/08/09 13:17 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Wed, 05 Aug 2009 14:06:07 +0100
> 
> > request_firmware() is meant for loading firmware that is stored in
> > volatile memory (RAM) on the device and therefore needs to be installed
> > on the host.
> > 
> > When the firmware is stored in flash on the device, updates only need to
> > be used once, and there should be no need to install them on the host.
> > So request_firmware() does not seem suitable.
> > 
> > I believe the ethtool EEPROM commands were meant for updating firmware
> > on NICs.  Although they assume random access and so are unsuitable for
> > flash-based firmware, they might be a better model for adding flash
> > update commands.  However, this command set is already available through
> > the MTD device class, which is what we use for firmware update as far as
> > possible.  Not only does this allow for an arbitrary number of separate
> > firmware partitions per network device, but it can be used in an
> > out-of-tree driver for older kernel versions.
> 
> I mostly agree with this.  Either use MTD or create an ethtool
> operation that explicitly specifies the file to flash onto the
> chip.  If you want, I suppose you can take no explicit file
> specification to mean "whatever firmware was loaded into the
> chip by the driver and is running right now"
Thanks. I will go with the ethtool option which would also take the 
filename and flash it onto the chip using the request_firmware() path.

-Ajit
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 90c4a36..9b92f44 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -489,6 +489,7 @@  struct ethtool_ops {
 	int	(*get_stats_count)(struct net_device *);/* use get_sset_count */
 	int	(*get_rxnfc)(struct net_device *, struct ethtool_rxnfc *, void *);
 	int	(*set_rxnfc)(struct net_device *, struct ethtool_rxnfc *);
+	int     (*flash_device)(struct net_device *);
 };
 #endif /* __KERNEL__ */
 
@@ -545,6 +546,8 @@  struct ethtool_ops {
 #define	ETHTOOL_GRXCLSRLALL	0x00000030 /* Get all RX classification rule */
 #define	ETHTOOL_SRXCLSRLDEL	0x00000031 /* Delete RX classification rule */
 #define	ETHTOOL_SRXCLSRLINS	0x00000032 /* Insert RX classification rule */
+#define	ETHTOOL_FLASHDEV	0x00000033 /* Flash firmware to device
+					    * (ethtool_value). */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 44e5711..0524201 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -898,6 +898,16 @@  static int ethtool_set_value(struct net_device *dev, char __user *useraddr,
 	return actor(dev, edata.data);
 }
 
+static int ethtool_flash_device(struct net_device *dev)
+{
+	int err;
+
+	if (!dev->ethtool_ops->flash_device)
+		return -EOPNOTSUPP;
+
+	return dev->ethtool_ops->flash_device(dev);
+}
+
 /* The main entry point in this file.  Called from net/core/dev.c */
 
 int dev_ethtool(struct net *net, struct ifreq *ifr)
@@ -1111,6 +1121,9 @@  int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_SGRO:
 		rc = ethtool_set_gro(dev, useraddr);
 		break;
+	case ETHTOOL_FLASHDEV:
+		rc = ethtool_flash_device(dev);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}