ethtool: Add "-f" option to flash a firmware image from the specified file to a device.
Subsystems (2)
PATCH 1/1 ethtool: Add "-f" option to flash a firmware image from the specified file to a device.
This patch adds a new "-f" option to the ethtool utility
to flash a firmware image specified by a file, to a network device. The filename is passed to the network driver which will flash the image on the chip using the request_firmware path.
The region to be flashed - like redboot, phy can be specified as an argument, which will be passed to the driver.
More options for other flash regions can be added in future.
The default behavior is to flash all the regions on the chip.
Usage:
ethtool -f <interface name> <filename of firmware image>
ethtool -f <interface name> <filename of firmware image> [ all|redboot|phy ]
--- ethtool-copy.h | 16 ++++++++++++ ethtool.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletions(-)
ethtool-copy.h +16 -0
@@ -272,6 +272,21 @@ struct ethtool_perm_addr {__u8 data[0 ]; }; +#define ETHTOOL_FLASH_MAX_FILENAME 128 +#define ETHTOOL_FLASH_OP_TYPE_SIZE 32 +enum ethtool_flash_op_type { +ETHTOOL_FLASH_ALL =0 , +ETHTOOL_FLASH_PHY , +ETHTOOL_FLASH_REDBOOT , +}; + +/* for passing firmware flashing related parameters */ +struct ethtool_flash { +__u32 cmd; +__u32 op_type; +char data[256 ]; +}; +/* boolean flags controlling per-interface behavior characteristics. * When reading, the flag indicates whether or not a certain behavior * is enabled/present. When writing, the flag indicates whether
@@ -338,6 +353,7 @@ struct ethtool_rxnfc {#define ETHTOOL_SRXFH 0x0000002a/* Set RX flow hash configuration */ #define ETHTOOL_GGRO 0x0000002b/* Get GRO enable (ethtool_value) */ #define ETHTOOL_SGRO 0x0000002c/* Set GRO enable (ethtool_value) */ +#define ETHTOOL_FLASHDEV 0x00000033/* Flash firmware to device */ /* compatibility with older code */ #define SPARC_ETH_GSET ETHTOOL_GSET
ethtool.c +70 -1
125 lines not on this page · raw message
Signature · 1 line
1.6.0.4
Quoted 25 lines · Ajit Khaparde
This patch adds a new "-f" option to the ethtool utility
to flash a firmware image specified by a file, to a network device. The filename is passed to the network driver which will flash the image on the chip using therequest_firmwarepath.
The region to be flashed - like redboot, phy can be specified as an argument, which will be passed to the driver.
More options for other flash regions can be added in future.
The default behavior is to flash all the regions on the chip.Usage:
ethtool -f <interface name> <filename of firmware image>ethtool -f <interface name> <filename of firmware image> [ all|redboot|phy ]
Signed-off-by Ajit Khaparde <[email protected]>--- ethtool-copy.h | 16 ++++++++++++ ethtool.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletions(-)ethtool-copy.h +0
-0
ethtool-copy.h struct ethtool_perm_addr { · line 276
Quoted 5 lines
ethtool-copy.h +2
-0@@ -272,6 +272,21 @@ struct ethtool_perm_addr {__u8 data[0 ]; }; +#define ETHTOOL_FLASH_MAX_FILENAME 128 +#define ETHTOOL_FLASH_OP_TYPE_SIZE 32
These are missing from your patch to the in-kernel ethtool.h. What do
they mean? They are inconsistent with the ethtool_flash::data field.
[...]
ethtool-copy.h static void parse_cmdline(int argc, char **argp) · line 529
Quoted 6 lines
ethtool-copy.h +2
-0@@ -516,6 +525,10 @@ static void parse_cmdline(int argc, char **argp)if (phys_id_time <0 )show_usage (1 );break ; + }else if (mode ==MODE_FLASHDEV ) { +sprintf (flash_file,"%s" , argp[i]);
This is missing a length check.
[...]
Quoted 6 lines
ethtool-copy.h +1
-0@@ -2398,6 +2424,49 @@ static int do_grxclass(int fd, struct ifreq *ifr)return 0 ; } +static int do_flash (int fd,struct ifreq *ifr)
ethtool-copy.h static int do_grxclass(int fd, struct ifreq · line 2438
Quoted 10 lines
ethtool-copy.h +10
-0+{ +struct ethtool_flash efl; +int err; +char path[ETHTOOL_FLASH_MAX_FILENAME *2 ] ="/lib/firmware/" ; +FILE *f; + +if (flash <0 ) { +fprintf (stdout,"Missing filename argument\n" ); +show_usage (1 ); +return -EPERM ;
This doesn't follow the ethtool convention for error codes, which is to use unique positive numbers.
Quoted 5 lines
ethtool-copy.h +5
-0+ } + +strcat (path, flash_file); + +if (strlen (flash_file) >ETHTOOL_FLASH_MAX_FILENAME ) {
ethtool-copy.h static int do_grxclass(int fd, struct ifreq · line 2453
Quoted 10 lines
ethtool-copy.h +10
-0+fprintf (stdout,"Filename too long\n" ); +return -EINVAL ; + } + + f =fopen (path,"r" ); +if (!f) { +perror (path); +return -ENOENT ; + } +fclose (f);
request_firmware() relies on a user-space agent to load firmware; normally this is udev's firmware_agent which can be configured to look in directories other than /lib/firmware/. So this check is too strict. The driver should report back any error from request_firmware(), so it should not be necessary to check here at all.
ethtool-copy.h static int do_grxclass(int fd, struct ifreq · line 2455
Quoted 2 lines
ethtool-copy.h +2
-0+ efl.cmd =ETHTOOL_FLASHDEV ; +sprintf (efl.data,"%s" , flash_file);
[...]
What's wrong with strcpy() anyway?
Ben.
Signature · 3 lines
Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.