linux-kernel - Re: [PATCH v1 2/6] power: supply: Add battery gauge driver for Acer Iconia Tab A500

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <31ec6865-4a33-bde8-73a6-20c188ec2ee7@gmail.com>
Date:   Mon, 24 Aug 2020 21:55:14 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Sebastian Reichel <sebastian.reichel@...labora.com>
Cc:     Lee Jones <lee.jones@...aro.org>, Rob Herring <robh+dt@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Pavel Machek <pavel@....cz>, Dan Murphy <dmurphy@...com>,
        Lubomir Rintel <lkundrak@...sk>, devicetree@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-leds@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/6] power: supply: Add battery gauge driver for Acer
 Iconia Tab A500

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@...il.com>");
>> +MODULE_ALIAS("platform:acer-a500-iconia-battery");
>> +MODULE_LICENSE("GPL v2");
> 
> MODULE_LICENSE("GPL");
> 
> Otherwise looks good to me.

Okay, thank you!