On Wed, 24 Apr 2024 16:30:22 +0300 Danielle Ratson wrote: > +static int > +module_flash_fw_schedule(struct net_device *dev, const char *file_name, > + struct ethtool_module_fw_flash_params *params, > + struct netlink_ext_ack *extack) > +{ > + struct ethtool_module_fw_flash *module_fw; > + int err; > + > + err = __module_flash_fw_schedule(dev, extack); > + if (err < 0) > + return err; Basic dev validation should probably be called directly from ethnl_act_module_fw_flash() rather than two functions down. > + module_fw = kzalloc(sizeof(*module_fw), GFP_KERNEL); > + if (!module_fw) > + return -ENOMEM; > + > + module_fw->params = *params; > + err = request_firmware_direct(&module_fw->fw, file_name, &dev->dev); > + if (err) { > + NL_SET_ERR_MSG(extack, > + "Failed to request module firmware image"); > + goto err_request_firmware; Please name the labels after the actions they perform. > + } > + > + err = module_flash_fw_work_init(module_fw, dev, extack); > + if (err < 0) { > + NL_SET_ERR_MSG(extack, > + "Flashing module firmware is not supported by this device"); This overwrites the more accurate extack msg already set by module_flash_fw_work_init() > + goto err_work_init; > + }