[PATCH] libertas: add sysfs hooks to update boot2 and persistent firmware

Brian Cavagnolo brian at cozybit.com
Tue Jun 3 21:17:23 EDT 2008


Hello,

On Tue, Jun 3, 2008 at 11:38 AM, Dan Williams <dcbw at redhat.com> wrote:
> Am I correct in assuming that the runtime firmware implements _all_ the
> same boot commands that the boot2 firmware does, including UPDATE_BOOT2
> (flash new boot2 to EEPROM), FW_BY_USB (execute uploaded firmware but do
> not flash), and UPDATE_FW (flash new runtime firmware to EEPROM)?  Or
> does the runtime firmware only support flashing, not re-executing itself
> with FW_BY_USB?

The latter is true.  The runtime firmware only supports the UPDATE_FW
and UPDATE_BOOT2.  Supporting FW_BY_USB in firmware would be pretty
tricky :)

> Are there _any_ side effects of sending the boot command to a running
> firmware?  We didn't have to care about these before because the module
> wasn't loaded, and when you were done flashing you were supposed to load
> the module which would reset the card.

There should be no side effects.  However, the 5.110.X firmware (and
the 5.126.X on non-active antennas) freezes the USB stack if we send
the UPDATE_FW or UPDATE_BOOT2 commands.  We believe this is a firmware
bug and will bring it up with Marvell.  But ultimately there should be
no side affects regardless of the hardware.

> You'll need to grab priv->driver_lock before calling if_usb_prog_fw().
> If you don't, anyone can issue WEXT commands or call 'ifconfig eth0 up'
> and start inserting random commands into the command queue.  I'm not
> even sure grabbing priv->driver_lock will help you here, because the
> firmware upload code was always executed _before_ there was a network
> interface registered with the system, and thus doesn't need to do _any_
> locking at all.  So just grabbing priv->driver_lock doesn't necessarily
> stop other USB commands from getting issued, though in practice since
> you have to have priv->driver_lock held before grabbing a free cmd_node
> from the driver core, this would stop the driver core from sending
> commands during a firmware upload.  So the behavior you'd get would be
> that an iwconfig call would block until the firmware upload was
> complete, then immediately send the pending command to the firmware.
> Hence my question about side-effects above.

This is a valid concern that is not addressed in my patch.  I don't
think grabbing driver_lock is a good solution, though, because the fw
prog is likely to take a while and we sleep all over the place.  I
think it might be better to manipulate priv->cur_cmd and
priv->dnld_sent in a suitable fashion in the sysfs functions.  I'll
investigate and resubmit the patches.  Any input would be appreciated.

> Does the loaded runtime firmware just write the new firmware to EEPROM
> and then continue as normal?  If so, I assume that the new firmware is
> not expected to be active until (a) reboot, (b) USB port reset, (c) OLPC
> EC-triggered reset, (d) CMD_802_11_RESET, or (e) hotplug of the active
> antenna.

The firmware images that get flashed are stand-alone images for the
active antennas released at [1], not regular USB firmware images.  The
image that gets flashed will only run when the active antenna is
powered up without a host.  If you inadvertently flash an active
antenna with the wrong firmware image, you can recover it by simply
reflashing.  In contrast, the boot2 that gets flashed is the only
boot2.

[1] http://www.laptop.org/teamwiki/index.php/Image:Standalone-firmware.tgz

> The code probably shouldn't allow updating the firmware if the battery
> is low.  Unfortunately, this isn't something that can or should be
> stuffed into the driver, which is one great benefit of doing the update
> in userspace.  Second, you probably want to make sure that the system is
> prohibited from entering the suspend state during a firmware update.
> This is also probably better done via userspace.  Obviously neither of
> these two is implemented in the current userspace tool but they would be
> nice, and not something that can be done from the kernel side in any
> upstream-able manner.

The risk of corrupting the firmware image is not major.  The user can
just reflash when power is available.  However, if power fails or if
the system sleeps during a boot2 update, the device will be bricked.
The only defense against this is making the sysfs hook write-only by
root.

Ciao,
Brian

