| Message ID | 20230205152809.2233436-1-dmitry.baryshkov@linaro.org |
|---|---|
| Headers | show |
| Series | power: supply: Lenovo Yoga C630 EC | expand |
On 05/02/2023 16:28, Dmitry Baryshkov wrote: > From: Bjorn Andersson <bjorn.andersson@linaro.org> > > Add binding for the Embedded Controller found in the Qualcomm > Snapdragon-based Lenovo Yoga C630. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Loooks ok for me. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> If there is going to be resend, three nits below: > --- > .../power/supply/lenovo,yoga-c630-ec.yaml | 83 +++++++++++++++++++ > 1 file changed, 83 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..37977344f157 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml > @@ -0,0 +1,83 @@ > +# 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. Drop trailing full stop (...) > + > +examples: > + - |+ Just: - | > + #include <dt-bindings/interrupt-controller/irq.h> > + i2c1 { Just: "i2c" > + clock-frequency = <400000>; > + > + #address-cells = <1>; > + #size-cells = <0>; Best regards, Krzysztof
On Sun, Feb 05, 2023 at 05:28:07PM +0200, Dmitry Baryshkov wrote: > From: Bjorn Andersson <bjorn.andersson@linaro.org> > > Add binding for the Embedded Controller found in the Qualcomm > Snapdragon-based Lenovo Yoga C630. > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > .../power/supply/lenovo,yoga-c630-ec.yaml | 83 +++++++++++++++++++ > 1 file changed, 83 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..37977344f157 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml > @@ -0,0 +1,83 @@ > +# 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> Since this is new: Should this be updated with Bjorn's kernel.org address? Last I checked, this address doesn't exist anymore. Brian
On Sun, Feb 05, 2023 at 05:28:08PM +0200, Dmitry Baryshkov wrote: > +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; You can simplify this function by getting rid of the switch statement and rc variable: if (psp == POWER_SUPPLY_PROP_ONLINE) { val->intval = ec->adapter_online; return 0; } return -EINVAL; Brian
On 06/02/2023 20:19, Brian Masney wrote: > On Sun, Feb 05, 2023 at 05:28:07PM +0200, Dmitry Baryshkov wrote: >> From: Bjorn Andersson <bjorn.andersson@linaro.org> >> >> Add binding for the Embedded Controller found in the Qualcomm >> Snapdragon-based Lenovo Yoga C630. >> >> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> .../power/supply/lenovo,yoga-c630-ec.yaml | 83 +++++++++++++++++++ >> 1 file changed, 83 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..37977344f157 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/supply/lenovo,yoga-c630-ec.yaml >> @@ -0,0 +1,83 @@ >> +# 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> > > Since this is new: Should this be updated with Bjorn's kernel.org > address? Last I checked, this address doesn't exist anymore. Ack, nice catch. > > Brian >
Hi, On Sun, Feb 05, 2023 at 05:28:08PM +0200, Dmitry Baryshkov wrote: > From: Bjorn Andersson <bjorn.andersson@linaro.org> > > 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. > > Tested-by: Steev Klimaszewski <steev@kali.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- [...] > +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; For other platforms I've seen the current capacity sometimes reaching higher values than the full charge capacity, so it's better to use <= here. > + return false; > +} [...] > +static int yoga_c630_ec_query_usb_event(struct yoga_c630_ec *ec) > +{ > + struct device *dev = &ec->client->dev; > + 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; > + } > + > + /* FIXME: handle the returned event to set the Type-C properties */ I guess this is more of a TODO than a FIXME? :) > + > + return 0; > +} -- Sebastian