diff mbox series

[1/5] ACPI: battery: Do not unload battery hooks on single error

Message ID 20220912125342.7395-2-W_Armin@gmx.de (mailing list archive)
State Changes Requested, archived
Headers show
Series platform/x86: dell: Add new dell-wmi-ddv driver | expand

Commit Message

Armin Wolf Sept. 12, 2022, 12:53 p.m. UTC
Currently, battery hooks are being unloaded if they return
an error when adding a single battery.
This however also causes the removal of successfully added
hooks if they return -ENODEV for a single unsupported
battery.

Do not unload battery hooks in such cases since the hook
handles any cleanup actions.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/battery.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

--
2.30.2

Comments

Hans de Goede Sept. 19, 2022, 10:42 a.m. UTC | #1
Hi,

On 9/12/22 13:53, Armin Wolf wrote:
> Currently, battery hooks are being unloaded if they return
> an error when adding a single battery.
> This however also causes the removal of successfully added
> hooks if they return -ENODEV for a single unsupported
> battery.
> 
> Do not unload battery hooks in such cases since the hook
> handles any cleanup actions.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Maybe instead of removing all error checking, allow -ENODEV
and behave as before when the error is not -ENODEV ?

Otherwise we should probably make the add / remove callbacks
void to indicate that any errors are ignored.

Rafael, do you have any opinion on this?

Regards,

Hans

> ---
>  drivers/acpi/battery.c | 24 +++---------------------
>  1 file changed, 3 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 306513fec1e1..e59c261c7c59 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -724,20 +724,10 @@ void battery_hook_register(struct acpi_battery_hook *hook)
>  	 * its attributes.
>  	 */
>  	list_for_each_entry(battery, &acpi_battery_list, list) {
> -		if (hook->add_battery(battery->bat)) {
> -			/*
> -			 * If a add-battery returns non-zero,
> -			 * the registration of the extension has failed,
> -			 * and we will not add it to the list of loaded
> -			 * hooks.
> -			 */
> -			pr_err("extension failed to load: %s", hook->name);
> -			__battery_hook_unregister(hook, 0);
> -			goto end;
> -		}
> +		hook->add_battery(battery->bat);
>  	}
>  	pr_info("new extension: %s\n", hook->name);
> -end:
> +
>  	mutex_unlock(&hook_mutex);
>  }
>  EXPORT_SYMBOL_GPL(battery_hook_register);
> @@ -762,15 +752,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
>  	 * during the battery module initialization.
>  	 */
>  	list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
> -		if (hook_node->add_battery(battery->bat)) {
> -			/*
> -			 * The notification of the extensions has failed, to
> -			 * prevent further errors we will unload the extension.
> -			 */
> -			pr_err("error in extension, unloading: %s",
> -					hook_node->name);
> -			__battery_hook_unregister(hook_node, 0);
> -		}
> +		hook_node->add_battery(battery->bat);
>  	}
>  	mutex_unlock(&hook_mutex);
>  }
> --
> 2.30.2
>
Rafael J. Wysocki Sept. 19, 2022, 4:27 p.m. UTC | #2
On Mon, Sep 19, 2022 at 12:42 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 9/12/22 13:53, Armin Wolf wrote:
> > Currently, battery hooks are being unloaded if they return
> > an error when adding a single battery.
> > This however also causes the removal of successfully added
> > hooks if they return -ENODEV for a single unsupported
> > battery.
> >
> > Do not unload battery hooks in such cases since the hook
> > handles any cleanup actions.
> >
> > Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>
> Maybe instead of removing all error checking, allow -ENODEV
> and behave as before when the error is not -ENODEV ?
>
> Otherwise we should probably make the add / remove callbacks
> void to indicate that any errors are ignored.
>
> Rafael, do you have any opinion on this?

IMV this is not a completely safe change, because things may simply
not work in the cases in which an error is returned.

It would be somewhat better to use a special error code to indicate
"no support" (eg. -ENOTSUPP) and ignore that one only.

