| Message ID | 20240612-yoga-ec-driver-v6-0-8e76ba060439@linaro.org |
|---|---|
| Headers | show |
| Series | power: supply: Lenovo Yoga C630 EC | expand |
On Wed, 12 Jun 2024, Dmitry Baryshkov wrote: > On the Lenovo Yoga C630 WOS laptop the EC provides access to the adapter > and battery status. Add the driver to read power supply status on the > laptop. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > +/* the mutex should already be locked */ Enforce this with lockdep_assert_held() (and remove the comment). > +static int yoga_c630_psy_update_bat_info(struct yoga_c630_psy *ecbat) > +{ > + struct yoga_c630_ec *ec = ecbat->ec; > + int val; > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_PRESENT); > + if (val < 0) > + return val; > + ecbat->bat_present = !!(val & LENOVO_EC_BAT_PRESENT_IS_PRESENT); > + if (!ecbat->bat_present) > + return val; > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > + if (val < 0) > + return val; > + ecbat->unit_mA = val & LENOVO_EC_BAT_ATTRIBUTES_UNIT_IS_MA; > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_DESIGN_CAPACITY); > + if (val < 0) > + return val; > + ecbat->design_capacity = val * 1000; > + > + /* > + * DSDT has delays after most of EC reads in these methods. > + * Having no documentation for the EC we have to follow and sleep here. > + */ > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_DESIGN_VOLTAGE); > + if (val < 0) > + return val; > + ecbat->design_voltage = val; > + > + msleep(50); > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_FULL_REGISTER); > + if (val < 0) > + return val; > + val = yoga_c630_ec_read16(ec, > + val & LENOVO_EC_BAT_FULL_REGISTER_IS_FACTORY ? > + LENOVO_EC_BAT_FULL_FACTORY : > + LENOVO_EC_BAT_FULL_CAPACITY); > + if (val < 0) > + return val; > + > + ecbat->full_charge_capacity = val * 1000; > + > + if (!ecbat->unit_mA) { > + ecbat->design_capacity *= 10; > + ecbat->full_charge_capacity *= 10; > + } > + > + return 0; > +} > + > +static int yoga_c630_psy_maybe_update_bat_status(struct yoga_c630_psy *ecbat) > +{ > + struct yoga_c630_ec *ec = ecbat->ec; > + int current_mA; > + int val; > + > + scoped_guard(mutex, &ecbat->lock) { This too could be simply guard() to bring down the indentation level. > + if (time_before(jiffies, ecbat->last_status_update + LENOVO_EC_CACHE_TIME)) > + return 0; > + > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_STATUS); > + if (val < 0) > + return val; > + ecbat->bat_status = val; > + > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_REMAIN_CAPACITY); > + if (val < 0) > + return val; > + ecbat->capacity_now = val * 1000; > + > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_VOLTAGE); > + if (val < 0) > + return val; > + ecbat->voltage_now = val * 1000; > + > + msleep(50); > + > + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_CURRENT); > + if (val < 0) > + return val; > + current_mA = sign_extend32(val, 15); > + ecbat->current_now = current_mA * 1000; > + ecbat->rate_now = current_mA * (ecbat->voltage_now / 1000); > + > + msleep(50); > + > + if (!ecbat->unit_mA) > + ecbat->capacity_now *= 10; > + > + ecbat->last_status_update = jiffies; > + } > + > + return 0; > +} > + > +static int yoga_c630_psy_update_adapter_status(struct yoga_c630_psy *ecbat) > +{ > + struct yoga_c630_ec *ec = ecbat->ec; > + int val; > + > + scoped_guard(mutex, &ecbat->lock) { Ditto. > + val = yoga_c630_ec_read8(ec, LENOVO_EC_ADPT_STATUS); > + if (val < 0) > + return val; > + > + ecbat->adapter_online = !!(val & LENOVO_EC_ADPT_STATUS_PRESENT); > + } > + > + return 0; > +} > +static const struct power_supply_desc yoga_c630_psy_bat_psy_desc_mA = { > + .name = "yoga-c630-battery", > + .type = POWER_SUPPLY_TYPE_BATTERY, > + .properties = yoga_c630_psy_bat_mA_properties, > + .num_properties = ARRAY_SIZE(yoga_c630_psy_bat_mA_properties), > + .get_property = yoga_c630_psy_bat_get_property, > +}; > + > +static const struct power_supply_desc yoga_c630_psy_bat_psy_desc_mWh = { > + .name = "yoga-c630-battery", > + .type = POWER_SUPPLY_TYPE_BATTERY, > + .properties = yoga_c630_psy_bat_mWh_properties, > + .num_properties = ARRAY_SIZE(yoga_c630_psy_bat_mWh_properties), > + .get_property = yoga_c630_psy_bat_get_property, > +}; > + > +static int yoga_c630_psy_adpt_get_property(struct power_supply *psy, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct yoga_c630_psy *ecbat = power_supply_get_drvdata(psy); > + int ret = 0; > + > + ret = yoga_c630_psy_update_adapter_status(ecbat); > + if (ret < 0) > + return ret; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_ONLINE: > + val->intval = ecbat->adapter_online; > + break; > + case POWER_SUPPLY_PROP_USB_TYPE: > + val->intval = POWER_SUPPLY_USB_TYPE_C; > + break; > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static enum power_supply_property yoga_c630_psy_adpt_properties[] = { > + POWER_SUPPLY_PROP_ONLINE, > + POWER_SUPPLY_PROP_USB_TYPE, > +}; > + > +static const enum power_supply_usb_type yoga_c630_psy_adpt_usb_type[] = { > + POWER_SUPPLY_USB_TYPE_C, > +}; > + > +static const struct power_supply_desc yoga_c630_psy_adpt_psy_desc = { > + .name = "yoga-c630-adapter", > + .type = POWER_SUPPLY_TYPE_USB, > + .usb_types = yoga_c630_psy_adpt_usb_type, > + .num_usb_types = ARRAY_SIZE(yoga_c630_psy_adpt_usb_type), > + .properties = yoga_c630_psy_adpt_properties, > + .num_properties = ARRAY_SIZE(yoga_c630_psy_adpt_properties), > + .get_property = yoga_c630_psy_adpt_get_property, > +}; > + > +static int yoga_c630_psy_register_bat_psy(struct yoga_c630_psy *ecbat) > +{ > + struct power_supply_config bat_cfg = {}; > + > + bat_cfg.drv_data = ecbat; > + bat_cfg.fwnode = ecbat->fwnode; > + ecbat->bat_psy = power_supply_register_no_ws(ecbat->dev, > + ecbat->unit_mA ? > + &yoga_c630_psy_bat_psy_desc_mA : > + &yoga_c630_psy_bat_psy_desc_mWh, > + &bat_cfg); > + if (IS_ERR(ecbat->bat_psy)) { > + dev_err(ecbat->dev, "failed to register battery supply\n"); > + return PTR_ERR(ecbat->bat_psy); > + } > + > + return 0; > +} > + > +static void yoga_c630_ec_refresh_bat_info(struct yoga_c630_psy *ecbat) > +{ > + bool current_unit; > + > + scoped_guard(mutex, &ecbat->lock) { guard() > + current_unit = ecbat->unit_mA; > + > + yoga_c630_psy_update_bat_info(ecbat); > + > + if (current_unit != ecbat->unit_mA) { > + power_supply_unregister(ecbat->bat_psy); > + yoga_c630_psy_register_bat_psy(ecbat); > + } > + } > +} > + adp_cfg.supplied_to = (char **)&yoga_c630_psy_bat_psy_desc_mA.name; This is not problem with your patch but I'm wondering why supplied_to needs to be non-const char *. Are those strings expected to be altered by something, I couldn't find anything to that effect (the pointer itself does not become const if supplied_to is changed to const char **)? -- i.
Hi, On Wed, Jun 12, 2024 at 12:59:35PM GMT, Dmitry Baryshkov wrote: > On the Lenovo Yoga C630 WOS laptop the EC provides access to the adapter > and battery status. Add the driver to read power supply status on the > laptop. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/power/supply/Kconfig | 9 + > drivers/power/supply/Makefile | 1 + > drivers/power/supply/lenovo_yoga_c630_battery.c | 500 ++++++++++++++++++++++++ > 3 files changed, 510 insertions(+) > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index 3e31375491d5..55ab8e90747d 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -167,6 +167,15 @@ config BATTERY_LEGO_EV3 > help > Say Y here to enable support for the LEGO MINDSTORMS EV3 battery. > > +config BATTERY_LENOVO_YOGA_C630 > + tristate "Lenovo Yoga C630 battery" > + depends on OF && EC_LENOVO_YOGA_C630 The driver should no longer depend on OF. Otherwise LGTM. Thanks for reworking it. Greetings, -- Sebastian
Hi, On Thu, Jun 13, 2024 at 10:57:41AM GMT, Ilpo Järvinen wrote: > > + adp_cfg.supplied_to = (char **)&yoga_c630_psy_bat_psy_desc_mA.name; > > This is not problem with your patch but I'm wondering why supplied_to > needs to be non-const char *. Are those strings expected to be altered by > something, I couldn't find anything to that effect (the pointer itself > does not become const if supplied_to is changed to const char **)? No, they are not supposed to be modified. It should be fine to make the struct member const char **, patches are welcome :) -- Sebastian
On Fri, Jun 14, 2024 at 03:35:25AM GMT, Sebastian Reichel wrote: > Hi, > > On Wed, Jun 12, 2024 at 12:59:35PM GMT, Dmitry Baryshkov wrote: > > On the Lenovo Yoga C630 WOS laptop the EC provides access to the adapter > > and battery status. Add the driver to read power supply status on the > > laptop. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/power/supply/Kconfig | 9 + > > drivers/power/supply/Makefile | 1 + > > drivers/power/supply/lenovo_yoga_c630_battery.c | 500 ++++++++++++++++++++++++ > > 3 files changed, 510 insertions(+) > > > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > > index 3e31375491d5..55ab8e90747d 100644 > > --- a/drivers/power/supply/Kconfig > > +++ b/drivers/power/supply/Kconfig > > @@ -167,6 +167,15 @@ config BATTERY_LEGO_EV3 > > help > > Say Y here to enable support for the LEGO MINDSTORMS EV3 battery. > > > > +config BATTERY_LENOVO_YOGA_C630 > > + tristate "Lenovo Yoga C630 battery" > > + depends on OF && EC_LENOVO_YOGA_C630 > > The driver should no longer depend on OF. Otherwise LGTM. > Thanks for reworking it. Ack, I'll post a fixed version once Ilpo announces an immutable branch.
This adds binding, driver and the DT support for the Lenovo Yoga C630 Embedded Controller, to provide battery information. Support for this EC was implemented by Bjorn, who later could not work on this driver. I've picked this patchset up and updated it following the pending review comments. DisplayPort support is still not a part of this patchset. It uses EC messages to provide AltMode information rather than implementing corresponding UCSI commands. However to have a cleaner uAPI story, the AltMode should be handled via the same Type-C port. Merge strategy: the driver bits depend on the platform/arm64 patch, which adds interface for the subdrivers. I'd either ask to get that patch merged to the immutable branch, which then can be picked up by power/supply and USB trees or, to make life simpler, ack merging all driver bits e.g. through USB subsystem (I'm biased here since I plan to send more cleanups for the UCSI subsystem, which would otherwise result in cross-subsystem conflicts). --- Changes in v6: - Use guard() instead of scoped_guard() (Ilpo) - Add a define for UCSI version register (Ilpo) - Added a check to prevent overflowing the address in reg16 read (Ilpo) - Link to v5: https://lore.kernel.org/r/20240607-yoga-ec-driver-v5-0-1ac91a0b4326@linaro.org Changes in v5: - Added missing article in the commit message (Bryan) - Changed yoga_c630_ec_ucsi_get_version() to explicitly set the register instead of just incrementing it (Bryan) - Dropped spurious debugging pr_info (Bryan) - Added missing includes all over the place (Ilpo) - Switched to scoped_guard() where it's suitable (Ilpo) - Defined register bits (Ilpo, Bryan) - Whitespace cleanup (Ilpo, Bryan) - Reworked yoga_c630_ucsi_notify() to use switch-case (Bryan) - Use ternary operators instead of if()s (Ilpo) - Switched power supply driver to use fwnode (Sebastian) - Fixed handling of the adapter's type vs usb_type (Sebastian) - Added SCOPE property to the battery (Sebastian) - Link to v4: https://lore.kernel.org/r/20240528-yoga-ec-driver-v4-0-4fa8dfaae7b6@linaro.org Changes in v4: - Moved bindings to platform/ to follow example of other Acer Aspire1 EC (Nikita Travkin) - Fixed dt validation for EC interrupt pin (Rob Herring) - Dropped separate 'scale' property (Oliver Neukum) - Link to v3: https://lore.kernel.org/r/20240527-yoga-ec-driver-v3-0-327a9851dad5@linaro.org Changes in v3: - Split the driver into core and power supply drivers, - Added UCSI driver part, handling USB connections, - Fixed Bjorn's address in DT bindings (Brian Masney) - Changed power-role for both ports to be "dual" per UCSI - Link to v2: https://lore.kernel.org/linux-arm-msm/20230205152809.2233436-1-dmitry.baryshkov@linaro.org/ Changes in v2: - Dropped DP support for now, as the bindings are in process of being discussed separately, - Merged dt patch into the same patchseries, - Removed the fixed serial number battery property, - Fixed indentation of dt bindings example, - Added property: reg and unevaluatedProperties to the connector bindings. - Link to v1: https://lore.kernel.org/linux-arm-msm/20220810035424.2796777-1-bjorn.andersson@linaro.org/ --- Bjorn Andersson (2): dt-bindings: platform: Add Lenovo Yoga C630 EC arm64: dts: qcom: c630: Add Embedded Controller node Dmitry Baryshkov (4): platform: arm64: add Lenovo Yoga C630 WOS EC driver usb: typec: ucsi: add Lenovo Yoga C630 glue driver power: supply: lenovo_yoga_c630_battery: add Lenovo C630 driver arm64: dts: qcom: sdm845: describe connections of USB/DP port .../bindings/platform/lenovo,yoga-c630-ec.yaml | 83 ++++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 53 ++- .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 75 ++++ drivers/platform/arm64/Kconfig | 14 + drivers/platform/arm64/Makefile | 1 + drivers/platform/arm64/lenovo-yoga-c630.c | 290 ++++++++++++ drivers/power/supply/Kconfig | 9 + drivers/power/supply/Makefile | 1 + drivers/power/supply/lenovo_yoga_c630_battery.c | 500 +++++++++++++++++++++ drivers/usb/typec/ucsi/Kconfig | 9 + drivers/usb/typec/ucsi/Makefile | 1 + drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 202 +++++++++ include/linux/platform_data/lenovo-yoga-c630.h | 44 ++ 13 files changed, 1281 insertions(+), 1 deletion(-) --- base-commit: 03d44168cbd7fc57d5de56a3730427db758fc7f6 change-id: 20240527-yoga-ec-driver-76fd7f5ddae8 Best regards,