>> Signed-off-by: Brian Cavagnolo <brian at cozybit.com>
>> ---
>>  drivers/net/wireless/libertas/if_usb.c |   65 ++++++++++++++++++++++++++++++++
>>  1 files changed, 65 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
>> index 91413a6..6a32f37 100644
>> --- a/drivers/net/wireless/libertas/if_usb.c
>> +++ b/drivers/net/wireless/libertas/if_usb.c
>> @@ -46,6 +46,62 @@ static void if_usb_free(struct if_usb_card *cardp);
>>  static int if_usb_submit_rx_urb(struct if_usb_card *cardp);
>>  static int if_usb_reset_device(struct if_usb_card *cardp);
>>
>> +/* sysfs hooks */
>> +
>> +/**
>> + *  Set function to write firmware to device's persistent memory
>> + */
>> +static ssize_t if_usb_firmware_set(struct device *dev,
>> +             struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> +     struct lbs_private *priv = to_net_dev(dev)->priv;
>> +     struct if_usb_card *cardp = priv->card;
>> +     char fwname[FIRMWARE_NAME_MAX];
>> +     int ret;
>> +
>> +     sscanf(buf, "%29s", fwname); /* FIRMWARE_NAME_MAX - 1 = 29 */
>> +     ret = if_usb_prog_firmware(cardp, fwname, BOOT_CMD_UPDATE_FW);
>> +     if (ret == 0)
>> +             return count;
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * lbs_fw attribute to be exported per ethX interface through sysfs
>> + * (/sys/class/net/ethX/lbs_fw).  Use this like so to write firmware to the
>> + * device's persistent memory:
>> + * echo usb8388-5.126.0.p5.bin > /sys/class/net/ethX/lbs_fw
>> + */
>> +static DEVICE_ATTR(lbs_fw, 0200, NULL, if_usb_firmware_set);
>> +
>> +/**
>> + *  Set function to write firmware to device's persistent memory
>> + */
>> +static ssize_t if_usb_boot2_set(struct device *dev,
>> +             struct device_attribute *attr, const char *buf, size_t count)
>> +{
>> +     struct lbs_private *priv = to_net_dev(dev)->priv;
>> +     struct if_usb_card *cardp = priv->card;
>> +     char fwname[FIRMWARE_NAME_MAX];
>> +     int ret;
>> +
>> +     sscanf(buf, "%29s", fwname); /* FIRMWARE_NAME_MAX - 1 = 29 */
>> +     ret = if_usb_prog_firmware(cardp, fwname, BOOT_CMD_UPDATE_BOOT2);
>> +     if (ret == 0)
>> +             return count;
>> +
>> +     return ret;
>> +}
>> +
>> +/**
>> + * lbs_boot2 attribute to be exported per ethX interface through sysfs
>> + * (/sys/class/net/ethX/lbs_boot2).  Use this like so to write firmware to the
>> + * device's persistent memory:
>> + * echo usb8388-5.126.0.p5.bin > /sys/class/net/ethX/lbs_boot2
>> + */
>> +static DEVICE_ATTR(lbs_boot2, 0200, NULL, if_usb_boot2_set);
>> +
>>  /**
>>   *  @brief  call back function to handle the status of the URB
>>   *  @param urb               pointer to urb structure
>> @@ -246,6 +302,12 @@ static int if_usb_probe(struct usb_interface *intf,
>>       usb_get_dev(udev);
>>       usb_set_intfdata(intf, cardp);
>>
>> +     if (device_create_file(&priv->dev->dev, &dev_attr_lbs_fw))
>> +             lbs_pr_err("cannot register lbs_fw attribute\n");
>> +
>> +     if (device_create_file(&priv->dev->dev, &dev_attr_lbs_boot2))
>> +             lbs_pr_err("cannot register lbs_boot2 attribute\n");
>> +
>>       return 0;
>>
>>  err_start_card:
>> @@ -271,6 +333,9 @@ static void if_usb_disconnect(struct usb_interface *intf)
>>
>>       lbs_deb_enter(LBS_DEB_MAIN);
>>
>> +     device_remove_file(&priv->dev->dev, &dev_attr_lbs_boot2);
>> +     device_remove_file(&priv->dev->dev, &dev_attr_lbs_fw);
>> +
>>       cardp->surprise_removed = 1;
>>
>>       if (priv) {
>
>



More information about the Devel mailing list