> > ---
> >  drivers/acpi/battery.c | 24 +++---------------------
> >  1 file changed, 3 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> > index 306513fec1e1..e59c261c7c59 100644
> > --- a/drivers/acpi/battery.c
> > +++ b/drivers/acpi/battery.c
> > @@ -724,20 +724,10 @@ void battery_hook_register(struct acpi_battery_hook *hook)
> >        * its attributes.
> >        */
> >       list_for_each_entry(battery, &acpi_battery_list, list) {
> > -             if (hook->add_battery(battery->bat)) {
> > -                     /*
> > -                      * If a add-battery returns non-zero,
> > -                      * the registration of the extension has failed,
> > -                      * and we will not add it to the list of loaded
> > -                      * hooks.
> > -                      */
> > -                     pr_err("extension failed to load: %s", hook->name);
> > -                     __battery_hook_unregister(hook, 0);
> > -                     goto end;
> > -             }
> > +             hook->add_battery(battery->bat);
> >       }
> >       pr_info("new extension: %s\n", hook->name);
> > -end:
> > +
> >       mutex_unlock(&hook_mutex);
> >  }
> >  EXPORT_SYMBOL_GPL(battery_hook_register);
> > @@ -762,15 +752,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
> >        * during the battery module initialization.
> >        */
> >       list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
> > -             if (hook_node->add_battery(battery->bat)) {
> > -                     /*
> > -                      * The notification of the extensions has failed, to
> > -                      * prevent further errors we will unload the extension.
> > -                      */
> > -                     pr_err("error in extension, unloading: %s",
> > -                                     hook_node->name);
> > -                     __battery_hook_unregister(hook_node, 0);
> > -             }
> > +             hook_node->add_battery(battery->bat);
> >       }
> >       mutex_unlock(&hook_mutex);
> >  }
> > --
> > 2.30.2
> >
>
Armin Wolf Sept. 19, 2022, 7:12 p.m. UTC | #3
Am 19.09.22 um 18:27 schrieb Rafael J. Wysocki:

> On Mon, Sep 19, 2022 at 12:42 PM Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 9/12/22 13:53, Armin Wolf wrote:
>>> Currently, battery hooks are being unloaded if they return
>>> an error when adding a single battery.
>>> This however also causes the removal of successfully added
>>> hooks if they return -ENODEV for a single unsupported
>>> battery.
>>>
>>> Do not unload battery hooks in such cases since the hook
>>> handles any cleanup actions.
>>>
>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>> Maybe instead of removing all error checking, allow -ENODEV
>> and behave as before when the error is not -ENODEV ?
>>
>> Otherwise we should probably make the add / remove callbacks
>> void to indicate that any errors are ignored.
>>
>> Rafael, do you have any opinion on this?
> IMV this is not a completely safe change, because things may simply
> not work in the cases in which an error is returned.
>
> It would be somewhat better to use a special error code to indicate
> "no support" (eg. -ENOTSUPP) and ignore that one only.

I would favor -ENODEV then, since it is already used by quiet a few drivers
to indicate a unsupported battery.

Armin Wolf

>>> ---
>>>   drivers/acpi/battery.c | 24 +++---------------------
>>>   1 file changed, 3 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>>> index 306513fec1e1..e59c261c7c59 100644
>>> --- a/drivers/acpi/battery.c
>>> +++ b/drivers/acpi/battery.c
>>> @@ -724,20 +724,10 @@ void battery_hook_register(struct acpi_battery_hook *hook)
>>>         * its attributes.
>>>         */
>>>        list_for_each_entry(battery, &acpi_battery_list, list) {
>>> -             if (hook->add_battery(battery->bat)) {
>>> -                     /*
>>> -                      * If a add-battery returns non-zero,
>>> -                      * the registration of the extension has failed,
>>> -                      * and we will not add it to the list of loaded
>>> -                      * hooks.
>>> -                      */
>>> -                     pr_err("extension failed to load: %s", hook->name);
>>> -                     __battery_hook_unregister(hook, 0);
>>> -                     goto end;
>>> -             }
>>> +             hook->add_battery(battery->bat);
>>>        }
>>>        pr_info("new extension: %s\n", hook->name);
>>> -end:
>>> +
>>>        mutex_unlock(&hook_mutex);
>>>   }
>>>   EXPORT_SYMBOL_GPL(battery_hook_register);
>>> @@ -762,15 +752,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
>>>         * during the battery module initialization.
>>>         */
>>>        list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
>>> -             if (hook_node->add_battery(battery->bat)) {
>>> -                     /*
>>> -                      * The notification of the extensions has failed, to
>>> -                      * prevent further errors we will unload the extension.
>>> -                      */
>>> -                     pr_err("error in extension, unloading: %s",
>>> -                                     hook_node->name);
>>> -                     __battery_hook_unregister(hook_node, 0);
>>> -             }
>>> +             hook_node->add_battery(battery->bat);
>>>        }
>>>        mutex_unlock(&hook_mutex);
>>>   }
>>> --
>>> 2.30.2
>>>
Armin Wolf Sept. 19, 2022, 8:35 p.m. UTC | #4
Am 19.09.22 um 21:12 schrieb Armin Wolf:

