Hi Michal,
thanks for review - see comments
On 17-12-05 01:30 PM, Michal Kubecek wrote:
On Tue, Dec 05, 2017 at 12:53:23PM -0800, Scott Branden wrote:
Add ETHTOOL_RESET support via --reset command.
ie. ethtool --reset DEVNAME <flagname(s)>
flagnames currently match the ETH_RESET_xxx names:
mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
Alternatively, you can specific component bitfield directly using
ethtool --reset DEVNAME flags %x
IMHO it would be more consistent with e.g. msglvl without the keyword
"flags".
I don't see the consistency in ethtool of specifying a number without a
keyword in front of it.
I can only find --set-dump specify a number?
Others have keyword and number. msglvl is the keyword after specifying
-s - same as flags is the keyword I use after specifying --reset.
Whichever convention is decided for the --reset command I will change
the patch to though.
It would be also nice to provide a symbolic way to specify the
shared flags.
I'll change to allow -shared to be added to the end of each component
specified to use the shared bit.
IE. mgmt-shared, irq-shared, dma-shared ?
+static int do_reset(struct cmd_context *ctx)
+{
+ struct ethtool_value resetinfo;
+ int argc = ctx->argc;
+ char **argp = ctx->argp;
+ int i;
+
+ if (argc == 0)
+ exit_bad_args();
+
+ resetinfo.data = 0;
+
+ for (i = 0; i < argc; i++) {
+ if (!strcmp(argp[i], "flags")) {
+ __u32 flags;
+
+ i++;
+ if (i >= argc)
+ exit_bad_args();
+ flags = strtoul(argp[i], NULL, 0);
+ if (flags == 0)
+ exit_bad_args();
+ else
+ resetinfo.data |= flags;
+ } else if (!strcmp(argp[i], "mgmt")) {
+ resetinfo.data |= ETH_RESET_MGMT;
+ } else if (!strcmp(argp[i], "irq")) {
+ resetinfo.data |= ETH_RESET_IRQ;
+ } else if (!strcmp(argp[i], "dma")) {
+ resetinfo.data |= ETH_RESET_DMA;
+ } else if (!strcmp(argp[i], "filter")) {
+ resetinfo.data |= ETH_RESET_FILTER;
+ } else if (!strcmp(argp[i], "offload")) {
+ resetinfo.data |= ETH_RESET_OFFLOAD;
+ } else if (!strcmp(argp[i], "mac")) {
+ resetinfo.data |= ETH_RESET_MAC;
+ } else if (!strcmp(argp[i], "phy")) {
+ resetinfo.data |= ETH_RESET_PHY;
+ } else if (!strcmp(argp[i], "ram")) {
+ resetinfo.data |= ETH_RESET_RAM;
+ } else if (!strcmp(argp[i], "ap")) {
+ resetinfo.data |= ETH_RESET_AP;
+ } else if (!strcmp(argp[i], "dedicated")) {
+ resetinfo.data |= ETH_RESET_DEDICATED;
+ } else if (!strcmp(argp[i], "all")) {
+ resetinfo.data |= ETH_RESET_ALL;
+ } else {
+ exit_bad_args();
+ }
+ }
+
+ resetinfo.cmd = ETHTOOL_RESET;
+
+ if (send_ioctl(ctx, &resetinfo)) {
+ perror("Cannot issue RESET");
+ return 1;
+ }
+ fprintf(stdout, "RESET 0x%x issued\n", resetinfo.data);
According to documentation, driver is supposed to clear the flags
corresponding to components which were reset so that what is left are
those which were _not_ reset.
I'll move the print above the send_ioctl.
Michal Kubecek
