On 12/7/23 12:20, Nikita Travkin wrote:
Acer Aspire 1 is a Snapdragon 7c based laptop. It uses an embedded
controller to control the charging and battery management, as well as to
perform a set of misc functions.
Unfortunately, while all this functionality is implemented in ACPI, it's
currently not possible to use ACPI to boot Linux on such Qualcomm
devices. To allow Linux to still support the features provided by EC,
this driver reimplments the relevant ACPI parts. This allows us to boot
the laptop with Device Tree and retain all the features.
Signed-off-by: Nikita Travkin <nikita@xxxxxxx>
---
[...]
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = le16_to_cpu(ddat.capacity_now) * 100
+ / le16_to_cpu(sdat.capacity_full);
It may be just my OCD and im not the maintainer here, but I'd do
/= here
[...]
+ case POWER_SUPPLY_PROP_MODEL_NAME:
+ if (sdat.model_id - 1 < ARRAY_SIZE(aspire_ec_psy_battery_model))
+ val->strval = aspire_ec_psy_battery_model[sdat.model_id - 1];
+ else
+ val->strval = "Unknown";
Would it make sense to print the model_id that's absent from the LUT
here and similarly below?
+ break;
+
+ case POWER_SUPPLY_PROP_MANUFACTURER:
+ if (sdat.vendor_id - 3 < ARRAY_SIZE(aspire_ec_psy_battery_vendor))
+ val->strval = aspire_ec_psy_battery_vendor[sdat.vendor_id - 3];
+ else
+ val->strval = "Unknown";
+ break;
+
+ default:
+ return -EINVAL;
+ }
Another ocd trip, i'd add a newline before return
+ return 0;
+}
[...]
+ /*
+ * The original ACPI firmware actually has a small sleep in the handler.
+ *
+ * It seems like in most cases it's not needed but when the device
+ * just exits suspend, our i2c driver has a brief time where data
+ * transfer is not possible yet. So this delay allows us to suppress
+ * quite a bunch of spurious error messages in dmesg. Thus it's kept.
Ouch.. do you think i2c-geni needs fixing on this part?
[...]
+ switch (id) {
+ case 0x0: /* No event */
+ break;
Is this a NOP/watchdog sort of thing?
[...]
+
+static struct i2c_driver aspire_ec_driver = {
+ .driver = {
+ .name = "aspire-ec",
+ .of_match_table = aspire_ec_of_match,
+ .pm = pm_sleep_ptr(&aspire_ec_pm_ops),
+ },
+ .probe = aspire_ec_probe,
+ .id_table = aspire_ec_id,
Since it's tristate, I'd expect an entry for .remove_new here
Konrad