> Am 19.09.22 um 18:27 schrieb Rafael J. Wysocki:
>
>> On Mon, Sep 19, 2022 at 12:42 PM Hans de Goede <hdegoede@redhat.com> 
>> wrote:
>>> Hi,
>>>
>>> On 9/12/22 13:53, Armin Wolf wrote:
>>>> Currently, battery hooks are being unloaded if they return
>>>> an error when adding a single battery.
>>>> This however also causes the removal of successfully added
>>>> hooks if they return -ENODEV for a single unsupported
>>>> battery.
>>>>
>>>> Do not unload battery hooks in such cases since the hook
>>>> handles any cleanup actions.
>>>>
>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>> Maybe instead of removing all error checking, allow -ENODEV
>>> and behave as before when the error is not -ENODEV ?
>>>
>>> Otherwise we should probably make the add / remove callbacks
>>> void to indicate that any errors are ignored.
>>>
>>> Rafael, do you have any opinion on this?
>> IMV this is not a completely safe change, because things may simply
>> not work in the cases in which an error is returned.
>>
>> It would be somewhat better to use a special error code to indicate
>> "no support" (eg. -ENOTSUPP) and ignore that one only.
>
> I would favor -ENODEV then, since it is already used by quiet a few 
> drivers
> to indicate a unsupported battery.
>
> Armin Wolf
>
While checking all instances where the battery hook mechanism is currently used,
i found out that all but a single battery hook return -ENODEV for all errors they
encounter, the exception being the huawei-wmi driver.

I do not know the reason for this, but i fear unloading the extension on for
example -ENOTSUP will result in similar behavior by hooks wanting to avoid being
unloaded on harmless errors.

However, i agree that when ignoring all errors, battery extensions which provide
similar attributes may currently delete each others attributes.

Any idea on how to solve this?

Armin Wolf

>>>> ---
>>>>   drivers/acpi/battery.c | 24 +++---------------------
>>>>   1 file changed, 3 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>>>> index 306513fec1e1..e59c261c7c59 100644
>>>> --- a/drivers/acpi/battery.c
>>>> +++ b/drivers/acpi/battery.c
>>>> @@ -724,20 +724,10 @@ void battery_hook_register(struct 
>>>> acpi_battery_hook *hook)
>>>>         * its attributes.
>>>>         */
>>>>        list_for_each_entry(battery, &acpi_battery_list, list) {
>>>> -             if (hook->add_battery(battery->bat)) {
>>>> -                     /*
>>>> -                      * If a add-battery returns non-zero,
>>>> -                      * the registration of the extension has failed,
>>>> -                      * and we will not add it to the list of loaded
>>>> -                      * hooks.
>>>> -                      */
>>>> -                     pr_err("extension failed to load: %s", 
>>>> hook->name);
>>>> -                     __battery_hook_unregister(hook, 0);
>>>> -                     goto end;
>>>> -             }
>>>> +             hook->add_battery(battery->bat);
>>>>        }
>>>>        pr_info("new extension: %s\n", hook->name);
>>>> -end:
>>>> +
>>>>        mutex_unlock(&hook_mutex);
>>>>   }
>>>>   EXPORT_SYMBOL_GPL(battery_hook_register);
>>>> @@ -762,15 +752,7 @@ static void battery_hook_add_battery(struct 
>>>> acpi_battery *battery)
>>>>         * during the battery module initialization.
>>>>         */
>>>>        list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, 
>>>> list) {
>>>> -             if (hook_node->add_battery(battery->bat)) {
>>>> -                     /*
>>>> -                      * The notification of the extensions has 
>>>> failed, to
>>>> -                      * prevent further errors we will unload the 
>>>> extension.
>>>> -                      */
>>>> -                     pr_err("error in extension, unloading: %s",
>>>> -                                     hook_node->name);
>>>> -                     __battery_hook_unregister(hook_node, 0);
>>>> -             }
>>>> +             hook_node->add_battery(battery->bat);
>>>>        }
>>>>        mutex_unlock(&hook_mutex);
>>>>   }
>>>> -- 
>>>> 2.30.2
>>>>
Hans de Goede Sept. 27, 2022, 2:29 p.m. UTC | #5
Hi,

