Message ID | 4AACC94E.9040208@tuffmail.co.uk (mailing list archive) |
---|---|
State | RFC, archived |
Headers | show |
On Sun, 2009-09-13 at 18:28 +0800, Alan Jenkins wrote: > Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk> It sounds reasonable although we don't fail in calling the function of acpi_fan_add_fs. > --- > drivers/acpi/fan.c | 8 ++++++-- > 1 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c > index 947556e..eb1511e 100644 > --- a/drivers/acpi/fan.c > +++ b/drivers/acpi/fan.c > @@ -283,13 +283,17 @@ static int acpi_fan_add(struct acpi_device *device) > > result = acpi_fan_add_fs(device); > if (result) > - goto end; > + goto unregister; > > printk(KERN_INFO PREFIX "%s [%s] (%s)\n", > acpi_device_name(device), acpi_device_bid(device), > !device->power.state ? "on" : "off"); At the same time we should return directly if the result is zero. But in this patch it seems that it will also remove the sysfs I/F link when the result is zero. Thanks. > > - end: > +unregister: > + sysfs_remove_link(&device->dev.kobj, "thermal_cooling"); > + sysfs_remove_link(&cdev->device.kobj, "device"); > + thermal_cooling_device_unregister(cdev); > +end: > return result; > } > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
ykzhao wrote: > On Sun, 2009-09-13 at 18:28 +0800, Alan Jenkins wrote: > >> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk> >> > It sounds reasonable although we don't fail in calling the function of > acpi_fan_add_fs. > Agreed, this applies to ENOMEM or other weird internal errors only. >> --- >> drivers/acpi/fan.c | 8 ++++++-- >> 1 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c >> index 947556e..eb1511e 100644 >> --- a/drivers/acpi/fan.c >> +++ b/drivers/acpi/fan.c >> @@ -283,13 +283,17 @@ static int acpi_fan_add(struct acpi_device *device) >> >> result = acpi_fan_add_fs(device); >> if (result) >> - goto end; >> + goto unregister; >> >> printk(KERN_INFO PREFIX "%s [%s] (%s)\n", >> acpi_device_name(device), acpi_device_bid(device), >> !device->power.state ? "on" : "off"); >> > > At the same time we should return directly if the result is zero. > But in this patch it seems that it will also remove the sysfs I/F link > when the result is zero. > > Thanks. > Eek, thanks for pointing this out. I'll send a fixed version Alan -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c index 947556e..eb1511e 100644 --- a/drivers/acpi/fan.c +++ b/drivers/acpi/fan.c @@ -283,13 +283,17 @@ static int acpi_fan_add(struct acpi_device *device) result = acpi_fan_add_fs(device); if (result) - goto end; + goto unregister; printk(KERN_INFO PREFIX "%s [%s] (%s)\n", acpi_device_name(device), acpi_device_bid(device), !device->power.state ? "on" : "off"); - end: +unregister: + sysfs_remove_link(&device->dev.kobj, "thermal_cooling"); + sysfs_remove_link(&cdev->device.kobj, "device"); + thermal_cooling_device_unregister(cdev); +end: return result; }
Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk> --- drivers/acpi/fan.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html