From: Magnus Damm <[email protected]>
Update the in-kernel hotplug example script to work
properly with recent kernels. Without this fix the
script may load the firmware twice - both at "add"
and "remove" time.
The second load only triggers in the case when multiple
firmware images are used. A good example is the b43
driver which does not work properly without this fix.
While at it, make sure sysfs files are removed.
Signed-off-by: Magnus Damm <[email protected]>
---
Documentation/firmware_class/hotplug-script | 17 +++++++++--------
drivers/base/firmware_class.c | 6 +++++-
2 files changed, 14 insertions(+), 9 deletions(-)
--- 0001/Documentation/firmware_class/hotplug-script
+++ work/Documentation/firmware_class/hotplug-script 2010-06-25 14:46:17.000000000 +0900
@@ -6,11 +6,12 @@
HOTPLUG_FW_DIR=/usr/lib/hotplug/firmware/
-echo 1 > /sys/$DEVPATH/loading
-cat $HOTPLUG_FW_DIR/$FIRMWARE > /sys/$DEVPATH/data
-echo 0 > /sys/$DEVPATH/loading
-
-# To cancel the load in case of error:
-#
-# echo -1 > /sys/$DEVPATH/loading
-#
+if [ "$SUBSYSTEM" == "firmware" -a "$ACTION" == "add" ]; then
+ if [ -f $HOTPLUG_FW_DIR/$FIRMWARE ]; then
+ echo 1 > /sys/$DEVPATH/loading
+ cat $HOTPLUG_FW_DIR/$FIRMWARE > /sys/$DEVPATH/data
+ echo 0 > /sys/$DEVPATH/loading
+ else
+ echo -1 > /sys/$DEVPATH/loading
+ fi
+fi
--- 0001/drivers/base/firmware_class.c
+++ work/drivers/base/firmware_class.c 2010-06-25 14:48:53.000000000 +0900
@@ -508,7 +508,7 @@ static int fw_setup_device(struct firmwa
retval = device_create_file(f_dev, &dev_attr_loading);
if (retval) {
dev_err(device, "%s: device_create_file failed\n", __func__);
- goto error_unreg;
+ goto error_unreg2;
}
if (uevent)
@@ -516,6 +516,8 @@ static int fw_setup_device(struct firmwa
*dev_p = f_dev;
goto out;
+error_unreg2:
+ sysfs_remove_bin_file(&f_dev->kobj, &fw_priv->attr_data);
error_unreg:
device_unregister(f_dev);
out:
@@ -578,6 +580,8 @@ _request_firmware(const struct firmware
}
fw_priv->fw = NULL;
mutex_unlock(&fw_lock);
+ sysfs_remove_bin_file(&f_dev->kobj, &fw_priv->attr_data);
+ device_remove_file(f_dev, &dev_attr_loading);
device_unregister(f_dev);
goto out;
On Fri, Jun 25, 2010 at 05:55:11PM +0900, Magnus Damm wrote:
> From: Magnus Damm <[email protected]>
>
> Update the in-kernel hotplug example script to work
> properly with recent kernels. Without this fix the
> script may load the firmware twice - both at "add"
> and "remove" time.
>
> The second load only triggers in the case when multiple
> firmware images are used. A good example is the b43
> driver which does not work properly without this fix.
>
> While at it, make sure sysfs files are removed.
Hm, the documentation update applied, but this second part didn't. Care
to redo it against the linux-next tree and resend it as a separate
patch? I've split this up and applied the documentation part now, so
you don't have to redo that.
thanks,
greg k-h
On Sat, Jun 26, 2010 at 3:22 AM, Greg KH <[email protected]> wrote:
> On Fri, Jun 25, 2010 at 05:55:11PM +0900, Magnus Damm wrote:
>> From: Magnus Damm <[email protected]>
>>
>> Update the in-kernel hotplug example script to work
>> properly with recent kernels. Without this fix the
>> script may load the firmware twice - both at "add"
>> and "remove" time.
>>
>> The second load only triggers in the case when multiple
>> firmware images are used. A good example is the b43
>> driver which does not work properly without this fix.
>>
>> While at it, make sure sysfs files are removed.
>
> Hm, the documentation update applied, but this second part didn't. ?Care
> to redo it against the linux-next tree and resend it as a separate
> patch? ?I've split this up and applied the documentation part now, so
> you don't have to redo that.
Thanks a lot for splitting up the patch. I don't mind redoing the
second part of the patch, but I'm not sure if it's actually needed to
prevent memory leaks. This piece of code serves as an example:
+error_unreg2:
+ sysfs_remove_bin_file(&f_dev->kobj, &fw_priv->attr_data);
error_unreg:
device_unregister(f_dev);
Perhaps device_unregister() call above already removes the kobj which
makes the added sysfs_remove_bin_file() unnecessary?
Cheers,
/ magnus
On Mon, Jun 28, 2010 at 03:17:22PM +0900, Magnus Damm wrote:
> On Sat, Jun 26, 2010 at 3:22 AM, Greg KH <[email protected]> wrote:
> > On Fri, Jun 25, 2010 at 05:55:11PM +0900, Magnus Damm wrote:
> >> From: Magnus Damm <[email protected]>
> >>
> >> Update the in-kernel hotplug example script to work
> >> properly with recent kernels. Without this fix the
> >> script may load the firmware twice - both at "add"
> >> and "remove" time.
> >>
> >> The second load only triggers in the case when multiple
> >> firmware images are used. A good example is the b43
> >> driver which does not work properly without this fix.
> >>
> >> While at it, make sure sysfs files are removed.
> >
> > Hm, the documentation update applied, but this second part didn't. ?Care
> > to redo it against the linux-next tree and resend it as a separate
> > patch? ?I've split this up and applied the documentation part now, so
> > you don't have to redo that.
>
> Thanks a lot for splitting up the patch. I don't mind redoing the
> second part of the patch, but I'm not sure if it's actually needed to
> prevent memory leaks. This piece of code serves as an example:
>
> +error_unreg2:
> + sysfs_remove_bin_file(&f_dev->kobj, &fw_priv->attr_data);
> error_unreg:
> device_unregister(f_dev);
>
> Perhaps device_unregister() call above already removes the kobj which
> makes the added sysfs_remove_bin_file() unnecessary?
>
It is so happens that device_unregister() does clean up leftover sysfs
attributes but we should not rely on that happen in the future. That
said I glanced at linux-next and it appears that we do clean up the
attributes properly now.
Thanks.
--
Dmitry