On 9/19/22 22:35, Armin Wolf wrote:
> Am 19.09.22 um 21:12 schrieb Armin Wolf:
> 
>> Am 19.09.22 um 18:27 schrieb Rafael J. Wysocki:
>>
>>> On Mon, Sep 19, 2022 at 12:42 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>> Hi,
>>>>
>>>> On 9/12/22 13:53, Armin Wolf wrote:
>>>>> Currently, battery hooks are being unloaded if they return
>>>>> an error when adding a single battery.
>>>>> This however also causes the removal of successfully added
>>>>> hooks if they return -ENODEV for a single unsupported
>>>>> battery.
>>>>>
>>>>> Do not unload battery hooks in such cases since the hook
>>>>> handles any cleanup actions.
>>>>>
>>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>> Maybe instead of removing all error checking, allow -ENODEV
>>>> and behave as before when the error is not -ENODEV ?
>>>>
>>>> Otherwise we should probably make the add / remove callbacks
>>>> void to indicate that any errors are ignored.
>>>>
>>>> Rafael, do you have any opinion on this?
>>> IMV this is not a completely safe change, because things may simply
>>> not work in the cases in which an error is returned.
>>>
>>> It would be somewhat better to use a special error code to indicate
>>> "no support" (eg. -ENOTSUPP) and ignore that one only.
>>
>> I would favor -ENODEV then, since it is already used by quiet a few drivers
>> to indicate a unsupported battery.
>>
>> Armin Wolf
>>
> While checking all instances where the battery hook mechanism is currently used,
> i found out that all but a single battery hook return -ENODEV for all errors they
> encounter, the exception being the huawei-wmi driver.

Right, so this means that using -ENODEV to not automatically unload the
extension on error will result in a behavior change for those drivers,
with possibly unwanted side-effects.

As such I believe that using -ENOTSUP for the case where the extension
does not work for 1 battery but should still be used for the other
batter{y|ies} would be better as this preserves the existing behavior
for existing drivers.

> I do not know the reason for this, but i fear unloading the extension on for
> example -ENOTSUP will result in similar behavior by hooks wanting to avoid being
> unloaded on harmless errors.

I am not sure what you are trying to say here. The whole idea is
to add new behavior for -ENOTSUP to allow drivers to opt out of
getting their extension unregistered when they return this.

Although I wonder why not just have extensions return 0 when
they don't want to register any sysfs attr and that not considered
an error. If it is not considered an error the hook can just
return 0, which would not require any ACPI battery code changes
at all. So maybe just returning 0 is the easiest (which is
also often the best) answer here?

> However, i agree that when ignoring all errors, battery extensions which provide
> similar attributes may currently delete each others attributes.

AFAIK there are no cases where more then 1 extension driver gets loaded,
since all the extension drivers are tied to a specific vendor's interfaces
so we won't e.g. see the thinkpad_acpi driver load on the same laptop as
where toshiba_acpi also loads.

IOW I think you are trying to solve a problem which does not exist here.

Regards,

Hans




