linux-kernel - Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver

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

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

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

Message-ID: <20220913104545.2u6mcyy3bg4dp6ly@mercury.elektranox.org>
Date:   Tue, 13 Sep 2022 12:45:45 +0200
From:   Sebastian Reichel <sebastian.reichel@...labora.com>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        Lee Jones <lee@...nel.org>, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 2/2] power: supply: Add Lenovo Yoga C630 EC driver

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@...aro.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

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)