On 2014年06月19日 07:00, Rafael J. Wysocki wrote:
> On Wednesday, June 18, 2014 03:15:47 PM David Rientjes wrote:
>> On Tue, 17 Jun 2014, Rafael J. Wysocki wrote:
>>
>>>> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
>>>> index e48fc98..8ed93a3 100644
>>>> --- a/drivers/acpi/battery.c
>>>> +++ b/drivers/acpi/battery.c
>>>> @@ -34,6 +34,7 @@
>>>> #include <linux/dmi.h>
>>>> #include <linux/slab.h>
>>>> #include <linux/suspend.h>
>>>> +#include <linux/delay.h>
>>>> #include <asm/unaligned.h>
>>>>
>>>> #ifdef CONFIG_ACPI_PROCFS_POWER
>>>> @@ -1119,7 +1120,7 @@ static struct dmi_system_id bat_dmi_table[] = {
>>>>
>>>> static int acpi_battery_add(struct acpi_device *device)
>>>> {
>>>> - int result = 0;
>>>> + int result = 0, retry = 5;
>>>> struct acpi_battery *battery = NULL;
>>>>
>>>> if (!device)
>>>> @@ -1135,9 +1136,20 @@ static int acpi_battery_add(struct acpi_device
>>>> *device)
>>>> mutex_init(&battery->sysfs_lock);
>>>> if (acpi_has_method(battery->device->handle, "_BIX"))
>>>> set_bit(ACPI_BATTERY_XINFO_PRESENT, &battery->flags);
>>>> - result = acpi_battery_update(battery, false);
>>>> +
>>>> + /*
>>>> + * Some machines'(E,G Lenovo Z480) ECs are not stable
>>>> + * during boot up and this causes battery driver fails to be
>>>> + * probed due to failure of getting battery information
>>>> + * from EC sometimes. After several retries, the operation
>>>> + * may work. So add retry code here and 20ms sleep between
>>>> + * every retries.
>>>> + */
>>>> + while ((result = acpi_battery_update(battery, false)) && retry--)
>>>> + msleep(20);
>>>> if (result)
>>>> goto fail;
>>>
>>> Why not to write this as
>>>
>>> for (;;) {
>>> result = acpi_battery_update(battery, false);
>>> if (!result)
>>> break;
>>> else if (!--retry)
>>> goto fail;
>>>
>>> msleep(20);
>>> }
>>>
>>
>> I suggested a similar for loop earlier, I think it's cleaner.
>
> Precisely. And it would be even more clean to introduce
>
> static bool acpi_battery_update_retry(struct acpi_battery *battery)
> {
> int retry, ret;
>
> for (retry = 5; retry; retry--) {
> ret = acpi_battery_update(battery, false);
> if (!ret)
> break;
>
> msleep(20);
> }
> return ret;
> }
>
> and use that instead of the open-coded loop.
Ok. I will update patch like this.
>
> Rafael
>
--
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