> 
> Any idea on how to solve this?
> 
> Armin Wolf
> 
>>>>> ---
>>>>>   drivers/acpi/battery.c | 24 +++---------------------
>>>>>   1 file changed, 3 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>>>>> index 306513fec1e1..e59c261c7c59 100644
>>>>> --- a/drivers/acpi/battery.c
>>>>> +++ b/drivers/acpi/battery.c
>>>>> @@ -724,20 +724,10 @@ void battery_hook_register(struct acpi_battery_hook *hook)
>>>>>         * its attributes.
>>>>>         */
>>>>>        list_for_each_entry(battery, &acpi_battery_list, list) {
>>>>> -             if (hook->add_battery(battery->bat)) {
>>>>> -                     /*
>>>>> -                      * If a add-battery returns non-zero,
>>>>> -                      * the registration of the extension has failed,
>>>>> -                      * and we will not add it to the list of loaded
>>>>> -                      * hooks.
>>>>> -                      */
>>>>> -                     pr_err("extension failed to load: %s", hook->name);
>>>>> -                     __battery_hook_unregister(hook, 0);
>>>>> -                     goto end;
>>>>> -             }
>>>>> +             hook->add_battery(battery->bat);
>>>>>        }
>>>>>        pr_info("new extension: %s\n", hook->name);
>>>>> -end:
>>>>> +
>>>>>        mutex_unlock(&hook_mutex);
>>>>>   }
>>>>>   EXPORT_SYMBOL_GPL(battery_hook_register);
>>>>> @@ -762,15 +752,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
>>>>>         * during the battery module initialization.
>>>>>         */
>>>>>        list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
>>>>> -             if (hook_node->add_battery(battery->bat)) {
>>>>> -                     /*
>>>>> -                      * The notification of the extensions has failed, to
>>>>> -                      * prevent further errors we will unload the extension.
>>>>> -                      */
>>>>> -                     pr_err("error in extension, unloading: %s",
>>>>> -                                     hook_node->name);
>>>>> -                     __battery_hook_unregister(hook_node, 0);
>>>>> -             }
>>>>> +             hook_node->add_battery(battery->bat);
>>>>>        }
>>>>>        mutex_unlock(&hook_mutex);
>>>>>   }
>>>>> -- 
>>>>> 2.30.2
>>>>>
Armin Wolf Sept. 27, 2022, 3:44 p.m. UTC | #6
Am 27.09.22 um 16:29 schrieb Hans de Goede:

> Hi,
>
> On 9/19/22 22:35, Armin Wolf wrote:
>> Am 19.09.22 um 21:12 schrieb Armin Wolf:
>>
>>> Am 19.09.22 um 18:27 schrieb Rafael J. Wysocki:
>>>
>>>> On Mon, Sep 19, 2022 at 12:42 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>>> Hi,
>>>>>
>>>>> On 9/12/22 13:53, Armin Wolf wrote:
>>>>>> Currently, battery hooks are being unloaded if they return
>>>>>> an error when adding a single battery.
>>>>>> This however also causes the removal of successfully added
>>>>>> hooks if they return -ENODEV for a single unsupported
>>>>>> battery.
>>>>>>
>>>>>> Do not unload battery hooks in such cases since the hook
>>>>>> handles any cleanup actions.
>>>>>>
>>>>>> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
>>>>> Maybe instead of removing all error checking, allow -ENODEV
>>>>> and behave as before when the error is not -ENODEV ?
>>>>>
>>>>> Otherwise we should probably make the add / remove callbacks
>>>>> void to indicate that any errors are ignored.
>>>>>
>>>>> Rafael, do you have any opinion on this?
>>>> IMV this is not a completely safe change, because things may simply
>>>> not work in the cases in which an error is returned.
>>>>
>>>> It would be somewhat better to use a special error code to indicate
>>>> "no support" (eg. -ENOTSUPP) and ignore that one only.
>>> I would favor -ENODEV then, since it is already used by quiet a few drivers
>>> to indicate a unsupported battery.
>>>
>>> Armin Wolf
>>>
>> While checking all instances where the battery hook mechanism is currently used,
>> i found out that all but a single battery hook return -ENODEV for all errors they
>> encounter, the exception being the huawei-wmi driver.
> Right, so this means that using -ENODEV to not automatically unload the
> extension on error will result in a behavior change for those drivers,
> with possibly unwanted side-effects.
>
> As such I believe that using -ENOTSUP for the case where the extension
> does not work for 1 battery but should still be used for the other
> batter{y|ies} would be better as this preserves the existing behavior
> for existing drivers.
>
>> I do not know the reason for this, but i fear unloading the extension on for
>> example -ENOTSUP will result in similar behavior by hooks wanting to avoid being
>> unloaded on harmless errors.
> I am not sure what you are trying to say here. The whole idea is
> to add new behavior for -ENOTSUP to allow drivers to opt out of
> getting their extension unregistered when they return this.
>
> Although I wonder why not just have extensions return 0 when
> they don't want to register any sysfs attr and that not considered
> an error. If it is not considered an error the hook can just
> return 0, which would not require any ACPI battery code changes
> at all. So maybe just returning 0 is the easiest (which is
> also often the best) answer here?

I agree, i will send v2 soon.

Armin Wolf

