24.08.2020 17:07, Sebastian Reichel пишет:
> Hi,
...
>> +static int a500_battery_get_serial_number(struct a500_battery *bat,
>> + union power_supply_propval *val)
>> +{
>> + unsigned int i;
>> + s32 ret = 0;
>> +
>> + if (bat->serial[0])
>> + goto done;
>> +
>> + a500_ec_lock(bat->ec_chip);
>> + for (i = 0; i < BATTERY_SERIAL_LEN / 2; i++) {
>> + ret = a500_ec_read_locked(bat->ec_chip,
>> + &ec_data[REG_SERIAL_NUMBER].cmd);
>> + if (ret < 0) {
>> + bat->serial[0] = '\0';
>> + break;
>> + }
>> +
>> + bat->serial[i * 2 + 0] = (ret >> 0) & 0xff;
>> + bat->serial[i * 2 + 1] = (ret >> 8) & 0xff;
>> + }
>> + a500_ec_unlock(bat->ec_chip);
>> +done:
>> + val->strval = bat->serial;
>> +
>> + return ret;
>> +}
>
> If battery is swapped, this will keep the old serial.
Hello, Sebastian! The battery isn't hot-swappable on A500, but it also
should be okay to always re-read the serialno. I'll consider removing
the caching in the v2, thanks.
...
>> + bat->psy = devm_power_supply_register_no_ws(&pdev->dev,
>> + &a500_battery_desc,
>> + &psy_cfg);
>> + err = PTR_ERR_OR_ZERO(bat->psy);
>> + if (err) {
>> + if (err == -EPROBE_DEFER)
>> + dev_dbg(&pdev->dev, "failed to register battery, deferring probe\n");
>> + else
>> + dev_err(&pdev->dev, "failed to register battery: %d\n",
>> + err);
>> + return err;
>> + }
>
> if (IS_ERR(bat->psy))
> return dev_err_probe(&pdev->dev, PTR_ERR(err), "failed to register battery\n");
I didn't know that dev_err_probe() is available now, very nice! I'll use
it in the v2, thanks.
...
>> +MODULE_DESCRIPTION("Battery gauge driver for Acer Iconia Tab A500");
>> +MODULE_AUTHOR("Dmitry Osipenko <digetx@xxxxxxxxx>");
>> +MODULE_ALIAS("platform:acer-a500-iconia-battery");
>> +MODULE_LICENSE("GPL v2");
>
> MODULE_LICENSE("GPL");
>
> Otherwise looks good to me.
Okay, thank you!
