* [PATCH 0/2] power: supply: Lenovo Yoga C630 EC
@ 2022-08-10 3:04 Bjorn Andersson
2022-08-10 3:04 ` [PATCH 1/2] dt-bindings: power: supply: Add " Bjorn Andersson
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Bjorn Andersson @ 2022-08-10 3:04 UTC (permalink / raw)
To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski
Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm
This adds binding and driver for the Lenovo Yoga C630 Embedded Controller, to
provide battery information and DisplayPort support.
Bjorn Andersson (2):
dt-bindings: power: supply: Add Lenovo Yoga C630 EC
power: supply: Add Lenovo Yoga C630 EC driver
.../power/supply/lenovo,yoga-c630-ec.yaml | 88 +++
drivers/power/supply/Kconfig | 11 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/yoga-c630-ec.c | 547 ++++++++++++++++++
4 files changed, 647 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml
create mode 100644 drivers/power/supply/yoga-c630-ec.c
--
2.37.1
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/2] dt-bindings: power: supply: Add Lenovo Yoga C630 EC 2022-08-10 3:04 [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Bjorn Andersson @ 2022-08-10 3:04 ` Bjorn Andersson 2022-08-10 13:35 ` Krzysztof Kozlowski 2022-08-10 15:11 ` Rob Herring 2022-08-10 3:05 ` [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver Bjorn Andersson 2022-08-10 4:04 ` [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Steev Klimaszewski 2 siblings, 2 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-08-10 3:04 UTC (permalink / raw) To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm Add binding for the Embedded Controller found in the Qualcomm Snapdragon-based Lenovo Yoga C630. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- .../power/supply/lenovo,yoga-c630-ec.yaml | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml diff --git a/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml b/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml new file mode 100644 index 000000000000..2dceb57a56b1 --- /dev/null +++ b/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/power/supply/lenovo,yoga-c630-ec.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Lenovo Yoga C630 Embedded Controller. + +maintainers: + - Bjorn Andersson <bjorn.andersson@linaro.org> + +description: + The Qualcomm Snapdragon-based Lenovo Yoga C630 has an Embedded Controller + (EC) which handles things such as battery and USB Type-C. This binding + describes the interface, on an I2C bus, to this EC. + +properties: + compatible: + const: lenovo,yoga-c630-ec + + reg: + const: 0x70 + + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + interrupts: + maxItems: 1 + +patternProperties: + '^connector@\d$': + $ref: /schemas/connector/usb-connector.yaml# + +required: + - compatible + - reg + - interrupts + +additionalProperties: false + +examples: + - |+ + #include <dt-bindings/interrupt-controller/irq.h> + i2c1 { + clock-frequency = <400000>; + + #address-cells = <1>; + #size-cells = <0>; + + embedded-controller@70 { + compatible = "lenovo,yoga-c630-ec"; + reg = <0x70>; + + interrupts-extended = <&tlmm 20 IRQ_TYPE_LEVEL_HIGH>; + + #address-cells = <1>; + #size-cells = <0>; + + connector@0 { + compatible = "usb-c-connector"; + reg = <0>; + power-role = "source"; + data-role = "host"; + }; + + connector@1 { + compatible = "usb-c-connector"; + reg = <1>; + power-role = "source"; + data-role = "host"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + port@1 { + reg = <1>; + lenovo_ec_dp_in: endpoint { + remote-endpoint = <&mdss_dp_out>; + }; + }; + }; + }; + }; + }; +... -- 2.37.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: power: supply: Add Lenovo Yoga C630 EC 2022-08-10 3:04 ` [PATCH 1/2] dt-bindings: power: supply: Add " Bjorn Andersson @ 2022-08-10 13:35 ` Krzysztof Kozlowski 2022-08-10 19:33 ` Bjorn Andersson 2022-08-10 15:11 ` Rob Herring 1 sibling, 1 reply; 19+ messages in thread From: Krzysztof Kozlowski @ 2022-08-10 13:35 UTC (permalink / raw) To: Bjorn Andersson, Sebastian Reichel, Rob Herring, Krzysztof Kozlowski Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm On 10/08/2022 06:04, Bjorn Andersson wrote: > Add binding for the Embedded Controller found in the Qualcomm > Snapdragon-based Lenovo Yoga C630. Thank you for your patch. There is something to discuss/improve. > + > +description: > + The Qualcomm Snapdragon-based Lenovo Yoga C630 has an Embedded Controller > + (EC) which handles things such as battery and USB Type-C. This binding > + describes the interface, on an I2C bus, to this EC. > + > +properties: > + compatible: > + const: lenovo,yoga-c630-ec > + > + reg: > + const: 0x70 > + > + '#address-cells': > + const: 1 Just to clarify: the EC have physically two USB connectors? > + > + '#size-cells': > + const: 0 > + > + interrupts: > + maxItems: 1 > + > +patternProperties: > + '^connector@\d$': > + $ref: /schemas/connector/usb-connector.yaml# unevaluatedProperties:false inside connector (on its level) > + > +required: > + - compatible > + - reg > + - interrupts > + > +additionalProperties: false > + > +examples: > + - |+ > + #include <dt-bindings/interrupt-controller/irq.h> > + i2c1 { > + clock-frequency = <400000>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + embedded-controller@70 { > + compatible = "lenovo,yoga-c630-ec"; > + reg = <0x70>; > + > + interrupts-extended = <&tlmm 20 IRQ_TYPE_LEVEL_HIGH>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + connector@0 { > + compatible = "usb-c-connector"; > + reg = <0>; > + power-role = "source"; > + data-role = "host"; > + }; > + > + connector@1 { > + compatible = "usb-c-connector"; > + reg = <1>; > + power-role = "source"; > + data-role = "host"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@1 { > + reg = <1>; > + lenovo_ec_dp_in: endpoint { > + remote-endpoint = <&mdss_dp_out>; You have inconsistent indentation. Use 4-spaces for entire DTS example. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: power: supply: Add Lenovo Yoga C630 EC 2022-08-10 13:35 ` Krzysztof Kozlowski @ 2022-08-10 19:33 ` Bjorn Andersson 0 siblings, 0 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-08-10 19:33 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm On Wed 10 Aug 06:35 PDT 2022, Krzysztof Kozlowski wrote: > On 10/08/2022 06:04, Bjorn Andersson wrote: > > Add binding for the Embedded Controller found in the Qualcomm > > Snapdragon-based Lenovo Yoga C630. > > Thank you for your patch. There is something to discuss/improve. > > > + > > +description: > > + The Qualcomm Snapdragon-based Lenovo Yoga C630 has an Embedded Controller > > + (EC) which handles things such as battery and USB Type-C. This binding > > + describes the interface, on an I2C bus, to this EC. > > + > > +properties: > > + compatible: > > + const: lenovo,yoga-c630-ec > > + > > + reg: > > + const: 0x70 > > + > > + '#address-cells': > > + const: 1 > > Just to clarify: the EC have physically two USB connectors? > Correct, while it's only possible to do Displayport on the second connector, the EC is involved in both the connectors - based on the events received from it. > > + > > + '#size-cells': > > + const: 0 > > + > > + interrupts: > > + maxItems: 1 > > + > > +patternProperties: > > + '^connector@\d$': > > + $ref: /schemas/connector/usb-connector.yaml# > > unevaluatedProperties:false inside connector (on its level) > Okay, will update accordingly. > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + > > +additionalProperties: false > > + > > +examples: > > + - |+ > > + #include <dt-bindings/interrupt-controller/irq.h> > > + i2c1 { > > + clock-frequency = <400000>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + embedded-controller@70 { > > + compatible = "lenovo,yoga-c630-ec"; > > + reg = <0x70>; > > + > > + interrupts-extended = <&tlmm 20 IRQ_TYPE_LEVEL_HIGH>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + connector@0 { > > + compatible = "usb-c-connector"; > > + reg = <0>; > > + power-role = "source"; > > + data-role = "host"; > > + }; > > + > > + connector@1 { > > + compatible = "usb-c-connector"; > > + reg = <1>; > > + power-role = "source"; > > + data-role = "host"; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@1 { > > + reg = <1>; > > + lenovo_ec_dp_in: endpoint { > > + remote-endpoint = <&mdss_dp_out>; > > You have inconsistent indentation. Use 4-spaces for entire DTS example. > Odd, will fix. Thanks, Bjorn > Best regards, > Krzysztof ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/2] dt-bindings: power: supply: Add Lenovo Yoga C630 EC 2022-08-10 3:04 ` [PATCH 1/2] dt-bindings: power: supply: Add " Bjorn Andersson 2022-08-10 13:35 ` Krzysztof Kozlowski @ 2022-08-10 15:11 ` Rob Herring 1 sibling, 0 replies; 19+ messages in thread From: Rob Herring @ 2022-08-10 15:11 UTC (permalink / raw) To: Bjorn Andersson Cc: linux-pm, Sebastian Reichel, Rob Herring, devicetree, linux-kernel, Krzysztof Kozlowski, linux-arm-msm On Tue, 09 Aug 2022 22:04:59 -0500, Bjorn Andersson wrote: > Add binding for the Embedded Controller found in the Qualcomm > Snapdragon-based Lenovo Yoga C630. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > .../power/supply/lenovo,yoga-c630-ec.yaml | 88 +++++++++++++++++++ > 1 file changed, 88 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.example.dtb: embedded-controller@70: connector@1:ports: 'port@0' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.example.dtb: connector@1: ports: 'port@0' is a required property From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/connector/usb-connector.yaml doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/ This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-08-10 3:04 [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Bjorn Andersson 2022-08-10 3:04 ` [PATCH 1/2] dt-bindings: power: supply: Add " Bjorn Andersson @ 2022-08-10 3:05 ` Bjorn Andersson 2022-08-13 10:23 ` kernel test robot ` (2 more replies) 2022-08-10 4:04 ` [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Steev Klimaszewski 2 siblings, 3 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-08-10 3:05 UTC (permalink / raw) To: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC providing AC-adapter and battery status, as well as USB Type-C altmode notifications for Displayport operation. The Yoga C630 ships with Windows, where these operations primarily are implemented in ACPI, but due to various issues with the hardware representation therein it's not possible to run Linux on this information. As such this is a best-effort re-implementation of these operations, based on the register map expressed in ACPI and a fair amount of trial and error. Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/power/supply/Kconfig | 11 + drivers/power/supply/Makefile | 1 + drivers/power/supply/yoga-c630-ec.c | 547 ++++++++++++++++++++++++++++ 3 files changed, 559 insertions(+) create mode 100644 drivers/power/supply/yoga-c630-ec.c diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig index 1aa8323ad9f6..6e706e948ad2 100644 --- a/drivers/power/supply/Kconfig +++ b/drivers/power/supply/Kconfig @@ -897,4 +897,15 @@ config BATTERY_UG3105 device is off or suspended, the functionality of this driver is limited to reporting capacity only. +config LENOVO_YOGA_C630_EC + tristate "Lenovo Yoga C630 EC battery driver" + depends on DRM + depends on I2C + help + Driver for the Embedded Controller in the Qualcomm Snapdragon-based + Lenovo Yoga C630, which provides battery information and USB Type-C + altmode notifications. + + Say M or Y here to include this support. + endif # POWER_SUPPLY diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile index 7f02f36aea55..24a4129cc427 100644 --- a/drivers/power/supply/Makefile +++ b/drivers/power/supply/Makefile @@ -108,3 +108,4 @@ obj-$(CONFIG_BATTERY_ACER_A500) += acer_a500_battery.o obj-$(CONFIG_BATTERY_SURFACE) += surface_battery.o obj-$(CONFIG_CHARGER_SURFACE) += surface_charger.o obj-$(CONFIG_BATTERY_UG3105) += ug3105_battery.o +obj-$(CONFIG_LENOVO_YOGA_C630_EC) += yoga-c630-ec.o diff --git a/drivers/power/supply/yoga-c630-ec.c b/drivers/power/supply/yoga-c630-ec.c new file mode 100644 index 000000000000..1fa0b5844e01 --- /dev/null +++ b/drivers/power/supply/yoga-c630-ec.c @@ -0,0 +1,547 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2022 Linaro Ltd. + */ +#include <linux/bitops.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/module.h> +#include <linux/power_supply.h> +#include <linux/usb/typec_mux.h> + +#include <drm/drm_bridge.h> + +#define LENOVO_EC_CACHE_TIME (10 * HZ) + +#define LENOVO_EC_RESPONSE_REG 0x1 +#define LENOVO_EC_REQUEST_REG 0x2 + +#define LENOVO_EC_READ_REG 0xb0 +#define LENOVO_EC_QUERY_USB_EVENT 0x8 +#define LENOVO_EC_USB_EVENT_MUX GENMASK(1, 0) +#define LENOVO_EC_USB_EVENT_ORIENTATION BIT(7) +#define LENOVO_EC_ADPT_STATUS 0xa3 +#define LENOVO_EC_ADPT_PRESENT BIT(7) +#define LENOVO_EC_BAT_STATUS 0xc1 +#define LENOVO_EC_BAT_VOLTAGE 0xc6 +#define LENOVO_EC_BAT_CURRENT 0xd2 +#define LENOVO_EC_BAT_PRESENT 0xda +#define LENOVO_EC_BAT_ATTRIBUTES 0xc0 +#define LENOVO_EC_BAT_ATTR_UNIT_IS_MA BIT(1) +#define LENOVO_EC_BAT_REMAIN_CAPACITY 0xc2 +#define LENOVO_EC_BAT_DESIGN_VOLTAGE 0xc8 +#define LENOVO_EC_BAT_DESIGN_CAPACITY 0xca +#define LENOVO_EC_BAT_FULL_CAPACITY 0xcc +#define LENOVO_EC_BAT_FULL_FACTORY 0xd6 +#define LENOVO_EC_BAT_FULL_REGISTER 0xdb + +#define LENOVO_EC_REQUEST_NEXT_EVENT 0x84 +#define LENOVO_EC_EVENT_USB 0x20 +#define LENOVO_EC_EVENT_UCSI 0x21 +#define LENOVO_EC_EVENT_HPD 0x22 +#define LENOVO_EC_EVENT_BAT_STATUS 0x24 +#define LENOVO_EC_EVENT_BAT_INFO 0x25 +#define LENOVO_EC_EVENT_BAT_ADPT_STATUS 0x37 + +struct yoga_c630_ec { + struct i2c_client *client; + struct mutex lock; + + struct power_supply *adp_psy; + struct power_supply *bat_psy; + + struct typec_switch *typec_switch; + + unsigned long last_status_update; + + bool adapter_online; + + bool unit_ma; + + unsigned int scale; + + bool bat_present; + unsigned int bat_status; + + unsigned int design_capacity; + unsigned int design_voltage; + unsigned int full_charge_capacity; + + unsigned int capacity_now; + unsigned int voltage_now; + + int rate_now; + + bool bridge_configured; + struct drm_bridge bridge; +}; + +static int yoga_c630_ec_request(struct yoga_c630_ec *ec, u8 *req, size_t req_len, + u8 *resp, size_t resp_len) +{ + int ret; + + WARN_ON(!mutex_is_locked(&ec->lock)); + + ret = i2c_smbus_write_i2c_block_data(ec->client, LENOVO_EC_REQUEST_REG, + req_len, req); + if (ret < 0) + return ret; + + return i2c_smbus_read_i2c_block_data(ec->client, LENOVO_EC_RESPONSE_REG, + resp_len, resp); +} + +static int yoga_c630_ec_read8(struct yoga_c630_ec *ec, u8 addr) +{ + int ret; + u8 val; + + ret = yoga_c630_ec_request(ec, (u8[]) { LENOVO_EC_READ_REG, addr }, 2, + &val, 1); + if (ret < 0) + return ret; + + return val; +} + +static int yoga_c630_ec_read16(struct yoga_c630_ec *ec, u8 addr) +{ + int ret; + u8 msb; + u8 lsb; + + ret = yoga_c630_ec_request(ec, (u8[]) { LENOVO_EC_READ_REG, addr }, 2, + &lsb, 1); + if (ret < 0) + return ret; + + ret = yoga_c630_ec_request(ec, (u8[]) { LENOVO_EC_READ_REG, addr + 1 }, 2, + &msb, 1); + if (ret < 0) + return ret; + + return msb << 8 | lsb; +} + +static void yoga_c630_ec_update_bat_info(struct yoga_c630_ec *ec) +{ + int val; + + mutex_lock(&ec->lock); + + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_PRESENT); + if (val < 0) + goto out_unlock; + ec->bat_present = !!(val & BIT(0)); + if (!ec->bat_present) + goto out_unlock; + + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); + if (val < 0) + goto out_unlock; + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; + if (!ec->unit_ma) + ec->scale = 1000; + else + ec->scale = 1; + + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_DESIGN_CAPACITY); + if (val < 0) + goto out_unlock; + ec->design_capacity = val * ec->scale; + + msleep(50); + + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_DESIGN_VOLTAGE); + if (val < 0) + goto out_unlock; + ec->design_voltage = val; + + msleep(50); + + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_FULL_REGISTER); + if (val < 0) + goto out_unlock; + if (val & BIT(0)) + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_FULL_FACTORY); + else + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_FULL_CAPACITY); + if (val < 0) + goto out_unlock; + ec->full_charge_capacity = val * ec->scale; + +out_unlock: + mutex_unlock(&ec->lock); +} + +static void yoga_c630_ec_maybe_update_bat_status(struct yoga_c630_ec *ec) +{ + int val = 0; + + mutex_lock(&ec->lock); + + if (time_before(jiffies, ec->last_status_update + LENOVO_EC_CACHE_TIME)) + goto out_unlock; + + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_STATUS); + if (val < 0) + goto out_unlock; + ec->bat_status = val; + + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_REMAIN_CAPACITY); + if (val < 0) + goto out_unlock; + ec->capacity_now = val * ec->scale; + + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_VOLTAGE); + if (val < 0) + goto out_unlock; + ec->voltage_now = val; + + val = yoga_c630_ec_read16(ec, LENOVO_EC_BAT_CURRENT); + if (val < 0) + goto out_unlock; + ec->rate_now = sign_extend32(val, 15) * ec->scale; + + if (ec->unit_ma) + ec->rate_now = ec->rate_now * ec->voltage_now / 1000; + + ec->last_status_update = jiffies; + +out_unlock: + mutex_unlock(&ec->lock); +} + +static int yoga_c630_ec_update_adapter_status(struct yoga_c630_ec *ec) +{ + int val; + + mutex_lock(&ec->lock); + + val = yoga_c630_ec_read8(ec, LENOVO_EC_ADPT_STATUS); + if (val > 0) + ec->adapter_online = FIELD_GET(LENOVO_EC_ADPT_PRESENT, val); + + mutex_unlock(&ec->lock); + + return val; +} + +static bool yoga_c630_ec_is_charged(struct yoga_c630_ec *ec) +{ + if (ec->bat_status != 0) + return false; + + if (ec->full_charge_capacity == ec->capacity_now) + return true; + + if (ec->design_capacity == ec->capacity_now) + return true; + + return false; +} + +static int yoga_c630_ec_bat_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct yoga_c630_ec *ec = power_supply_get_drvdata(psy); + int rc = 0; + + if (ec->bat_present) + yoga_c630_ec_maybe_update_bat_status(ec); + else if (psp != POWER_SUPPLY_PROP_PRESENT) + return -ENODEV; + + switch (psp) { + case POWER_SUPPLY_PROP_STATUS: + if (ec->bat_status & BIT(0)) + val->intval = POWER_SUPPLY_STATUS_DISCHARGING; + else if (ec->bat_status & BIT(1)) + val->intval = POWER_SUPPLY_STATUS_CHARGING; + else if (yoga_c630_ec_is_charged(ec)) + val->intval = POWER_SUPPLY_STATUS_FULL; + else + val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING; + break; + case POWER_SUPPLY_PROP_PRESENT: + val->intval = ec->bat_present; + break; + case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN: + val->intval = ec->design_voltage; + break; + case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: + val->intval = ec->design_capacity; + break; + case POWER_SUPPLY_PROP_CHARGE_FULL: + val->intval = ec->full_charge_capacity; + break; + case POWER_SUPPLY_PROP_CHARGE_NOW: + val->intval = ec->capacity_now; + break; + case POWER_SUPPLY_PROP_CURRENT_NOW: + val->intval = ec->rate_now; + break; + case POWER_SUPPLY_PROP_VOLTAGE_NOW: + val->intval = ec->voltage_now; + break; + case POWER_SUPPLY_PROP_TECHNOLOGY: + val->intval = POWER_SUPPLY_TECHNOLOGY_LION; + break; + case POWER_SUPPLY_PROP_MODEL_NAME: + val->strval = "PABAS0241231"; + break; + case POWER_SUPPLY_PROP_MANUFACTURER: + val->strval = "Compal"; + break; + case POWER_SUPPLY_PROP_SERIAL_NUMBER: + val->strval = "05072018"; + break; + default: + rc = -EINVAL; + break; + } + + return rc; +} + +static enum power_supply_property yoga_c630_ec_bat_properties[] = { + POWER_SUPPLY_PROP_STATUS, + POWER_SUPPLY_PROP_PRESENT, + POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN, + POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN, + POWER_SUPPLY_PROP_CHARGE_FULL, + POWER_SUPPLY_PROP_CHARGE_NOW, + POWER_SUPPLY_PROP_CURRENT_NOW, + POWER_SUPPLY_PROP_VOLTAGE_NOW, + POWER_SUPPLY_PROP_TECHNOLOGY, + POWER_SUPPLY_PROP_MODEL_NAME, + POWER_SUPPLY_PROP_MANUFACTURER, + POWER_SUPPLY_PROP_SERIAL_NUMBER, +}; + +static const struct power_supply_desc yoga_c630_ec_bat_psy_desc = { + .name = "yoga-c630-battery", + .type = POWER_SUPPLY_TYPE_BATTERY, + .properties = yoga_c630_ec_bat_properties, + .num_properties = ARRAY_SIZE(yoga_c630_ec_bat_properties), + .get_property = yoga_c630_ec_bat_get_property, +}; + +static int yoga_c630_ec_adpt_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct yoga_c630_ec *ec = power_supply_get_drvdata(psy); + int rc = 0; + + yoga_c630_ec_update_adapter_status(ec); + + switch (psp) { + case POWER_SUPPLY_PROP_ONLINE: + val->intval = ec->adapter_online; + break; + default: + rc = -EINVAL; + break; + } + + return rc; +} + +static enum power_supply_property yoga_c630_ec_adpt_properties[] = { + POWER_SUPPLY_PROP_ONLINE, +}; + +static const struct power_supply_desc yoga_c630_ec_adpt_psy_desc = { + .name = "yoga-c630-adapter", + .type = POWER_SUPPLY_TYPE_USB_TYPE_C, + .properties = yoga_c630_ec_adpt_properties, + .num_properties = ARRAY_SIZE(yoga_c630_ec_adpt_properties), + .get_property = yoga_c630_ec_adpt_get_property, +}; + +static int yoga_c630_ec_attach(struct drm_bridge *bridge, + enum drm_bridge_attach_flags flags) +{ + return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL; +} + +static const struct drm_bridge_funcs yoga_c630_ec_bridge_funcs = { + .attach = yoga_c630_ec_attach, +}; + +static int yoga_c630_ec_query_usb_event(struct yoga_c630_ec *ec) +{ + struct device *dev = &ec->client->dev; + enum typec_orientation orientation; + enum drm_connector_status status; + int event; + + mutex_lock(&ec->lock); + event = yoga_c630_ec_read8(ec, LENOVO_EC_QUERY_USB_EVENT); + mutex_unlock(&ec->lock); + if (event < 0) { + dev_err(dev, "unable to query USB event\n"); + return event; + } + + if (FIELD_GET(LENOVO_EC_USB_EVENT_MUX, event)) + status = connector_status_connected; + else + status = connector_status_disconnected; + + if (FIELD_GET(LENOVO_EC_USB_EVENT_ORIENTATION, event)) + orientation = TYPEC_ORIENTATION_REVERSE; + else + orientation = TYPEC_ORIENTATION_NORMAL; + + typec_switch_set(ec->typec_switch, orientation); + if (ec->bridge_configured) + drm_bridge_hpd_notify(&ec->bridge, status); + + return 0; +} + +static irqreturn_t yoga_c630_ec_intr(int irq, void *data) +{ + struct yoga_c630_ec *ec = data; + u8 event; + int ret; + + mutex_lock(&ec->lock); + ret = yoga_c630_ec_request(ec, (u8[]){ LENOVO_EC_REQUEST_NEXT_EVENT }, 1, + &event, 1); + mutex_unlock(&ec->lock); + if (ret < 0) + return IRQ_HANDLED; + + switch (event) { + case LENOVO_EC_EVENT_USB: + case LENOVO_EC_EVENT_HPD: + yoga_c630_ec_query_usb_event(ec); + break; + case LENOVO_EC_EVENT_BAT_INFO: + yoga_c630_ec_update_bat_info(ec); + break; + case LENOVO_EC_EVENT_BAT_ADPT_STATUS: + power_supply_changed(ec->adp_psy); + fallthrough; + case LENOVO_EC_EVENT_BAT_STATUS: + power_supply_changed(ec->bat_psy); + break; + } + + return IRQ_HANDLED; +} + +static void yoga_c630_ec_put_switch(void *data) +{ + typec_switch_put(data); +} + +static int yoga_c630_ec_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct power_supply_config adp_cfg = {}; + struct power_supply_config bat_cfg = {}; + struct fwnode_handle *fwnode; + struct yoga_c630_ec *ec; + struct device *dev = &client->dev; + u32 port; + int ret; + + ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL); + if (!ec) + return -ENOMEM; + + mutex_init(&ec->lock); + ec->client = client; + + adp_cfg.drv_data = ec; + adp_cfg.of_node = client->dev.of_node; + ec->adp_psy = devm_power_supply_register(dev, &yoga_c630_ec_adpt_psy_desc, &adp_cfg); + if (IS_ERR(ec->adp_psy)) { + dev_err(dev, "failed to register AC adapter supply\n"); + return PTR_ERR(ec->adp_psy); + } + + bat_cfg.drv_data = ec; + bat_cfg.of_node = client->dev.of_node; + ec->bat_psy = devm_power_supply_register(dev, &yoga_c630_ec_bat_psy_desc, &bat_cfg); + if (IS_ERR(ec->bat_psy)) { + dev_err(dev, "failed to register battery supply\n"); + return PTR_ERR(ec->bat_psy); + } + + device_for_each_child_node(dev, fwnode) { + ret = fwnode_property_read_u32(fwnode, "reg", &port); + if (ret < 0) + continue; + + /* Got multiple ports, but altmode is only possible on port 1 */ + if (port != 1) + continue; + + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; + ec->bridge.of_node = to_of_node(fwnode); + ec->bridge.ops = DRM_BRIDGE_OP_HPD; + ec->bridge.type = DRM_MODE_CONNECTOR_USB; + + ret = devm_drm_bridge_add(dev, &ec->bridge); + if (ret) { + dev_err(dev, "failed to register drm bridge\n"); + fwnode_handle_put(fwnode); + return ret; + } + + ec->bridge_configured = true; + + ec->typec_switch = fwnode_typec_switch_get(fwnode); + if (IS_ERR(ec->typec_switch)) + return dev_err_probe(dev, PTR_ERR(ec->typec_switch), + "failed to acquire orientation-switch for port 1\n"); + + ret = devm_add_action_or_reset(dev, yoga_c630_ec_put_switch, + ec->typec_switch); + if (ret) + return ret; + } + + ret = devm_request_threaded_irq(dev, client->irq, + NULL, yoga_c630_ec_intr, + IRQF_ONESHOT, "yoga_c630_ec", ec); + if (ret < 0) + return dev_err_probe(dev, ret, "unable to request irq\n"); + + yoga_c630_ec_update_bat_info(ec); + + return 0; +} + +static const struct of_device_id yoga_c630_ec_of_match[] = { + { .compatible = "lenovo,yoga-c630-ec" }, + {} +}; +MODULE_DEVICE_TABLE(of, yoga_c630_ec_of_match); + +static const struct i2c_device_id yoga_c630_ec_i2c_id_table[] = { + { "yoga-c630-ec", }, + {} +}; +MODULE_DEVICE_TABLE(i2c, yoga_c630_ec_i2c_id_table); + +static struct i2c_driver yoga_c630_ec_i2c_driver = { + .driver = { + .name = "yoga-c630-ec", + .of_match_table = yoga_c630_ec_of_match, + }, + .probe = yoga_c630_ec_probe, + .id_table = yoga_c630_ec_i2c_id_table, +}; +module_i2c_driver(yoga_c630_ec_i2c_driver); + +MODULE_DESCRIPTION("Lenovo Yoga C630 EC driver"); +MODULE_LICENSE("GPL"); -- 2.37.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-08-10 3:05 ` [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver Bjorn Andersson @ 2022-08-13 10:23 ` kernel test robot 2022-09-12 17:00 ` Daniel Thompson 2022-09-13 10:45 ` Sebastian Reichel 2 siblings, 0 replies; 19+ messages in thread From: kernel test robot @ 2022-08-13 10:23 UTC (permalink / raw) To: Bjorn Andersson; +Cc: llvm, kbuild-all Hi Bjorn, I love your patch! Yet something to improve: [auto build test ERROR on sre-power-supply/for-next] [also build test ERROR on robh/for-next krzk-dt/for-next linus/master v5.19 next-20220812] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Bjorn-Andersson/power-supply-Lenovo-Yoga-C630-EC/20220810-110737 base: https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git for-next config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20220813/202208131821.HWmmofHI-lkp@intel.com/config) compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 5f1c7e2cc5a3c07cbc2412e851a7283c1841f520) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install powerpc cross compiling tool for clang build # apt-get install binutils-powerpc-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/d194cf7394bbf827844847bbde97efa1283ccb08 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Bjorn-Andersson/power-supply-Lenovo-Yoga-C630-EC/20220810-110737 git checkout d194cf7394bbf827844847bbde97efa1283ccb08 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/power/supply/yoga-c630-ec.c:225:24: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] ec->adapter_online = FIELD_GET(LENOVO_EC_ADPT_PRESENT, val); ^ drivers/power/supply/yoga-c630-ec.c:391:6: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] if (FIELD_GET(LENOVO_EC_USB_EVENT_MUX, event)) ^ >> drivers/power/supply/yoga-c630-ec.c:493:9: error: call to undeclared function 'devm_drm_bridge_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] ret = devm_drm_bridge_add(dev, &ec->bridge); ^ drivers/power/supply/yoga-c630-ec.c:493:9: note: did you mean 'drm_bridge_add'? include/drm/drm_bridge.h:798:6: note: 'drm_bridge_add' declared here void drm_bridge_add(struct drm_bridge *bridge); ^ 3 errors generated. vim +/FIELD_GET +225 drivers/power/supply/yoga-c630-ec.c 216 217 static int yoga_c630_ec_update_adapter_status(struct yoga_c630_ec *ec) 218 { 219 int val; 220 221 mutex_lock(&ec->lock); 222 223 val = yoga_c630_ec_read8(ec, LENOVO_EC_ADPT_STATUS); 224 if (val > 0) > 225 ec->adapter_online = FIELD_GET(LENOVO_EC_ADPT_PRESENT, val); 226 227 mutex_unlock(&ec->lock); 228 229 return val; 230 } 231 -- 0-DAY CI Kernel Test Service https://01.org/lkp ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-08-10 3:05 ` [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver Bjorn Andersson 2022-08-13 10:23 ` kernel test robot @ 2022-09-12 17:00 ` Daniel Thompson 2022-09-13 8:17 ` Daniel Thompson 2022-09-13 10:45 ` Sebastian Reichel 2 siblings, 1 reply; 19+ messages in thread From: Daniel Thompson @ 2022-09-12 17:00 UTC (permalink / raw) To: Bjorn Andersson Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > providing AC-adapter and battery status, as well as USB Type-C altmode > notifications for Displayport operation. There's a couple of minor review comments but before we get to that: woo hoo! > The Yoga C630 ships with Windows, where these operations primarily are > implemented in ACPI, but due to various issues with the hardware > representation therein it's not possible to run Linux on this > information. As such this is a best-effort re-implementation of these > operations, based on the register map expressed in ACPI and a fair > amount of trial and error. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Reported-by: kernel test robot <lkp@intel.com> > --- > drivers/power/supply/Kconfig | 11 + > drivers/power/supply/Makefile | 1 + > drivers/power/supply/yoga-c630-ec.c | 547 ++++++++++++++++++++++++++++ > 3 files changed, 559 insertions(+) > create mode 100644 drivers/power/supply/yoga-c630-ec.c > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index 1aa8323ad9f6..6e706e948ad2 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -897,4 +897,15 @@ config BATTERY_UG3105 > device is off or suspended, the functionality of this driver is > limited to reporting capacity only. > > +config LENOVO_YOGA_C630_EC > + tristate "Lenovo Yoga C630 EC battery driver" > + depends on DRM > + depends on I2C This needs a "depends on TYPEC" in order to avoid linker errors. > + help > + Driver for the Embedded Controller in the Qualcomm Snapdragon-based > + Lenovo Yoga C630, which provides battery information and USB Type-C > + altmode notifications. > + > + Say M or Y here to include this support. > + > endif # POWER_SUPPLY > diff --git a/drivers/power/supply/yoga-c630-ec.c b/drivers/power/supply/yoga-c630-ec.c > new file mode 100644 > index 000000000000..1fa0b5844e01 > --- /dev/null > +++ b/drivers/power/supply/yoga-c630-ec.c > @@ -0,0 +1,547 @@ > <snip> > +static int yoga_c630_ec_bat_get_property(struct power_supply *psy, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct yoga_c630_ec *ec = power_supply_get_drvdata(psy); > + int rc = 0; > + > + if (ec->bat_present) > + yoga_c630_ec_maybe_update_bat_status(ec); > + else if (psp != POWER_SUPPLY_PROP_PRESENT) > + return -ENODEV; > + > + switch (psp) { > <snip> > + case POWER_SUPPLY_PROP_TECHNOLOGY: > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION; > + break; > + case POWER_SUPPLY_PROP_MODEL_NAME: > + val->strval = "PABAS0241231"; > + break; > + case POWER_SUPPLY_PROP_MANUFACTURER: > + val->strval = "Compal"; > + break; > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > + val->strval = "05072018"; > + break; I'm a little sceptical that hardcoding a serial number into the driver provides anybody any benefit (regardless of whether the AML code does this). AFAICT this is not a commonly implemented property in other power supplies so I'm not clear why this is needed. Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-09-12 17:00 ` Daniel Thompson @ 2022-09-13 8:17 ` Daniel Thompson 0 siblings, 0 replies; 19+ messages in thread From: Daniel Thompson @ 2022-09-13 8:17 UTC (permalink / raw) To: Bjorn Andersson Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm On Mon, Sep 12, 2022 at 05:00:17PM +0000, Daniel Thompson wrote: > On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > > providing AC-adapter and battery status, as well as USB Type-C altmode > > notifications for Displayport operation. > > There's a couple of minor review comments but before we get to that: > woo hoo! ... and now with the correct address for Bjorn too (comments still below and indented > one level). Daniel. > > The Yoga C630 ships with Windows, where these operations primarily are > > implemented in ACPI, but due to various issues with the hardware > > representation therein it's not possible to run Linux on this > > information. As such this is a best-effort re-implementation of these > > operations, based on the register map expressed in ACPI and a fair > > amount of trial and error. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Reported-by: kernel test robot <lkp@intel.com> > > --- > > drivers/power/supply/Kconfig | 11 + > > drivers/power/supply/Makefile | 1 + > > drivers/power/supply/yoga-c630-ec.c | 547 ++++++++++++++++++++++++++++ > > 3 files changed, 559 insertions(+) > > create mode 100644 drivers/power/supply/yoga-c630-ec.c > > > > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > > index 1aa8323ad9f6..6e706e948ad2 100644 > > --- a/drivers/power/supply/Kconfig > > +++ b/drivers/power/supply/Kconfig > > @@ -897,4 +897,15 @@ config BATTERY_UG3105 > > device is off or suspended, the functionality of this driver is > > limited to reporting capacity only. > > > > +config LENOVO_YOGA_C630_EC > > + tristate "Lenovo Yoga C630 EC battery driver" > > + depends on DRM > > + depends on I2C > > This needs a "depends on TYPEC" in order to avoid linker errors. > > > > + help > > + Driver for the Embedded Controller in the Qualcomm Snapdragon-based > > + Lenovo Yoga C630, which provides battery information and USB Type-C > > + altmode notifications. > > + > > + Say M or Y here to include this support. > > + > > endif # POWER_SUPPLY > > diff --git a/drivers/power/supply/yoga-c630-ec.c b/drivers/power/supply/yoga-c630-ec.c > > new file mode 100644 > > index 000000000000..1fa0b5844e01 > > --- /dev/null > > +++ b/drivers/power/supply/yoga-c630-ec.c > > @@ -0,0 +1,547 @@ > > <snip> > > +static int yoga_c630_ec_bat_get_property(struct power_supply *psy, > > + enum power_supply_property psp, > > + union power_supply_propval *val) > > +{ > > + struct yoga_c630_ec *ec = power_supply_get_drvdata(psy); > > + int rc = 0; > > + > > + if (ec->bat_present) > > + yoga_c630_ec_maybe_update_bat_status(ec); > > + else if (psp != POWER_SUPPLY_PROP_PRESENT) > > + return -ENODEV; > > + > > + switch (psp) { > > <snip> > > + case POWER_SUPPLY_PROP_TECHNOLOGY: > > + val->intval = POWER_SUPPLY_TECHNOLOGY_LION; > > + break; > > + case POWER_SUPPLY_PROP_MODEL_NAME: > > + val->strval = "PABAS0241231"; > > + break; > > + case POWER_SUPPLY_PROP_MANUFACTURER: > > + val->strval = "Compal"; > > + break; > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > > + val->strval = "05072018"; > > + break; > > I'm a little sceptical that hardcoding a serial number into the > driver provides anybody any benefit (regardless of whether the > AML code does this). AFAICT this is not a commonly implemented property > in other power supplies so I'm not clear why this is needed. > > > Daniel. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-08-10 3:05 ` [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver Bjorn Andersson @ 2022-09-13 10:45 ` Sebastian Reichel 2022-09-12 17:00 ` Daniel Thompson 2022-09-13 10:45 ` Sebastian Reichel 2 siblings, 0 replies; 19+ messages in thread From: Sebastian Reichel @ 2022-09-13 10:45 UTC (permalink / raw) To: Bjorn Andersson Cc: Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm, Lee Jones, dri-devel [-- Attachment #1: Type: text/plain, Size: 2117 bytes --] Hi, [+Cc Lee Jones, DRI devel] On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > providing AC-adapter and battery status, as well as USB Type-C altmode > notifications for Displayport operation. > > The Yoga C630 ships with Windows, where these operations primarily are > implemented in ACPI, but due to various issues with the hardware > representation therein it's not possible to run Linux on this > information. As such this is a best-effort re-implementation of these > operations, based on the register map expressed in ACPI and a fair > amount of trial and error. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > [...] > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > + if (val < 0) > + goto out_unlock; > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > + if (!ec->unit_ma) > + ec->scale = 1000; > + else > + ec->scale = 1; Since I'm not sure how much of information was gained by reverse engineering: Is this really milliamps vs microamps and not milliamps vs milliwatt? SBS batteries usually report either mA or mW. > [...] > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > + val->strval = "05072018"; > + break; why is this hardcoded? :) > [...] > + device_for_each_child_node(dev, fwnode) { > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > + if (ret < 0) > + continue; > + > + /* Got multiple ports, but altmode is only possible on port 1 */ > + if (port != 1) > + continue; > + > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > + ec->bridge.of_node = to_of_node(fwnode); > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > + > + ret = devm_drm_bridge_add(dev, &ec->bridge); > + if (ret) { > + dev_err(dev, "failed to register drm bridge\n"); > + fwnode_handle_put(fwnode); > + return ret; > + } I wonder if DRM people want to see this in drivers/gpu/drm/bridge. Maybe it's better to make this a MFD driver? > [...] -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver @ 2022-09-13 10:45 ` Sebastian Reichel 0 siblings, 0 replies; 19+ messages in thread From: Sebastian Reichel @ 2022-09-13 10:45 UTC (permalink / raw) To: Bjorn Andersson Cc: devicetree, linux-pm, linux-arm-msm, Lee Jones, linux-kernel, dri-devel, Rob Herring, Krzysztof Kozlowski [-- Attachment #1: Type: text/plain, Size: 2117 bytes --] Hi, [+Cc Lee Jones, DRI devel] On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > providing AC-adapter and battery status, as well as USB Type-C altmode > notifications for Displayport operation. > > The Yoga C630 ships with Windows, where these operations primarily are > implemented in ACPI, but due to various issues with the hardware > representation therein it's not possible to run Linux on this > information. As such this is a best-effort re-implementation of these > operations, based on the register map expressed in ACPI and a fair > amount of trial and error. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > [...] > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > + if (val < 0) > + goto out_unlock; > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > + if (!ec->unit_ma) > + ec->scale = 1000; > + else > + ec->scale = 1; Since I'm not sure how much of information was gained by reverse engineering: Is this really milliamps vs microamps and not milliamps vs milliwatt? SBS batteries usually report either mA or mW. > [...] > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > + val->strval = "05072018"; > + break; why is this hardcoded? :) > [...] > + device_for_each_child_node(dev, fwnode) { > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > + if (ret < 0) > + continue; > + > + /* Got multiple ports, but altmode is only possible on port 1 */ > + if (port != 1) > + continue; > + > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > + ec->bridge.of_node = to_of_node(fwnode); > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > + > + ret = devm_drm_bridge_add(dev, &ec->bridge); > + if (ret) { > + dev_err(dev, "failed to register drm bridge\n"); > + fwnode_handle_put(fwnode); > + return ret; > + } I wonder if DRM people want to see this in drivers/gpu/drm/bridge. Maybe it's better to make this a MFD driver? > [...] -- Sebastian [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-09-13 10:45 ` Sebastian Reichel @ 2022-09-15 21:25 ` Bjorn Andersson -1 siblings, 0 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-09-15 21:25 UTC (permalink / raw) To: Sebastian Reichel Cc: Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm, Lee Jones, dri-devel On Tue, Sep 13, 2022 at 12:45:45PM +0200, Sebastian Reichel wrote: > Hi, > > [+Cc Lee Jones, DRI devel] > > On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > > providing AC-adapter and battery status, as well as USB Type-C altmode > > notifications for Displayport operation. > > > > The Yoga C630 ships with Windows, where these operations primarily are > > implemented in ACPI, but due to various issues with the hardware > > representation therein it's not possible to run Linux on this > > information. As such this is a best-effort re-implementation of these > > operations, based on the register map expressed in ACPI and a fair > > amount of trial and error. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > [...] > > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > > + if (val < 0) > > + goto out_unlock; > > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > > + if (!ec->unit_ma) > > + ec->scale = 1000; > > + else > > + ec->scale = 1; > > Since I'm not sure how much of information was gained by reverse > engineering: Is this really milliamps vs microamps and not milliamps > vs milliwatt? SBS batteries usually report either mA or mW. > Unfortunately I don't know the answer to this. > > [...] > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > > + val->strval = "05072018"; > > + break; > > why is this hardcoded? :) > I don't know, but as Daniel suggests, it would make sense to just drop it. > > [...] > > + device_for_each_child_node(dev, fwnode) { > > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > > + if (ret < 0) > > + continue; > > + > > + /* Got multiple ports, but altmode is only possible on port 1 */ > > + if (port != 1) > > + continue; > > + > > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > > + ec->bridge.of_node = to_of_node(fwnode); > > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > > + > > + ret = devm_drm_bridge_add(dev, &ec->bridge); > > + if (ret) { > > + dev_err(dev, "failed to register drm bridge\n"); > > + fwnode_handle_put(fwnode); > > + return ret; > > + } > > I wonder if DRM people want to see this in drivers/gpu/drm/bridge. > Maybe it's better to make this a MFD driver? > I did consider it, but it adds a whole bunch of boiler plate code without a lot of benefit. There are a few other cases where I think it would make sense to have drm bridges outside of drivers/gpu/drm, such as drivers/usb/typec/altmodes/ and drivers/platform/chrome/... > > [...] > > -- Sebastian Thanks, Bjorn ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver @ 2022-09-15 21:25 ` Bjorn Andersson 0 siblings, 0 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-09-15 21:25 UTC (permalink / raw) To: Sebastian Reichel Cc: devicetree, linux-pm, linux-arm-msm, Lee Jones, linux-kernel, dri-devel, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski On Tue, Sep 13, 2022 at 12:45:45PM +0200, Sebastian Reichel wrote: > Hi, > > [+Cc Lee Jones, DRI devel] > > On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > > providing AC-adapter and battery status, as well as USB Type-C altmode > > notifications for Displayport operation. > > > > The Yoga C630 ships with Windows, where these operations primarily are > > implemented in ACPI, but due to various issues with the hardware > > representation therein it's not possible to run Linux on this > > information. As such this is a best-effort re-implementation of these > > operations, based on the register map expressed in ACPI and a fair > > amount of trial and error. > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > [...] > > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > > + if (val < 0) > > + goto out_unlock; > > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > > + if (!ec->unit_ma) > > + ec->scale = 1000; > > + else > > + ec->scale = 1; > > Since I'm not sure how much of information was gained by reverse > engineering: Is this really milliamps vs microamps and not milliamps > vs milliwatt? SBS batteries usually report either mA or mW. > Unfortunately I don't know the answer to this. > > [...] > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > > + val->strval = "05072018"; > > + break; > > why is this hardcoded? :) > I don't know, but as Daniel suggests, it would make sense to just drop it. > > [...] > > + device_for_each_child_node(dev, fwnode) { > > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > > + if (ret < 0) > > + continue; > > + > > + /* Got multiple ports, but altmode is only possible on port 1 */ > > + if (port != 1) > > + continue; > > + > > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > > + ec->bridge.of_node = to_of_node(fwnode); > > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > > + > > + ret = devm_drm_bridge_add(dev, &ec->bridge); > > + if (ret) { > > + dev_err(dev, "failed to register drm bridge\n"); > > + fwnode_handle_put(fwnode); > > + return ret; > > + } > > I wonder if DRM people want to see this in drivers/gpu/drm/bridge. > Maybe it's better to make this a MFD driver? > I did consider it, but it adds a whole bunch of boiler plate code without a lot of benefit. There are a few other cases where I think it would make sense to have drm bridges outside of drivers/gpu/drm, such as drivers/usb/typec/altmodes/ and drivers/platform/chrome/... > > [...] > > -- Sebastian Thanks, Bjorn ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-09-15 21:25 ` Bjorn Andersson @ 2022-09-15 21:53 ` Dmitry Baryshkov -1 siblings, 0 replies; 19+ messages in thread From: Dmitry Baryshkov @ 2022-09-15 21:53 UTC (permalink / raw) To: Bjorn Andersson Cc: Sebastian Reichel, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm, Lee Jones, dri-devel On Fri, 16 Sept 2022 at 00:25, Bjorn Andersson <andersson@kernel.org> wrote: > > On Tue, Sep 13, 2022 at 12:45:45PM +0200, Sebastian Reichel wrote: > > Hi, > > > > [+Cc Lee Jones, DRI devel] > > > > On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > > > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > > > providing AC-adapter and battery status, as well as USB Type-C altmode > > > notifications for Displayport operation. > > > > > > The Yoga C630 ships with Windows, where these operations primarily are > > > implemented in ACPI, but due to various issues with the hardware > > > representation therein it's not possible to run Linux on this > > > information. As such this is a best-effort re-implementation of these > > > operations, based on the register map expressed in ACPI and a fair > > > amount of trial and error. > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > --- > > > [...] > > > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > > > + if (val < 0) > > > + goto out_unlock; > > > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > > > + if (!ec->unit_ma) > > > + ec->scale = 1000; > > > + else > > > + ec->scale = 1; > > > > Since I'm not sure how much of information was gained by reverse > > engineering: Is this really milliamps vs microamps and not milliamps > > vs milliwatt? SBS batteries usually report either mA or mW. > > > > Unfortunately I don't know the answer to this. > > > > [...] > > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > > > + val->strval = "05072018"; > > > + break; > > > > why is this hardcoded? :) > > > > I don't know, but as Daniel suggests, it would make sense to just drop > it. > > > > [...] > > > + device_for_each_child_node(dev, fwnode) { > > > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > > > + if (ret < 0) > > > + continue; > > > + > > > + /* Got multiple ports, but altmode is only possible on port 1 */ > > > + if (port != 1) > > > + continue; > > > + > > > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > > > + ec->bridge.of_node = to_of_node(fwnode); > > > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > > > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > > > + > > > + ret = devm_drm_bridge_add(dev, &ec->bridge); > > > + if (ret) { > > > + dev_err(dev, "failed to register drm bridge\n"); > > > + fwnode_handle_put(fwnode); > > > + return ret; > > > + } > > > > I wonder if DRM people want to see this in drivers/gpu/drm/bridge. > > Maybe it's better to make this a MFD driver? > > > > I did consider it, but it adds a whole bunch of boiler plate code > without a lot of benefit. > > There are a few other cases where I think it would make sense to have > drm bridges outside of drivers/gpu/drm, such as > drivers/usb/typec/altmodes/ and drivers/platform/chrome/... What about a solution which might sound simpler than MFD: from your driver's probe() register a platform device which represents a drm bridge. Add drm_bridge driver for it into drivers/gpu/drm/bridges/. -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver @ 2022-09-15 21:53 ` Dmitry Baryshkov 0 siblings, 0 replies; 19+ messages in thread From: Dmitry Baryshkov @ 2022-09-15 21:53 UTC (permalink / raw) To: Bjorn Andersson Cc: devicetree, linux-pm, linux-kernel, Lee Jones, Sebastian Reichel, dri-devel, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, linux-arm-msm On Fri, 16 Sept 2022 at 00:25, Bjorn Andersson <andersson@kernel.org> wrote: > > On Tue, Sep 13, 2022 at 12:45:45PM +0200, Sebastian Reichel wrote: > > Hi, > > > > [+Cc Lee Jones, DRI devel] > > > > On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > > > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > > > providing AC-adapter and battery status, as well as USB Type-C altmode > > > notifications for Displayport operation. > > > > > > The Yoga C630 ships with Windows, where these operations primarily are > > > implemented in ACPI, but due to various issues with the hardware > > > representation therein it's not possible to run Linux on this > > > information. As such this is a best-effort re-implementation of these > > > operations, based on the register map expressed in ACPI and a fair > > > amount of trial and error. > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > --- > > > [...] > > > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > > > + if (val < 0) > > > + goto out_unlock; > > > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > > > + if (!ec->unit_ma) > > > + ec->scale = 1000; > > > + else > > > + ec->scale = 1; > > > > Since I'm not sure how much of information was gained by reverse > > engineering: Is this really milliamps vs microamps and not milliamps > > vs milliwatt? SBS batteries usually report either mA or mW. > > > > Unfortunately I don't know the answer to this. > > > > [...] > > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > > > + val->strval = "05072018"; > > > + break; > > > > why is this hardcoded? :) > > > > I don't know, but as Daniel suggests, it would make sense to just drop > it. > > > > [...] > > > + device_for_each_child_node(dev, fwnode) { > > > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > > > + if (ret < 0) > > > + continue; > > > + > > > + /* Got multiple ports, but altmode is only possible on port 1 */ > > > + if (port != 1) > > > + continue; > > > + > > > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > > > + ec->bridge.of_node = to_of_node(fwnode); > > > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > > > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > > > + > > > + ret = devm_drm_bridge_add(dev, &ec->bridge); > > > + if (ret) { > > > + dev_err(dev, "failed to register drm bridge\n"); > > > + fwnode_handle_put(fwnode); > > > + return ret; > > > + } > > > > I wonder if DRM people want to see this in drivers/gpu/drm/bridge. > > Maybe it's better to make this a MFD driver? > > > > I did consider it, but it adds a whole bunch of boiler plate code > without a lot of benefit. > > There are a few other cases where I think it would make sense to have > drm bridges outside of drivers/gpu/drm, such as > drivers/usb/typec/altmodes/ and drivers/platform/chrome/... What about a solution which might sound simpler than MFD: from your driver's probe() register a platform device which represents a drm bridge. Add drm_bridge driver for it into drivers/gpu/drm/bridges/. -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver 2022-09-15 21:53 ` Dmitry Baryshkov @ 2022-09-21 22:10 ` Bjorn Andersson -1 siblings, 0 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-09-21 22:10 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Sebastian Reichel, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm, Lee Jones, dri-devel On Fri, Sep 16, 2022 at 12:53:42AM +0300, Dmitry Baryshkov wrote: > On Fri, 16 Sept 2022 at 00:25, Bjorn Andersson <andersson@kernel.org> wrote: > > > > On Tue, Sep 13, 2022 at 12:45:45PM +0200, Sebastian Reichel wrote: > > > Hi, > > > > > > [+Cc Lee Jones, DRI devel] > > > > > > On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > > > > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > > > > providing AC-adapter and battery status, as well as USB Type-C altmode > > > > notifications for Displayport operation. > > > > > > > > The Yoga C630 ships with Windows, where these operations primarily are > > > > implemented in ACPI, but due to various issues with the hardware > > > > representation therein it's not possible to run Linux on this > > > > information. As such this is a best-effort re-implementation of these > > > > operations, based on the register map expressed in ACPI and a fair > > > > amount of trial and error. > > > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > --- > > > > [...] > > > > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > > > > + if (val < 0) > > > > + goto out_unlock; > > > > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > > > > + if (!ec->unit_ma) > > > > + ec->scale = 1000; > > > > + else > > > > + ec->scale = 1; > > > > > > Since I'm not sure how much of information was gained by reverse > > > engineering: Is this really milliamps vs microamps and not milliamps > > > vs milliwatt? SBS batteries usually report either mA or mW. > > > > > > > Unfortunately I don't know the answer to this. > > > > > > [...] > > > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > > > > + val->strval = "05072018"; > > > > + break; > > > > > > why is this hardcoded? :) > > > > > > > I don't know, but as Daniel suggests, it would make sense to just drop > > it. > > > > > > [...] > > > > + device_for_each_child_node(dev, fwnode) { > > > > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > > > > + if (ret < 0) > > > > + continue; > > > > + > > > > + /* Got multiple ports, but altmode is only possible on port 1 */ > > > > + if (port != 1) > > > > + continue; > > > > + > > > > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > > > > + ec->bridge.of_node = to_of_node(fwnode); > > > > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > > > > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > > > > + > > > > + ret = devm_drm_bridge_add(dev, &ec->bridge); > > > > + if (ret) { > > > > + dev_err(dev, "failed to register drm bridge\n"); > > > > + fwnode_handle_put(fwnode); > > > > + return ret; > > > > + } > > > > > > I wonder if DRM people want to see this in drivers/gpu/drm/bridge. > > > Maybe it's better to make this a MFD driver? > > > > > > > I did consider it, but it adds a whole bunch of boiler plate code > > without a lot of benefit. > > > > There are a few other cases where I think it would make sense to have > > drm bridges outside of drivers/gpu/drm, such as > > drivers/usb/typec/altmodes/ and drivers/platform/chrome/... > > What about a solution which might sound simpler than MFD: from your > driver's probe() register a platform device which represents a drm > bridge. Add drm_bridge driver for it into drivers/gpu/drm/bridges/. > That is indeed simpler But then rather than just calling drm_bridge_hpd_notify(status) we need a new custom interface for this driver to call the other drive when status changes and we waste 840 bytes just to track another struct platform_device. Perhaps that could be made generic, so we'd have "proxy_drm_bridge" driver for drivers who implements a HPD drm_bridge but don't want to implement that outside drivers/gpu/drm. But this is just based on the assumption that the "DRM people" require drm_bridges to only live in drivers/gpu/drm/. It's all one kernel and the API is well defined, so I don't think there's a concrete problem to solve. Regards, Bjorn ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver @ 2022-09-21 22:10 ` Bjorn Andersson 0 siblings, 0 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-09-21 22:10 UTC (permalink / raw) To: Dmitry Baryshkov Cc: devicetree, linux-pm, linux-kernel, Lee Jones, Sebastian Reichel, dri-devel, Bjorn Andersson, Rob Herring, Krzysztof Kozlowski, linux-arm-msm On Fri, Sep 16, 2022 at 12:53:42AM +0300, Dmitry Baryshkov wrote: > On Fri, 16 Sept 2022 at 00:25, Bjorn Andersson <andersson@kernel.org> wrote: > > > > On Tue, Sep 13, 2022 at 12:45:45PM +0200, Sebastian Reichel wrote: > > > Hi, > > > > > > [+Cc Lee Jones, DRI devel] > > > > > > On Tue, Aug 09, 2022 at 10:05:00PM -0500, Bjorn Andersson wrote: > > > > The Qualcomm Snapdragon-based Lenovo Yoga C630 has some sort of EC > > > > providing AC-adapter and battery status, as well as USB Type-C altmode > > > > notifications for Displayport operation. > > > > > > > > The Yoga C630 ships with Windows, where these operations primarily are > > > > implemented in ACPI, but due to various issues with the hardware > > > > representation therein it's not possible to run Linux on this > > > > information. As such this is a best-effort re-implementation of these > > > > operations, based on the register map expressed in ACPI and a fair > > > > amount of trial and error. > > > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > > > --- > > > > [...] > > > > + val = yoga_c630_ec_read8(ec, LENOVO_EC_BAT_ATTRIBUTES); > > > > + if (val < 0) > > > > + goto out_unlock; > > > > + ec->unit_ma = val & LENOVO_EC_BAT_ATTR_UNIT_IS_MA; > > > > + if (!ec->unit_ma) > > > > + ec->scale = 1000; > > > > + else > > > > + ec->scale = 1; > > > > > > Since I'm not sure how much of information was gained by reverse > > > engineering: Is this really milliamps vs microamps and not milliamps > > > vs milliwatt? SBS batteries usually report either mA or mW. > > > > > > > Unfortunately I don't know the answer to this. > > > > > > [...] > > > > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > > > > + val->strval = "05072018"; > > > > + break; > > > > > > why is this hardcoded? :) > > > > > > > I don't know, but as Daniel suggests, it would make sense to just drop > > it. > > > > > > [...] > > > > + device_for_each_child_node(dev, fwnode) { > > > > + ret = fwnode_property_read_u32(fwnode, "reg", &port); > > > > + if (ret < 0) > > > > + continue; > > > > + > > > > + /* Got multiple ports, but altmode is only possible on port 1 */ > > > > + if (port != 1) > > > > + continue; > > > > + > > > > + ec->bridge.funcs = &yoga_c630_ec_bridge_funcs; > > > > + ec->bridge.of_node = to_of_node(fwnode); > > > > + ec->bridge.ops = DRM_BRIDGE_OP_HPD; > > > > + ec->bridge.type = DRM_MODE_CONNECTOR_USB; > > > > + > > > > + ret = devm_drm_bridge_add(dev, &ec->bridge); > > > > + if (ret) { > > > > + dev_err(dev, "failed to register drm bridge\n"); > > > > + fwnode_handle_put(fwnode); > > > > + return ret; > > > > + } > > > > > > I wonder if DRM people want to see this in drivers/gpu/drm/bridge. > > > Maybe it's better to make this a MFD driver? > > > > > > > I did consider it, but it adds a whole bunch of boiler plate code > > without a lot of benefit. > > > > There are a few other cases where I think it would make sense to have > > drm bridges outside of drivers/gpu/drm, such as > > drivers/usb/typec/altmodes/ and drivers/platform/chrome/... > > What about a solution which might sound simpler than MFD: from your > driver's probe() register a platform device which represents a drm > bridge. Add drm_bridge driver for it into drivers/gpu/drm/bridges/. > That is indeed simpler But then rather than just calling drm_bridge_hpd_notify(status) we need a new custom interface for this driver to call the other drive when status changes and we waste 840 bytes just to track another struct platform_device. Perhaps that could be made generic, so we'd have "proxy_drm_bridge" driver for drivers who implements a HPD drm_bridge but don't want to implement that outside drivers/gpu/drm. But this is just based on the assumption that the "DRM people" require drm_bridges to only live in drivers/gpu/drm/. It's all one kernel and the API is well defined, so I don't think there's a concrete problem to solve. Regards, Bjorn ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] power: supply: Lenovo Yoga C630 EC 2022-08-10 3:04 [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Bjorn Andersson 2022-08-10 3:04 ` [PATCH 1/2] dt-bindings: power: supply: Add " Bjorn Andersson 2022-08-10 3:05 ` [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver Bjorn Andersson @ 2022-08-10 4:04 ` Steev Klimaszewski 2022-08-10 19:15 ` Bjorn Andersson 2 siblings, 1 reply; 19+ messages in thread From: Steev Klimaszewski @ 2022-08-10 4:04 UTC (permalink / raw) To: Bjorn Andersson, Sebastian Reichel, Rob Herring, Krzysztof Kozlowski Cc: linux-pm, devicetree, linux-kernel, linux-arm-msm On 8/9/22 10:04 PM, Bjorn Andersson wrote: > This adds binding and driver for the Lenovo Yoga C630 Embedded Controller, to > provide battery information and DisplayPort support. > > Bjorn Andersson (2): > dt-bindings: power: supply: Add Lenovo Yoga C630 EC > power: supply: Add Lenovo Yoga C630 EC driver > > .../power/supply/lenovo,yoga-c630-ec.yaml | 88 +++ > drivers/power/supply/Kconfig | 11 + > drivers/power/supply/Makefile | 1 + > drivers/power/supply/yoga-c630-ec.c | 547 ++++++++++++++++++ > 4 files changed, 647 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml > create mode 100644 drivers/power/supply/yoga-c630-ec.c > It will be so nice to drop the "some-battery" patches that I've been carrying in my kernel sources since 5.7 :D Tested-by: Steev Klimaszewski <steev@kali.org> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] power: supply: Lenovo Yoga C630 EC 2022-08-10 4:04 ` [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Steev Klimaszewski @ 2022-08-10 19:15 ` Bjorn Andersson 0 siblings, 0 replies; 19+ messages in thread From: Bjorn Andersson @ 2022-08-10 19:15 UTC (permalink / raw) To: Steev Klimaszewski Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, linux-pm, devicetree, linux-kernel, linux-arm-msm On Tue 09 Aug 21:04 PDT 2022, Steev Klimaszewski wrote: > > On 8/9/22 10:04 PM, Bjorn Andersson wrote: > > This adds binding and driver for the Lenovo Yoga C630 Embedded Controller, to > > provide battery information and DisplayPort support. > > > > Bjorn Andersson (2): > > dt-bindings: power: supply: Add Lenovo Yoga C630 EC > > power: supply: Add Lenovo Yoga C630 EC driver > > > > .../power/supply/lenovo,yoga-c630-ec.yaml | 88 +++ > > drivers/power/supply/Kconfig | 11 + > > drivers/power/supply/Makefile | 1 + > > drivers/power/supply/yoga-c630-ec.c | 547 ++++++++++++++++++ > > 4 files changed, 647 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml > > create mode 100644 drivers/power/supply/yoga-c630-ec.c > > > It will be so nice to drop the "some-battery" patches that I've been > carrying in my kernel sources since 5.7 :D > Not only that, this is the only patch I've been carrying ont op of Torvalds' tree for a couple of releases now. As such, if this lands, I expect to be able to run an unmodified v6.1 straight off on my laptop. Regards, Bjorn > Tested-by: Steev Klimaszewski <steev@kali.org> > ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-09-21 22:11 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-10 3:04 [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Bjorn Andersson 2022-08-10 3:04 ` [PATCH 1/2] dt-bindings: power: supply: Add " Bjorn Andersson 2022-08-10 13:35 ` Krzysztof Kozlowski 2022-08-10 19:33 ` Bjorn Andersson 2022-08-10 15:11 ` Rob Herring 2022-08-10 3:05 ` [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver Bjorn Andersson 2022-08-13 10:23 ` kernel test robot 2022-09-12 17:00 ` Daniel Thompson 2022-09-13 8:17 ` Daniel Thompson 2022-09-13 10:45 ` Sebastian Reichel 2022-09-13 10:45 ` Sebastian Reichel 2022-09-15 21:25 ` Bjorn Andersson 2022-09-15 21:25 ` Bjorn Andersson 2022-09-15 21:53 ` Dmitry Baryshkov 2022-09-15 21:53 ` Dmitry Baryshkov 2022-09-21 22:10 ` Bjorn Andersson 2022-09-21 22:10 ` Bjorn Andersson 2022-08-10 4:04 ` [PATCH 0/2] power: supply: Lenovo Yoga C630 EC Steev Klimaszewski 2022-08-10 19:15 ` Bjorn Andersson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.