>> However, i agree that when ignoring all errors, battery extensions which provide
>> similar attributes may currently delete each others attributes.
> AFAIK there are no cases where more then 1 extension driver gets loaded,
> since all the extension drivers are tied to a specific vendor's interfaces
> so we won't e.g. see the thinkpad_acpi driver load on the same laptop as
> where toshiba_acpi also loads.
>
> IOW I think you are trying to solve a problem which does not exist here.
>
> Regards,
>
> Hans
>
>
>
>
>> Any idea on how to solve this?
>>
>> Armin Wolf
>>
>>>>>> ---
>>>>>>    drivers/acpi/battery.c | 24 +++---------------------
>>>>>>    1 file changed, 3 insertions(+), 21 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>>>>>> index 306513fec1e1..e59c261c7c59 100644
>>>>>> --- a/drivers/acpi/battery.c
>>>>>> +++ b/drivers/acpi/battery.c
>>>>>> @@ -724,20 +724,10 @@ void battery_hook_register(struct acpi_battery_hook *hook)
>>>>>>          * its attributes.
>>>>>>          */
>>>>>>         list_for_each_entry(battery, &acpi_battery_list, list) {
>>>>>> -             if (hook->add_battery(battery->bat)) {
>>>>>> -                     /*
>>>>>> -                      * If a add-battery returns non-zero,
>>>>>> -                      * the registration of the extension has failed,
>>>>>> -                      * and we will not add it to the list of loaded
>>>>>> -                      * hooks.
>>>>>> -                      */
>>>>>> -                     pr_err("extension failed to load: %s", hook->name);
>>>>>> -                     __battery_hook_unregister(hook, 0);
>>>>>> -                     goto end;
>>>>>> -             }
>>>>>> +             hook->add_battery(battery->bat);
>>>>>>         }
>>>>>>         pr_info("new extension: %s\n", hook->name);
>>>>>> -end:
>>>>>> +
>>>>>>         mutex_unlock(&hook_mutex);
>>>>>>    }
>>>>>>    EXPORT_SYMBOL_GPL(battery_hook_register);
>>>>>> @@ -762,15 +752,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
>>>>>>          * during the battery module initialization.
>>>>>>          */
>>>>>>         list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
>>>>>> -             if (hook_node->add_battery(battery->bat)) {
>>>>>> -                     /*
>>>>>> -                      * The notification of the extensions has failed, to
>>>>>> -                      * prevent further errors we will unload the extension.
>>>>>> -                      */
>>>>>> -                     pr_err("error in extension, unloading: %s",
>>>>>> -                                     hook_node->name);
>>>>>> -                     __battery_hook_unregister(hook_node, 0);
>>>>>> -             }
>>>>>> +             hook_node->add_battery(battery->bat);
>>>>>>         }
>>>>>>         mutex_unlock(&hook_mutex);
>>>>>>    }
>>>>>> --
>>>>>> 2.30.2
>>>>>>
diff mbox series

Patch

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 306513fec1e1..e59c261c7c59 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -724,20 +724,10 @@  void battery_hook_register(struct acpi_battery_hook *hook)
 	 * its attributes.
 	 */
 	list_for_each_entry(battery, &acpi_battery_list, list) {
-		if (hook->add_battery(battery->bat)) {
-			/*
-			 * If a add-battery returns non-zero,
-			 * the registration of the extension has failed,
-			 * and we will not add it to the list of loaded
-			 * hooks.
-			 */
-			pr_err("extension failed to load: %s", hook->name);
-			__battery_hook_unregister(hook, 0);
-			goto end;
-		}
+		hook->add_battery(battery->bat);
 	}
 	pr_info("new extension: %s\n", hook->name);
-end:
+
 	mutex_unlock(&hook_mutex);
 }
 EXPORT_SYMBOL_GPL(battery_hook_register);
@@ -762,15 +752,7 @@  static void battery_hook_add_battery(struct acpi_battery *battery)
 	 * during the battery module initialization.
 	 */
 	list_for_each_entry_safe(hook_node, tmp, &battery_hook_list, list) {
-		if (hook_node->add_battery(battery->bat)) {
-			/*
-			 * The notification of the extensions has failed, to
-			 * prevent further errors we will unload the extension.
-			 */
-			pr_err("error in extension, unloading: %s",
-					hook_node->name);
-			__battery_hook_unregister(hook_node, 0);
-		}
+		hook_node->add_battery(battery->bat);
 	}
 	mutex_unlock(&hook_mutex);
 }